| Message ID | E1nFdPN-006Wh6-LC@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: b53: convert to phylink_generic_validate() and mark as non-legacy | expand |
On 2/3/22 6:48 AM, Russell King (Oracle) wrote: > Populate the supported interfaces and MAC capabilities for the Broadcom > B53 DSA switches in preparation to using these for the generic > validation functionality. > > The interface modes are derived from: > - b53_serdes_phylink_validate() > - SRAB mux configuration > > NOTE: much of this conversion is a guess as the driver doesn't contain > sufficient information. I would appreciate a thorough review and > testing of this change before it is merged. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> This patch breaks with the following: [ 2.680318] b53-srab-switch 18036000.ethernet-switch sfp (uninitialized): failed to validate link configuration for in-band status [ 2.692470] error creating PHYLINK: -22 [ 2.696441] b53-srab-switch 18036000.ethernet-switch sfp (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 5 Adding more debug shows us the following: [ 2.804854] phylink_validate: unable to find a mode for 4 0000000,000001ff,0060004c [ 2.812807] phylink_validate: unable to find a mode for 4 0000000,000001ff,0060004c [ 2.820733] b53-srab-switch 18036000.ethernet-switch sfp (uninitialized): failed to validate link configuration for in-band status [ 2.832868] error creating PHYLINK: -22 4 = PHY_INTERFACE_MODE_SGMII and the config->supported_interfaces bitmap is printed. If we add this hunk, you entire patch set works again: @@ -178,10 +180,14 @@ void b53_serdes_phylink_get_caps(struct b53_device *dev, int port, __set_bit(PHY_INTERFACE_MODE_1000BASEX, config->supported_interfaces); config->mac_capabilities |= MAC_1000FD; + __set_bit(PHY_INTERFACE_MODE_SGMII, + config->supported_interfaces); break; default: break; } -- Florian
On Wed, Feb 16, 2022 at 11:51:49AM -0800, Florian Fainelli wrote: > On 2/3/22 6:48 AM, Russell King (Oracle) wrote: > > Populate the supported interfaces and MAC capabilities for the Broadcom > > B53 DSA switches in preparation to using these for the generic > > validation functionality. > > > > The interface modes are derived from: > > - b53_serdes_phylink_validate() > > - SRAB mux configuration > > > > NOTE: much of this conversion is a guess as the driver doesn't contain > > sufficient information. I would appreciate a thorough review and > > testing of this change before it is merged. > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > This patch breaks with the following: > > [ 2.680318] b53-srab-switch 18036000.ethernet-switch sfp > (uninitialized): failed to validate link configuration for in-band status > [ 2.692470] error creating PHYLINK: -22 > [ 2.696441] b53-srab-switch 18036000.ethernet-switch sfp > (uninitialized): error -22 setting up PHY for tree 0, switch 0, port 5 > > Adding more debug shows us the following: > > [ 2.804854] phylink_validate: unable to find a mode for 4 > 0000000,000001ff,0060004c > [ 2.812807] phylink_validate: unable to find a mode for 4 > 0000000,000001ff,0060004c > [ 2.820733] b53-srab-switch 18036000.ethernet-switch sfp > (uninitialized): failed to validate link configuration for in-band status > [ 2.832868] error creating PHYLINK: -22 > > 4 = PHY_INTERFACE_MODE_SGMII and the config->supported_interfaces bitmap > is printed. If we add this hunk, you entire patch set works again: > > @@ -178,10 +180,14 @@ void b53_serdes_phylink_get_caps(struct b53_device > *dev, int port, > __set_bit(PHY_INTERFACE_MODE_1000BASEX, > config->supported_interfaces); > config->mac_capabilities |= MAC_1000FD; > + __set_bit(PHY_INTERFACE_MODE_SGMII, > + config->supported_interfaces); > break; > default: > break; > } Thanks, added, and I've updated the comment as well just above the first line of the above hunk to be consistent with the code.
diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index 7d62b0aeaae9..211c0e499370 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -1353,6 +1353,33 @@ void b53_phylink_validate(struct dsa_switch *ds, int port, } EXPORT_SYMBOL(b53_phylink_validate); +static void b53_phylink_get_caps(struct dsa_switch *ds, int port, + struct phylink_config *config) +{ + struct b53_device *dev = ds->priv; + + /* Internal ports need GMII for PHYLIB */ + __set_bit(PHY_INTERFACE_MODE_GMII, config->supported_interfaces); + + /* These switches appear to support MII and RevMII too, but beyond + * this, the code gives very few clues. FIXME: We probably need more + * interface modes here. + */ + __set_bit(PHY_INTERFACE_MODE_MII, config->supported_interfaces); + __set_bit(PHY_INTERFACE_MODE_REVMII, config->supported_interfaces); + + config->mac_capabilities = MAC_ASYM_PAUSE | MAC_SYM_PAUSE | + MAC_10 | MAC_100; + + /* 5325/5365 are not capable of gigabit speeds, everything else is */ + if (!(is5325(dev) || is5365(dev))) + config->mac_capabilities |= MAC_1000; + + /* Get the implementation specific capabilities */ + if (dev->ops->phylink_get_caps) + dev->ops->phylink_get_caps(dev, port, config); +} + int b53_phylink_mac_link_state(struct dsa_switch *ds, int port, struct phylink_link_state *state) { @@ -2262,6 +2289,7 @@ static const struct dsa_switch_ops b53_switch_ops = { .phy_read = b53_phy_read16, .phy_write = b53_phy_write16, .adjust_link = b53_adjust_link, + .phylink_get_caps = b53_phylink_get_caps, .phylink_validate = b53_phylink_validate, .phylink_mac_link_state = b53_phylink_mac_link_state, .phylink_mac_config = b53_phylink_mac_config, diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h index b41dc8ac2ca8..b9d1b4819c5f 100644 --- a/drivers/net/dsa/b53/b53_priv.h +++ b/drivers/net/dsa/b53/b53_priv.h @@ -46,6 +46,8 @@ struct b53_io_ops { int (*phy_write16)(struct b53_device *dev, int addr, int reg, u16 value); int (*irq_enable)(struct b53_device *dev, int port); void (*irq_disable)(struct b53_device *dev, int port); + void (*phylink_get_caps)(struct b53_device *dev, int port, + struct phylink_config *config); u8 (*serdes_map_lane)(struct b53_device *dev, int port); int (*serdes_link_state)(struct b53_device *dev, int port, struct phylink_link_state *state); diff --git a/drivers/net/dsa/b53/b53_serdes.c b/drivers/net/dsa/b53/b53_serdes.c index 5ae3d9783b68..7e1ec51ab4c9 100644 --- a/drivers/net/dsa/b53/b53_serdes.c +++ b/drivers/net/dsa/b53/b53_serdes.c @@ -180,6 +180,33 @@ void b53_serdes_phylink_validate(struct b53_device *dev, int port, } EXPORT_SYMBOL(b53_serdes_phylink_validate); +void b53_serdes_phylink_get_caps(struct b53_device *dev, int port, + struct phylink_config *config) +{ + u8 lane = b53_serdes_map_lane(dev, port); + + if (lane == B53_INVALID_LANE) + return; + + switch (lane) { + case 0: + /* It appears lane 0 supports 2500base-X and 1000base-X */ + __set_bit(PHY_INTERFACE_MODE_2500BASEX, + config->supported_interfaces); + config->mac_capabilities |= MAC_2500FD; + fallthrough; + case 1: + /* It appears lane 1 only supports 1000base-X */ + __set_bit(PHY_INTERFACE_MODE_1000BASEX, + config->supported_interfaces); + config->mac_capabilities |= MAC_1000FD; + break; + default: + break; + } +} +EXPORT_SYMBOL(b53_serdes_phylink_get_caps); + int b53_serdes_init(struct b53_device *dev, int port) { u8 lane = b53_serdes_map_lane(dev, port); diff --git a/drivers/net/dsa/b53/b53_serdes.h b/drivers/net/dsa/b53/b53_serdes.h index 55d280fe38e4..8fa24f7001aa 100644 --- a/drivers/net/dsa/b53/b53_serdes.h +++ b/drivers/net/dsa/b53/b53_serdes.h @@ -115,6 +115,8 @@ void b53_serdes_config(struct b53_device *dev, int port, unsigned int mode, void b53_serdes_an_restart(struct b53_device *dev, int port); void b53_serdes_link_set(struct b53_device *dev, int port, unsigned int mode, phy_interface_t interface, bool link_up); +void b53_serdes_phylink_get_caps(struct b53_device *dev, int port, + struct phylink_config *config); void b53_serdes_phylink_validate(struct b53_device *dev, int port, unsigned long *supported, struct phylink_link_state *state); diff --git a/drivers/net/dsa/b53/b53_srab.c b/drivers/net/dsa/b53/b53_srab.c index 4591bb1c05d2..7d72f3b293d3 100644 --- a/drivers/net/dsa/b53/b53_srab.c +++ b/drivers/net/dsa/b53/b53_srab.c @@ -443,6 +443,39 @@ static void b53_srab_irq_disable(struct b53_device *dev, int port) } } +static void b53_srab_phylink_get_caps(struct b53_device *dev, int port, + struct phylink_config *config) +{ + struct b53_srab_priv *priv = dev->priv; + struct b53_srab_port_priv *p = &priv->port_intrs[port]; + + switch (p->mode) { + case PHY_INTERFACE_MODE_SGMII: +#if IS_ENABLED(CONFIG_B53_SERDES) + /* If p->mode indicates SGMII mode, that essentially means we + * are using a serdes. As the serdes for the capabilities. + */ + b53_serdes_phylink_get_caps(dev, port, config); +#endif + break; + + case PHY_INTERFACE_MODE_NA: + break; + + case PHY_INTERFACE_MODE_RGMII: + /* If we support RGMII, support all RGMII modes, since + * that dictates the PHY delay settings. + */ + phy_interface_set_rgmii(config->supported_interfaces); + break; + + default: + /* Some other mode (e.g. MII, GMII etc) */ + __set_bit(p->mode, config->supported_interfaces); + break; + } +} + static const struct b53_io_ops b53_srab_ops = { .read8 = b53_srab_read8, .read16 = b53_srab_read16, @@ -456,6 +489,7 @@ static const struct b53_io_ops b53_srab_ops = { .write64 = b53_srab_write64, .irq_enable = b53_srab_irq_enable, .irq_disable = b53_srab_irq_disable, + .phylink_get_caps = b53_srab_phylink_get_caps, #if IS_ENABLED(CONFIG_B53_SERDES) .serdes_map_lane = b53_srab_serdes_map_lane, .serdes_link_state = b53_serdes_link_state,
Populate the supported interfaces and MAC capabilities for the Broadcom B53 DSA switches in preparation to using these for the generic validation functionality. The interface modes are derived from: - b53_serdes_phylink_validate() - SRAB mux configuration NOTE: much of this conversion is a guess as the driver doesn't contain sufficient information. I would appreciate a thorough review and testing of this change before it is merged. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- drivers/net/dsa/b53/b53_common.c | 28 ++++++++++++++++++++++++++ drivers/net/dsa/b53/b53_priv.h | 2 ++ drivers/net/dsa/b53/b53_serdes.c | 27 +++++++++++++++++++++++++ drivers/net/dsa/b53/b53_serdes.h | 2 ++ drivers/net/dsa/b53/b53_srab.c | 34 ++++++++++++++++++++++++++++++++ 5 files changed, 93 insertions(+)