[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8eb06ee9-d02d-4113-ba1e-e8ee99acc2fd@gmail.com>
Date: Tue, 2 Jan 2024 15:06:54 +0100
From: Heiner Kallweit <hkallweit1@...il.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: ezra@...ergy-village.org, Russell King <linux@...linux.org.uk>,
Tristram Ha <Tristram.Ha@...rochip.com>, Michael Walle <michael@...le.cc>,
Jesse Brandeburg <jesse.brandeburg@...el.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net] net: mdio: Prevent Clause 45 scan on SMSC PHYs
On 02.01.2024 14:42, Andrew Lunn wrote:
> On Mon, Jan 01, 2024 at 11:44:38PM +0100, Heiner Kallweit wrote:
>> On 01.01.2024 22:31, Ezra Buehler wrote:
>>> Since commit 1a136ca2e089 ("net: mdio: scan bus based on bus
>>> capabilities for C22 and C45") our AT91SAM9G25-based GARDENA smart
>>> Gateway will no longer boot.
>>>
>>> Prior to the mentioned change, probe_capabilities would be set to
>>> MDIOBUS_NO_CAP (0) and therefore, no Clause 45 scan was performed.
>>> Running a Clause 45 scan on an SMSC/Microchip LAN8720A PHY will (at
>>> least with our setup) considerably slow down kernel startup and
>>> ultimately result in a board reset.
>>>
>>> AFAICT all SMSC/Microchip PHYs are Clause 22 devices. Some have a
>>> "Clause 45 protection" feature (e.g. LAN8830) and others like the
>>> LAN8804 will explicitly state the following in the datasheet:
>>>
>>> This device may respond to Clause 45 accesses and so must not be
>>> mixed with Clause 45 devices on the same MDIO bus.
>>>
>>
>> I'm not convinced that some heuristic based on vendors is a
>> sustainable approach. Also I'd like to avoid (as far as possible)
>> that core code includes vendor driver headers. Maybe we could use
>> a new PHY driver flag. Approaches I could think of:
>
> We already have a core hack for these broken PHYs:
>
Excluding all PHY's from a vendor for me is a quite big hammer.
I think we should make this more granular.
And mdio-bus.c including micrel_phy.h also isn't too nice.
Maybe we should move all OUI definitions in drivers to a
core header. Because the OUI seems to be all we need from
these headers.
> /*
> * There are some C22 PHYs which do bad things when where is a C45
> * transaction on the bus, like accepting a read themselves, and
> * stomping over the true devices reply, to performing a write to
> * themselves which was intended for another device. Now that C22
> * devices have been found, see if any of them are bad for C45, and if we
> * should skip the C45 scan.
> */
> static bool mdiobus_prevent_c45_scan(struct mii_bus *bus)
> {
> int i;
>
> for (i = 0; i < PHY_MAX_ADDR; i++) {
> struct phy_device *phydev;
> u32 oui;
>
> phydev = mdiobus_get_phy(bus, i);
> if (!phydev)
> continue;
> oui = phydev->phy_id >> 10;
>
> if (oui == MICREL_OUI)
> return true;
> }
> return false;
> }
>
> So it seems we need to extend this with another OUI.
>
> Andrew
Powered by blists - more mailing lists