Re: [PATCH net V2 1/1] net: phy: fix invalid phy id when probe using C22

On 3/18/2021 11:14 AM, Greg KH wrote:

On Thu, Mar 18, 2021 at 09:02:22AM -0700, Florian Fainelli wrote:

quoted

On 3/18/2021 6:25 AM, Heiner Kallweit wrote:

quoted

On 18.03.2021 10:09, Wong Vee Khee wrote:

quoted

When using Clause-22 to probe for PHY devices such as the Marvell
88E2110, PHY ID with value 0 is read from the MII PHYID registers
which caused the PHY framework failed to attach the Marvell PHY
driver.

Fixed this by adding a check of PHY ID equals to all zeroes.

I was wondering whether we have, and may break, use cases where a PHY,
for whatever reason, reports PHY ID 0, but works with the genphy
driver. And indeed in swphy_read_reg() we return PHY ID 0, therefore
the patch may break the fixed phy.
Having said that I think your patch is ok, but we need a change of
the PHY ID reported by swphy_read_reg() first.
At a first glance changing the PHY ID to 0x00000001 in swphy_read_reg()
should be sufficient. This value shouldn't collide with any real world
PHY ID.

It most likely would not, but it could be considered an ABI breakage,
unless we filter out what we report to user-space via SIOGCMIIREG and
/sys/class/mdio_bus/*/*/phy_id

Ideally we would have assigned an unique PHY OUI to the fixed PHY but
that would have required registering Linux as a vendor, and the process
is not entirely clear to me about how to go about doing that.

If you need me to do that under the umbrella of the Linux Foundation,
I'll be glad to do so if you point me at the proper group to do that
with.

We did this for a few years with the USB-IF and have a vendor id
assigned to us for Linux through them, until they kicked us out because.
But as the number is in a global namespace, it can't be reused so we
keep it :)

We would still be creating what is technically an user-space interface
breakage since prior to a given kernel version we would return 0 for
that PHY OUI, and after another point it would be something different. I
don't know what software out there may be expecting to find 0 and not
determine that the PHY was fixed already because it is under
/sys/class/mdio_bus/fixed-0
-- 
Florian