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 <[email protected]>
---
drivers/net/dsa/realtek/rtl8366rb.c | 44 +++++++++++++++++++++++++++++--------
1 file changed, 35 insertions(+), 9 deletions(-)
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);
---
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
change-id: 20230912-rtl8366rb-link-7b2ffa01d042
Best regards,
--
Linus Walleij <[email protected]>
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 <[email protected]>
Reviewed-by: Alvin Šipraga <[email protected]>
> ---
> drivers/net/dsa/realtek/rtl8366rb.c | 44 +++++++++++++++++++++++++++++--------
> 1 file changed, 35 insertions(+), 9 deletions(-)
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 <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
--
Florian
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 <[email protected]>
> ---
Reviewed-by: Vladimir Oltean <[email protected]>
Hello:
This patch was applied to netdev/net-next.git (main)
by David S. Miller <[email protected]>:
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html