[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200714072501.GA5072@pengutronix.de>
Date: Tue, 14 Jul 2020 09:25:01 +0200
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: Andrew Lunn <andrew@...n.ch>
Cc: Florian Fainelli <f.fainelli@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>,
"David S. Miller" <davem@...emloft.net>, kernel@...gutronix.de,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
Philippe Schenker <philippe.schenker@...adex.com>
Subject: Re: [PATCH net-next v1 5/5] net: phy: micrel: ksz886x/ksz8081: add
cabletest support
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 |
Powered by blists - more mailing lists