Re: [PATCH net-next v1 5/5] net: phy: micrel: ksz886x/ksz8081: add cabletest support

[Oleksij Rempel <o.rempel@pengutronix.de>]

On Mon, Jul 13, 2020 at 05:17:19PM +0200, Andrew Lunn wrote:
> > > Hi Oleksij
> > > 
> > > Do the PHY register read/writes pass through the DSA driver for the
> > > 8873?  I was wondering if the switch could intercept reads/writes on
> > > port1 for KSZ8081_LMD and return EOPNOTSUPP? That would be a more
> > > robust solution than DT properties, which are going to get forgotten.
> > 
> > Yes, it was my first idea as well. But this switch allows direct MDIO
> > access to the PHYs and this PHY driver could be used without DSA driver.
> > Not sure if we should support both variants?
> > 
> > Beside, the Port 1 need at least one more quirk. The pause souport is
> > announced but is not working. Should we some how clear Puase bit in the PHY and
> > tell PHY framework to not use it? What is the best way to do it?
> 
> It all seems rather odd, the way one PHY is messed up, but the other
> works. Does this PHY exist as a standalone device, not integrated into
> the switch? Do the same erratas apply to such a standalone device?

I found multiple microchip devices with same PHYid: KSZ8463, KSZ8851
KSZ8463 - switch. Would be covered by DSA driver
KSZ8851 - single MAC device with PHY. Supported by ethernet/micrel
driver.

This erratum is not documented for other devices. So it may exist or
not.

> If the issues are just limited to integrated PHYs, there is maybe
> something you can do via DSA:
> 
> in slave.c:
> 
> static int dsa_slave_phy_setup(struct net_device *slave_dev)
> {
> ...
>         if (ds->ops->get_phy_flags)
>                 phy_flags = ds->ops->get_phy_flags(ds, dp->index);
> 
>         ret = phylink_of_phy_connect(dp->pl, port_dn, phy_flags);
> 
> It is either B53 or SF2 which uses this, i forget which. flags get
> or'ed into phydev->dev_flags. These are device specific flags. So you
> could define a bit to represent this errata. And then in the PHY
> driver do whatever needs to be done when you see the flag set for a
> specific PHY.
> 
> If Pause is broken, then yes, it would be good to remove the Pause
> from the available features, and return an error if requested to use
> it.

OK. So, i'll cover both errata with separate flags? Set flags in the DSA
driver and apply workarounds in the PHY. ACK?

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 |