| Message ID | 20210406203508.476122-3-martin.blumenstingl@googlemail.com |
|---|---|
| State | New |
| Headers | show |
| Series | lantiq: GSWIP: two more fixes | expand |
On 4/6/21 10:35 PM, Martin Blumenstingl wrote: > There are a few more bits in the GSWIP_MII_CFG register for which we > did rely on the boot-loader (or the hardware defaults) to set them up > properly. > > For some external RMII PHYs we need to select the GSWIP_MII_CFG_RMII_CLK > bit and also we should un-set it for non-RMII PHYs. The GSWIP_MII_CFG_RMII_CLK option is ignored in other modes. > The GSWIP IP also > supports in-band auto-negotiation for RGMII PHYs. Set or unset the > corresponding bit depending on the auto-negotiation mode. > > Clear the xMII isolation bit when set at initialization time if it was > previously set by the bootloader. Not doing so could lead to no traffic > (neither RX nor TX) on a port with this bit set. > > While here, also add the GSWIP_MII_CFG_RESET bit. We don't need to > manage it because this bit is self-clearning when set. We still add it > here to get a better overview of the GSWIP_MII_CFG register. > > Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200") > Cc: stable@vger.kernel.org > Suggested-by: Hauke Mehrtens <hauke@hauke-m.de> > Acked-by: Hauke Mehrtens <hauke@hauke-m.de> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > drivers/net/dsa/lantiq_gswip.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c > index 47ea3a8c90a4..f330035ed85b 100644 > --- a/drivers/net/dsa/lantiq_gswip.c > +++ b/drivers/net/dsa/lantiq_gswip.c > @@ -93,8 +93,12 @@ > > /* GSWIP MII Registers */ > #define GSWIP_MII_CFGp(p) (0x2 * (p)) > +#define GSWIP_MII_CFG_RESET BIT(15) > #define GSWIP_MII_CFG_EN BIT(14) > +#define GSWIP_MII_CFG_ISOLATE BIT(13) > #define GSWIP_MII_CFG_LDCLKDIS BIT(12) > +#define GSWIP_MII_CFG_RGMII_IBS BIT(8) > +#define GSWIP_MII_CFG_RMII_CLK BIT(7) > #define GSWIP_MII_CFG_MODE_MIIP 0x0 > #define GSWIP_MII_CFG_MODE_MIIM 0x1 > #define GSWIP_MII_CFG_MODE_RMIIP 0x2 > @@ -821,9 +825,11 @@ static int gswip_setup(struct dsa_switch *ds) > /* Configure the MDIO Clock 2.5 MHz */ > gswip_mdio_mask(priv, 0xff, 0x09, GSWIP_MDIO_MDC_CFG1); > > - /* Disable the xMII link */ > + /* Disable the xMII interface and clear it's isolation bit */ > for (i = 0; i < priv->hw_info->max_ports; i++) > - gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_EN, 0, i); > + gswip_mii_mask_cfg(priv, > + GSWIP_MII_CFG_EN | GSWIP_MII_CFG_ISOLATE, > + 0, i); > > /* enable special tag insertion on cpu port */ > gswip_switch_mask(priv, 0, GSWIP_FDMA_PCTRL_STEN, > @@ -1597,19 +1603,29 @@ static void gswip_phylink_mac_config(struct dsa_switch *ds, int port, > break; > case PHY_INTERFACE_MODE_RMII: > miicfg |= GSWIP_MII_CFG_MODE_RMIIM; > + > + /* Configure the RMII clock as output: */ > + miicfg |= GSWIP_MII_CFG_RMII_CLK; > break; > case PHY_INTERFACE_MODE_RGMII: > case PHY_INTERFACE_MODE_RGMII_ID: > case PHY_INTERFACE_MODE_RGMII_RXID: > case PHY_INTERFACE_MODE_RGMII_TXID: > miicfg |= GSWIP_MII_CFG_MODE_RGMII; > + > + if (phylink_autoneg_inband(mode)) > + miicfg |= GSWIP_MII_CFG_RGMII_IBS; > break; > default: > dev_err(ds->dev, > "Unsupported interface: %d\n", state->interface); > return; > } > - gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_MODE_MASK, miicfg, port); > + > + gswip_mii_mask_cfg(priv, > + GSWIP_MII_CFG_MODE_MASK | GSWIP_MII_CFG_RMII_CLK | > + GSWIP_MII_CFG_RGMII_IBS | GSWIP_MII_CFG_LDCLKDIS, > + miicfg, port); > > gswip_port_set_link(priv, port, state->link); > gswip_port_set_speed(priv, port, state->speed, state->interface); >
> case PHY_INTERFACE_MODE_RGMII: > case PHY_INTERFACE_MODE_RGMII_ID: > case PHY_INTERFACE_MODE_RGMII_RXID: > case PHY_INTERFACE_MODE_RGMII_TXID: > miicfg |= GSWIP_MII_CFG_MODE_RGMII; > + > + if (phylink_autoneg_inband(mode)) > + miicfg |= GSWIP_MII_CFG_RGMII_IBS; Is there any other MAC driver doing this? Are there any boards actually enabling it? Since it is so odd, if there is nothing using it, i would be tempted to leave this out. Andrew
On 4/6/2021 5:32 PM, Andrew Lunn wrote: >> case PHY_INTERFACE_MODE_RGMII: >> case PHY_INTERFACE_MODE_RGMII_ID: >> case PHY_INTERFACE_MODE_RGMII_RXID: >> case PHY_INTERFACE_MODE_RGMII_TXID: >> miicfg |= GSWIP_MII_CFG_MODE_RGMII; >> + >> + if (phylink_autoneg_inband(mode)) >> + miicfg |= GSWIP_MII_CFG_RGMII_IBS; > > Is there any other MAC driver doing this? Are there any boards > actually enabling it? Since it is so odd, if there is nothing using > it, i would be tempted to leave this out. Some PHYs (Broadcom namely) support suppressing the RGMII in-band signaling towards the MAC, so if the MAC relies on that signaling to configure itself based on what the PHY reports this may not work.
Hello, On Wed, Apr 7, 2021 at 6:47 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > > > > On 4/6/2021 5:32 PM, Andrew Lunn wrote: > >> case PHY_INTERFACE_MODE_RGMII: > >> case PHY_INTERFACE_MODE_RGMII_ID: > >> case PHY_INTERFACE_MODE_RGMII_RXID: > >> case PHY_INTERFACE_MODE_RGMII_TXID: > >> miicfg |= GSWIP_MII_CFG_MODE_RGMII; > >> + > >> + if (phylink_autoneg_inband(mode)) > >> + miicfg |= GSWIP_MII_CFG_RGMII_IBS; > > > > Is there any other MAC driver doing this? Are there any boards > > actually enabling it? Since it is so odd, if there is nothing using > > it, i would be tempted to leave this out. > > Some PHYs (Broadcom namely) support suppressing the RGMII in-band > signaling towards the MAC, so if the MAC relies on that signaling to > configure itself based on what the PHY reports this may not work. point taken. in v2 we'll not set GSWIP_MII_CFG_RGMII_IBS unless there's someone who can actually test this. so far I don't know any hardware with Lantiq SoC that uses it Best regards, Martin
On 4/7/21 2:32 AM, Andrew Lunn wrote: >> case PHY_INTERFACE_MODE_RGMII: >> case PHY_INTERFACE_MODE_RGMII_ID: >> case PHY_INTERFACE_MODE_RGMII_RXID: >> case PHY_INTERFACE_MODE_RGMII_TXID: >> miicfg |= GSWIP_MII_CFG_MODE_RGMII; >> + >> + if (phylink_autoneg_inband(mode)) >> + miicfg |= GSWIP_MII_CFG_RGMII_IBS; > > Is there any other MAC driver doing this? Are there any boards > actually enabling it? Since it is so odd, if there is nothing using > it, i would be tempted to leave this out. We saw this option in the switch documentation and activated it to prepare for such systems, but I do not have any board which uses this and I am also not aware that this is used anywhere. Hauke
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c index 47ea3a8c90a4..f330035ed85b 100644 --- a/drivers/net/dsa/lantiq_gswip.c +++ b/drivers/net/dsa/lantiq_gswip.c @@ -93,8 +93,12 @@ /* GSWIP MII Registers */ #define GSWIP_MII_CFGp(p) (0x2 * (p)) +#define GSWIP_MII_CFG_RESET BIT(15) #define GSWIP_MII_CFG_EN BIT(14) +#define GSWIP_MII_CFG_ISOLATE BIT(13) #define GSWIP_MII_CFG_LDCLKDIS BIT(12) +#define GSWIP_MII_CFG_RGMII_IBS BIT(8) +#define GSWIP_MII_CFG_RMII_CLK BIT(7) #define GSWIP_MII_CFG_MODE_MIIP 0x0 #define GSWIP_MII_CFG_MODE_MIIM 0x1 #define GSWIP_MII_CFG_MODE_RMIIP 0x2 @@ -821,9 +825,11 @@ static int gswip_setup(struct dsa_switch *ds) /* Configure the MDIO Clock 2.5 MHz */ gswip_mdio_mask(priv, 0xff, 0x09, GSWIP_MDIO_MDC_CFG1); - /* Disable the xMII link */ + /* Disable the xMII interface and clear it's isolation bit */ for (i = 0; i < priv->hw_info->max_ports; i++) - gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_EN, 0, i); + gswip_mii_mask_cfg(priv, + GSWIP_MII_CFG_EN | GSWIP_MII_CFG_ISOLATE, + 0, i); /* enable special tag insertion on cpu port */ gswip_switch_mask(priv, 0, GSWIP_FDMA_PCTRL_STEN, @@ -1597,19 +1603,29 @@ static void gswip_phylink_mac_config(struct dsa_switch *ds, int port, break; case PHY_INTERFACE_MODE_RMII: miicfg |= GSWIP_MII_CFG_MODE_RMIIM; + + /* Configure the RMII clock as output: */ + miicfg |= GSWIP_MII_CFG_RMII_CLK; break; case PHY_INTERFACE_MODE_RGMII: case PHY_INTERFACE_MODE_RGMII_ID: case PHY_INTERFACE_MODE_RGMII_RXID: case PHY_INTERFACE_MODE_RGMII_TXID: miicfg |= GSWIP_MII_CFG_MODE_RGMII; + + if (phylink_autoneg_inband(mode)) + miicfg |= GSWIP_MII_CFG_RGMII_IBS; break; default: dev_err(ds->dev, "Unsupported interface: %d\n", state->interface); return; } - gswip_mii_mask_cfg(priv, GSWIP_MII_CFG_MODE_MASK, miicfg, port); + + gswip_mii_mask_cfg(priv, + GSWIP_MII_CFG_MODE_MASK | GSWIP_MII_CFG_RMII_CLK | + GSWIP_MII_CFG_RGMII_IBS | GSWIP_MII_CFG_LDCLKDIS, + miicfg, port); gswip_port_set_link(priv, port, state->link); gswip_port_set_speed(priv, port, state->speed, state->interface);
There are a few more bits in the GSWIP_MII_CFG register for which we did rely on the boot-loader (or the hardware defaults) to set them up properly. For some external RMII PHYs we need to select the GSWIP_MII_CFG_RMII_CLK bit and also we should un-set it for non-RMII PHYs. The GSWIP IP also supports in-band auto-negotiation for RGMII PHYs. Set or unset the corresponding bit depending on the auto-negotiation mode. Clear the xMII isolation bit when set at initialization time if it was previously set by the bootloader. Not doing so could lead to no traffic (neither RX nor TX) on a port with this bit set. While here, also add the GSWIP_MII_CFG_RESET bit. We don't need to manage it because this bit is self-clearning when set. We still add it here to get a better overview of the GSWIP_MII_CFG register. Fixes: 14fceff4771e51 ("net: dsa: Add Lantiq / Intel DSA driver for vrx200") Cc: stable@vger.kernel.org Suggested-by: Hauke Mehrtens <hauke@hauke-m.de> Acked-by: Hauke Mehrtens <hauke@hauke-m.de> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- drivers/net/dsa/lantiq_gswip.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-)