| Message ID | 20210611071527.9333-1-o.rempel@pengutronix.de |
|---|---|
| Headers | show |
| Series | provide cable test support for the ksz886x switch | expand |
On Fri, Jun 11, 2021 at 09:15:20AM +0200, Oleksij Rempel wrote: > From: Michael Grzeschik <m.grzeschik@pengutronix.de> > > This patch adds the phylink support to the ksz8795 driver to provide > configuration exceptions on quirky KSZ8863 and KSZ8873 ports. > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- Reviewed-by: Vladimir Oltean <olteanv@gmail.com> but it would be a good idea for Russell to take a look too.
On 6/11/2021 12:15 AM, Oleksij Rempel wrote: > From: Michael Grzeschik <m.grzeschik@pengutronix.de> > > This patch adds the phylink support to the ksz8795 driver to provide > configuration exceptions on quirky KSZ8863 and KSZ8873 ports. > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
On Fri, Jun 11, 2021 at 09:15:23AM +0200, Oleksij Rempel wrote: > Add support for MDI-X status and configuration > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Fri, Jun 11, 2021 at 09:15:24AM +0200, Oleksij Rempel wrote: > Add support for MDI-X status and configuration > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
> +static int ksz886x_cable_test_get_status(struct phy_device *phydev, > + bool *finished) > +{ > + unsigned long pair_mask = 0x3; > + int retries = 20; > + int pair, ret; > + > + *finished = false; > + > + /* Try harder if link partner is active */ > + while (pair_mask && retries--) { > + for_each_set_bit(pair, &pair_mask, 4) { > + ret = ksz886x_cable_test_one_pair(phydev, pair); > + if (ret == -EAGAIN) > + continue; > + if (ret < 0) > + return ret; > + clear_bit(pair, &pair_mask); > + } > + /* If link partner is in autonegotiation mode it will send 2ms > + * of FLPs with at least 6ms of silence. > + * Add 2ms sleep to have better chances to hit this silence. > + */ > + if (pair_mask) > + msleep(2); > + } > + > + *finished = true; > + > + return 0; If ksz886x_cable_test_one_pair() returns -EAGAIN 20x and it gives up, you end up returning 0. Maybe it would be better to return ret? Andrew
On 6/11/2021 12:15 AM, Oleksij Rempel wrote: > Add support for MDI-X status and configuration > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian
On 6/11/2021 12:15 AM, Oleksij Rempel wrote: > Add support for MDI-X status and configuration > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian
On Fri, Jun 11, 2021 at 10:24:17PM +0300, Vladimir Oltean wrote: > On Fri, Jun 11, 2021 at 09:15:26AM +0200, Oleksij Rempel wrote: > > This patch extends the flags of the phy that's being connected with the > > port specific flags of the switch port. > > > > This is needed to handle a port specific erratum of the KSZ8873 switch, > > which is added in a later patch. > > > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > > --- > > What happens differently between having this patch and not having it? Without this patch, the PHY driver will not get the phyflag provided by the DSA driver. Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Fri, Jun 11, 2021 at 04:26:33PM -0700, Florian Fainelli wrote: > > > On 6/11/2021 12:24 PM, Vladimir Oltean wrote: > > On Fri, Jun 11, 2021 at 09:15:26AM +0200, Oleksij Rempel wrote: > >> This patch extends the flags of the phy that's being connected with the > >> port specific flags of the switch port. > >> > >> This is needed to handle a port specific erratum of the KSZ8873 switch, > >> which is added in a later patch. > >> > >> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > >> --- > > > > What happens differently between having this patch and not having it? > > The current get_phy_flags() is only processed when we connect to a PHY > via a designed phy-handle property via phylink_of_phy_connect((, but if > we fallback on the internal MDIO bus created by a switch and take the > dsa_slave_phy_connect() path then we would not be processing that flag > and using it at PHY connection time. Oleksij, your proposed patch fails > to check that dsa_switch_ops::get_phy_flags is actually non-NULL, how > about this approach instead where we only fetch the flags once, and we > deal with an option get_phy_flags callback too: ok, sounds good. Thank you. > diff --git a/net/dsa/slave.c b/net/dsa/slave.c > index d4756b920108..ba7866ec946f 100644 > --- a/net/dsa/slave.c > +++ b/net/dsa/slave.c > @@ -1749,7 +1749,7 @@ static void dsa_slave_phylink_fixed_state(struct > phylink_config *config, > } > > /* slave device setup > *******************************************************/ > -static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr) > +static int dsa_slave_phy_connect(struct net_device *slave_dev, int > addr, u32 flags) > { > struct dsa_port *dp = dsa_slave_to_port(slave_dev); > struct dsa_switch *ds = dp->ds; > @@ -1760,6 +1760,8 @@ static int dsa_slave_phy_connect(struct net_device > *slave_dev, int addr) > return -ENODEV; > } > > + slave_dev->phydev->dev_flags |= flags; > + > return phylink_connect_phy(dp->pl, slave_dev->phydev); > } > > @@ -1804,7 +1806,7 @@ static int dsa_slave_phy_setup(struct net_device > *slave_dev) > /* We could not connect to a designated PHY or SFP, so > try to > * use the switch internal MDIO bus instead > */ > - ret = dsa_slave_phy_connect(slave_dev, dp->index); > + ret = dsa_slave_phy_connect(slave_dev, dp->index, > phy_flags); > if (ret) { > netdev_err(slave_dev, > "failed to connect to port %d: %d\n", > -- > Florian > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Sat, Jun 12, 2021 at 08:23:53PM +0200, Andrew Lunn wrote: > > +static int ksz886x_cable_test_get_status(struct phy_device *phydev, > > + bool *finished) > > +{ > > + unsigned long pair_mask = 0x3; > > + int retries = 20; > > + int pair, ret; > > + > > + *finished = false; > > + > > + /* Try harder if link partner is active */ > > + while (pair_mask && retries--) { > > + for_each_set_bit(pair, &pair_mask, 4) { > > + ret = ksz886x_cable_test_one_pair(phydev, pair); > > + if (ret == -EAGAIN) > > + continue; > > + if (ret < 0) > > + return ret; > > + clear_bit(pair, &pair_mask); > > + } > > + /* If link partner is in autonegotiation mode it will send 2ms > > + * of FLPs with at least 6ms of silence. > > + * Add 2ms sleep to have better chances to hit this silence. > > + */ > > + if (pair_mask) > > + msleep(2); > > + } > > + > > + *finished = true; > > + > > + return 0; > > If ksz886x_cable_test_one_pair() returns -EAGAIN 20x and it gives up, > you end up returning 0. Maybe it would be better to return ret? Good point. Fixed. Regards, Oleksij -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
changes v4: - use fallthrough; - use EOPNOTSUPP instead of ENOTSUPP - drop flags variable in dsa_slave_phy_connect patch - extend description for the "net: phy: micrel: apply resume errat" patch - fix "use consistent alignments" patch changes v3: - remove RFC tag changes v2: - use generic MII_* defines where possible - rework phylink validate - remove phylink get state function - reorder cabletest patches to make PHY flag patch in the right order - fix MDI-X detection This patches provide support for cable testing on the ksz886x switches. Since it has one special port, we needed to add phylink with validation and extra quirk for the PHY to signal, that one port will not provide valid cable testing reports. Michael Grzeschik (2): net: phy: micrel: move phy reg offsets to common header net: dsa: microchip: ksz8795: add phylink support Oleksij Rempel (7): net: phy: micrel: use consistent alignments net: phy: micrel: apply resume errata workaround for ksz8873 and ksz8863 net: phy/dsa micrel/ksz886x add MDI-X support net: phy: micrel: ksz8081 add MDI-X support net: dsa: microchip: ksz8795: add LINK_MD register support net: dsa: dsa_slave_phy_connect(): extend phy's flags with port specific phy flags net: phy: micrel: ksz886x/ksz8081: add cabletest support drivers/net/dsa/microchip/ksz8795.c | 214 ++++++++---- drivers/net/dsa/microchip/ksz8795_reg.h | 67 +--- drivers/net/ethernet/micrel/ksz884x.c | 105 +----- drivers/net/phy/micrel.c | 423 ++++++++++++++++++++++-- include/linux/micrel_phy.h | 16 + net/dsa/slave.c | 4 + 6 files changed, 593 insertions(+), 236 deletions(-)