| Message ID | E1nNGmL-00AOjC-HP@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: sja1105: phylink updates | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 40 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Thu, Feb 24, 2022 at 04:15:41PM +0000, Russell King (Oracle) wrote: > Vladimir Oltean suggests that sla1105 can support switching between s/sla1105/sja1105/ > SGMII and 2500BASE-X modes. Augment sja1105_phylink_get_caps() to > fill in both interface modes if they can be supported. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- > drivers/net/dsa/sja1105/sja1105_main.c | 28 +++++++++++++++++++++----- > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c > index 5beef06d8ff7..36001b1d7968 100644 > --- a/drivers/net/dsa/sja1105/sja1105_main.c > +++ b/drivers/net/dsa/sja1105/sja1105_main.c > @@ -1396,6 +1396,7 @@ static void sja1105_phylink_get_caps(struct dsa_switch *ds, int port, > { > struct sja1105_private *priv = ds->priv; > struct sja1105_xmii_params_entry *mii; > + phy_interface_t phy_mode; > > /* This driver does not make use of the speed, duplex, pause or the > * advertisement in its mac_config, so it is safe to mark this driver > @@ -1403,11 +1404,28 @@ static void sja1105_phylink_get_caps(struct dsa_switch *ds, int port, > */ > config->legacy_pre_march2020 = false; > > - /* The SJA1105 MAC programming model is through the static config > - * (the xMII Mode table cannot be dynamically reconfigured), and > - * we have to program that early. > - */ > - __set_bit(priv->phy_mode[port], config->supported_interfaces); > + phy_mode = priv->phy_mode[port]; > + if (phy_mode == PHY_INTERFACE_MODE_SGMII || > + phy_mode == PHY_INTERFACE_MODE_2500BASEX) { > + /* Changing the PHY mode on SERDES ports is possible and makes > + * sense, because that is done through the XPCS. We allow > + * changes between SGMII and 2500base-X (it is unknown whether > + * 1000base-X is supported). > + */ It is actually known (or so I think). Bits 2:1 (PCS_MODE) of register VR_MII_AN_CTRL (MMD 0x1f, address 0x8001) of the XPCS, as instantiated in SJA1105R/S, says: 00: Clause 37 auto-negotiation for 1000BASE-X mode *Not supported* 10: Clause 37 auto-negotiation for SGMII mode When I look at the XPCS documentation for SJA1110, it doesn't say "Not supported", however I don't have the setup to try it. If it's anything like the XPCS instantiation from SJA1105 though, this is possibly a documentation glitch and I wouldn't say it was implemented just because the documentation doesn't say it isn't. On the other hand, disabling SGMII in-band autoneg is possible, and the resulting mode is electrically compatible with 1000Base-X without in-band autoneg. Interpret this as you wish. > + if (priv->info->supports_sgmii[port]) > + __set_bit(PHY_INTERFACE_MODE_SGMII, > + config->supported_interfaces); > + > + if (priv->info->supports_2500basex[port]) > + __set_bit(PHY_INTERFACE_MODE_2500BASEX, > + config->supported_interfaces); > + } else { > + /* The SJA1105 MAC programming model is through the static > + * config (the xMII Mode table cannot be dynamically > + * reconfigured), and we have to program that early. > + */ > + __set_bit(phy_mode, config->supported_interfaces); > + } > > /* The MAC does not support pause frames, and also doesn't > * support half-duplex traffic modes. > -- > 2.30.2 >
On Fri, Feb 25, 2022 at 01:16:49PM +0200, Vladimir Oltean wrote: > On Thu, Feb 24, 2022 at 04:15:41PM +0000, Russell King (Oracle) wrote: > > Vladimir Oltean suggests that sla1105 can support switching between > > s/sla1105/sja1105/ Thanks for catching that. > > SGMII and 2500BASE-X modes. Augment sja1105_phylink_get_caps() to > > fill in both interface modes if they can be supported. > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > --- > > drivers/net/dsa/sja1105/sja1105_main.c | 28 +++++++++++++++++++++----- > > 1 file changed, 23 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c > > index 5beef06d8ff7..36001b1d7968 100644 > > --- a/drivers/net/dsa/sja1105/sja1105_main.c > > +++ b/drivers/net/dsa/sja1105/sja1105_main.c > > @@ -1396,6 +1396,7 @@ static void sja1105_phylink_get_caps(struct dsa_switch *ds, int port, > > { > > struct sja1105_private *priv = ds->priv; > > struct sja1105_xmii_params_entry *mii; > > + phy_interface_t phy_mode; > > > > /* This driver does not make use of the speed, duplex, pause or the > > * advertisement in its mac_config, so it is safe to mark this driver > > @@ -1403,11 +1404,28 @@ static void sja1105_phylink_get_caps(struct dsa_switch *ds, int port, > > */ > > config->legacy_pre_march2020 = false; > > > > - /* The SJA1105 MAC programming model is through the static config > > - * (the xMII Mode table cannot be dynamically reconfigured), and > > - * we have to program that early. > > - */ > > - __set_bit(priv->phy_mode[port], config->supported_interfaces); > > + phy_mode = priv->phy_mode[port]; > > + if (phy_mode == PHY_INTERFACE_MODE_SGMII || > > + phy_mode == PHY_INTERFACE_MODE_2500BASEX) { > > + /* Changing the PHY mode on SERDES ports is possible and makes > > + * sense, because that is done through the XPCS. We allow > > + * changes between SGMII and 2500base-X (it is unknown whether > > + * 1000base-X is supported). > > + */ > > It is actually known (or so I think). > Bits 2:1 (PCS_MODE) of register VR_MII_AN_CTRL (MMD 0x1f, address 0x8001) > of the XPCS, as instantiated in SJA1105R/S, says: > 00: Clause 37 auto-negotiation for 1000BASE-X mode > *Not supported* > 10: Clause 37 auto-negotiation for SGMII mode > > When I look at the XPCS documentation for SJA1110, it doesn't say > "Not supported", however I don't have the setup to try it. > If it's anything like the XPCS instantiation from SJA1105 though, this > is possibly a documentation glitch and I wouldn't say it was implemented > just because the documentation doesn't say it isn't. > > On the other hand, disabling SGMII in-band autoneg is possible, and the > resulting mode is electrically compatible with 1000Base-X without > in-band autoneg. Interpret this as you wish. The comment above comes directly from your patch back in November. Are you suggesting you aren't happy with your own comment? If you would like to update it, please let me have a suitable replacement for it. Thanks.
On Fri, Feb 25, 2022 at 11:23:20AM +0000, Russell King (Oracle) wrote: > The comment above comes directly from your patch back in November. > Are you suggesting you aren't happy with your own comment? If you > would like to update it, please let me have a suitable replacement > for it. Yes, I'm not happy with my own comment, just remove the text between the parentheses. Thanks.
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c index 5beef06d8ff7..36001b1d7968 100644 --- a/drivers/net/dsa/sja1105/sja1105_main.c +++ b/drivers/net/dsa/sja1105/sja1105_main.c @@ -1396,6 +1396,7 @@ static void sja1105_phylink_get_caps(struct dsa_switch *ds, int port, { struct sja1105_private *priv = ds->priv; struct sja1105_xmii_params_entry *mii; + phy_interface_t phy_mode; /* This driver does not make use of the speed, duplex, pause or the * advertisement in its mac_config, so it is safe to mark this driver @@ -1403,11 +1404,28 @@ static void sja1105_phylink_get_caps(struct dsa_switch *ds, int port, */ config->legacy_pre_march2020 = false; - /* The SJA1105 MAC programming model is through the static config - * (the xMII Mode table cannot be dynamically reconfigured), and - * we have to program that early. - */ - __set_bit(priv->phy_mode[port], config->supported_interfaces); + phy_mode = priv->phy_mode[port]; + if (phy_mode == PHY_INTERFACE_MODE_SGMII || + phy_mode == PHY_INTERFACE_MODE_2500BASEX) { + /* Changing the PHY mode on SERDES ports is possible and makes + * sense, because that is done through the XPCS. We allow + * changes between SGMII and 2500base-X (it is unknown whether + * 1000base-X is supported). + */ + if (priv->info->supports_sgmii[port]) + __set_bit(PHY_INTERFACE_MODE_SGMII, + config->supported_interfaces); + + if (priv->info->supports_2500basex[port]) + __set_bit(PHY_INTERFACE_MODE_2500BASEX, + config->supported_interfaces); + } else { + /* The SJA1105 MAC programming model is through the static + * config (the xMII Mode table cannot be dynamically + * reconfigured), and we have to program that early. + */ + __set_bit(phy_mode, config->supported_interfaces); + } /* The MAC does not support pause frames, and also doesn't * support half-duplex traffic modes.
Vladimir Oltean suggests that sla1105 can support switching between SGMII and 2500BASE-X modes. Augment sja1105_phylink_get_caps() to fill in both interface modes if they can be supported. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/net/dsa/sja1105/sja1105_main.c | 28 +++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-)