| Message ID | 20230912-rtl8366rb-link-v1-1-216eed63f357@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7c192ce9ff1d93f004fa6cdc0f567f5ab3dd4feb |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: dsa: rtl8366rb: Implement setting up link on CPU port | expand |
On Tue, Sep 12, 2023 at 11:24:18PM +0200, Linus Walleij wrote: > We auto-negotiate most ports in the RTL8366RB driver, but > the CPU port is hard-coded to 1Gbit, full duplex, tx and > rx pause. > > This isn't very nice. People may configure speed and > duplex differently in the device tree. > > Actually respect the arguments passed to the function for > the CPU port, which get passed properly after Russell's > patch "net: dsa: realtek: add phylink_get_caps implementation" > > After this the link is still set up properly. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Reviewed-by: Alvin Šipraga <alsi@bang-olufsen.dk> > --- > drivers/net/dsa/realtek/rtl8366rb.c | 44 +++++++++++++++++++++++++++++-------- > 1 file changed, 35 insertions(+), 9 deletions(-)
On Tue, Sep 12, 2023 at 11:24:18PM +0200, Linus Walleij wrote: > We auto-negotiate most ports in the RTL8366RB driver, but > the CPU port is hard-coded to 1Gbit, full duplex, tx and > rx pause. > > This isn't very nice. People may configure speed and > duplex differently in the device tree. > > Actually respect the arguments passed to the function for > the CPU port, which get passed properly after Russell's > patch "net: dsa: realtek: add phylink_get_caps implementation" > > After this the link is still set up properly. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
On 9/12/2023 2:24 PM, Linus Walleij wrote: > We auto-negotiate most ports in the RTL8366RB driver, but > the CPU port is hard-coded to 1Gbit, full duplex, tx and > rx pause. > > This isn't very nice. People may configure speed and > duplex differently in the device tree. > > Actually respect the arguments passed to the function for > the CPU port, which get passed properly after Russell's > patch "net: dsa: realtek: add phylink_get_caps implementation" > > After this the link is still set up properly. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Hello: This patch was applied to netdev/net-next.git (main) by David S. Miller <davem@davemloft.net>: On Tue, 12 Sep 2023 23:24:18 +0200 you wrote: > We auto-negotiate most ports in the RTL8366RB driver, but > the CPU port is hard-coded to 1Gbit, full duplex, tx and > rx pause. > > This isn't very nice. People may configure speed and > duplex differently in the device tree. > > [...] Here is the summary with links: - [net-next] net: dsa: rtl8366rb: Implement setting up link on CPU port https://git.kernel.org/netdev/net-next/c/7c192ce9ff1d You are awesome, thank you!
diff --git a/drivers/net/dsa/realtek/rtl8366rb.c b/drivers/net/dsa/realtek/rtl8366rb.c index 7868ef237f6c..b39b719a5b8f 100644 --- a/drivers/net/dsa/realtek/rtl8366rb.c +++ b/drivers/net/dsa/realtek/rtl8366rb.c @@ -95,12 +95,6 @@ #define RTL8366RB_PAACR_RX_PAUSE BIT(6) #define RTL8366RB_PAACR_AN BIT(7) -#define RTL8366RB_PAACR_CPU_PORT (RTL8366RB_PAACR_SPEED_1000M | \ - RTL8366RB_PAACR_FULL_DUPLEX | \ - RTL8366RB_PAACR_LINK_UP | \ - RTL8366RB_PAACR_TX_PAUSE | \ - RTL8366RB_PAACR_RX_PAUSE) - /* bits 0..7 = port 0, bits 8..15 = port 1 */ #define RTL8366RB_PSTAT0 0x0014 /* bits 0..7 = port 2, bits 8..15 = port 3 */ @@ -1081,29 +1075,61 @@ rtl8366rb_mac_link_up(struct dsa_switch *ds, int port, unsigned int mode, int speed, int duplex, bool tx_pause, bool rx_pause) { struct realtek_priv *priv = ds->priv; + unsigned int val; int ret; + /* Allow forcing the mode on the fixed CPU port, no autonegotiation. + * We assume autonegotiation works on the PHY-facing ports. + */ if (port != priv->cpu_port) return; dev_dbg(priv->dev, "MAC link up on CPU port (%d)\n", port); - /* Force the fixed CPU port into 1Gbit mode, no autonegotiation */ ret = regmap_update_bits(priv->map, RTL8366RB_MAC_FORCE_CTRL_REG, BIT(port), BIT(port)); if (ret) { - dev_err(priv->dev, "failed to force 1Gbit on CPU port\n"); + dev_err(priv->dev, "failed to force CPU port\n"); return; } + /* Conjure port config */ + switch (speed) { + case SPEED_10: + val = RTL8366RB_PAACR_SPEED_10M; + break; + case SPEED_100: + val = RTL8366RB_PAACR_SPEED_100M; + break; + case SPEED_1000: + val = RTL8366RB_PAACR_SPEED_1000M; + break; + default: + val = RTL8366RB_PAACR_SPEED_1000M; + break; + } + + if (duplex == DUPLEX_FULL) + val |= RTL8366RB_PAACR_FULL_DUPLEX; + + if (tx_pause) + val |= RTL8366RB_PAACR_TX_PAUSE; + + if (rx_pause) + val |= RTL8366RB_PAACR_RX_PAUSE; + + val |= RTL8366RB_PAACR_LINK_UP; + ret = regmap_update_bits(priv->map, RTL8366RB_PAACR2, 0xFF00U, - RTL8366RB_PAACR_CPU_PORT << 8); + val << 8); if (ret) { dev_err(priv->dev, "failed to set PAACR on CPU port\n"); return; } + dev_dbg(priv->dev, "set PAACR to %04x\n", val); + /* Enable the CPU port */ ret = regmap_update_bits(priv->map, RTL8366RB_PECR, BIT(port), 0);
We auto-negotiate most ports in the RTL8366RB driver, but the CPU port is hard-coded to 1Gbit, full duplex, tx and rx pause. This isn't very nice. People may configure speed and duplex differently in the device tree. Actually respect the arguments passed to the function for the CPU port, which get passed properly after Russell's patch "net: dsa: realtek: add phylink_get_caps implementation" After this the link is still set up properly. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/net/dsa/realtek/rtl8366rb.c | 44 +++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 9 deletions(-) --- base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d change-id: 20230912-rtl8366rb-link-7b2ffa01d042 Best regards,