Re: [PATCH v2 10/19] net: phy: Use the compatibility driver attachment API

[Leon Romanovsky]

On Tue, Nov 10, 2026 at 03:31:15PM +0100, Andrew Lunn wrote:
> On Tue, Nov 10, 2026 at 09:39:19AM +0200, Leon Romanovsky wrote:
> > On Mon, Nov 09, 2026 at 02:45:35PM +0100, Andrew Lunn wrote:
> > > > @@ -1956,11 +1958,10 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
> > > >  	}
> > > >  	phydev->drv_owner = drv_owner;
> > > >  
> > > > -	if (phydev->is_genphy_driven) {
> > > > -		err = d->driver->probe(d);
> > > > -		if (err >= 0)
> > > > -			err = device_bind_driver(d);
> > > > -
> > > > +	if (bind_genphy) {
> > > > +		err = device_driver_attach_full(drv, d);
> > > > +		if (err == -EBUSY)
> > > > +			irq = phydev->irq;
> > > 
> > > Just looking at this, it is not clear where the EBUSY can from? 
> > 
> > I relied on the comment above: “Assume that if there is no driver, ...”.
> > Since PHYs do not use the standard automatic probing mechanism, I have no
> > way to tell whether a driver is already bound.
> 
> They do use the automatic probe mechanism. But also extend it.
> 
> Maybe some background will help.
> 
> IEEE 802.3 specifies how PHYs should operate, it list all the
> registers which are needed for basic operation etc. If a PHY follows
> the standard, genphy is sufficient to drive the PHY.
> 
> Now. Networking vendors like to do two things:
> 
> 1) Do more than what the standard specifies, like interrupts and
> blinking LEDs.
> 
> 2) Don't closely read the standard, and break their implementation
> 
> These two cases require a specific driver.
> 
> There is a lot of history to phylib, from before my time. It seems
> like the current model was retrofitted around earlier code. The result
> is that when the MDIO bus is enumerated, PHYs can be found on the bus
> and are registered to the bus in the normal way. The driver core will
> look to see if there is a driver which matches the ID values in
> registers 2 and 3, and if so probe that driver. If there is no
> matching driver, a call is made to user space to ask it to load the
> driver with the given IDs.  That operates asynchronous, when such a
> driver is loaded, the driver core will run the match again and probe
> the device with the driver.
> 
> Because there can be up to 32 devices on the MDIO bus, the MAC driver
> often knows which PHY device it should use. If not, MAC drivers
> sometimes use the first PHY on the bus. It does not really matter, the
> point is, we know the device exists, even if it has not been probed
> yet, so we can get a reference to it.
> 
> The MAC most often connects to its PHY during open(). At that point,
> if the driver core has bound a PHY driver to the PHY device, it is
> good to go.
> 
> If however, there is no PHY driver for the PHY device, it can mean a
> few different things:
> 
> There is no driver of that PHY because it follows 802.3, and genphy is
> expected to be used. However, genphy does not have a list of IDs it
> can drive, so the usual probing mechanism does not match it to the
> device. It would be nice if it did actually have such a list, but it
> does not.
> 
> There is driver, but it has not been built, or is missing from the
> initramfs, or is not part of the image, etc. So we just use the genphy
> driver and hope for the best.
> 
> So, if we have got this far, there is no driver for the PHY, we make
> use of genphy.
> 
> > > And why set irq to phydev->irq?
> > 
> > If the above operation can fail with -EBUSY, the wrong IRQ was restored.
> 
> How has it got the wrong interrupt?
> 
> genphy cannot use interrupts, because interrupts are not part of
> 802.3. Device tree can list an interrupt for the device, and if the
> device specific driver is used, phylib can use that
> interrupt. However, since interrupts are optional, phylib defaults to
> polling.  phydev->irq is over powered and set to PHY_POLL to indicate
> polling should be used. The phylib core will look at the PHY driver,
> see if it supports interrupts, and if not, set phydev->irq ==
> PHY_POLL.
> 
> The binding of genphy to the device cannot fail, at least in the
> current phylib code. So up until recently, setting phydev->irq to
> PHY_POLL was fine. However, that is changing. There is a use case,
> because of bad hardware design, that genphy is initially used to get
> the device up. But later, the real driver module becomes available and
> gets loaded. /sysfs is then used to unbind genphy and bind the
> specific driver. Because phylib has set phydev->irq to PHY_POLL, the
> specific driver cannot use interrupts. So on unbind, phydev->irq is
> set back to the value in the MDIO bus.

Thanks for the explanation, I definitely lack knowledge in phylib
history.

> 
> As is said, this code is fragile, and complex. You have to assume your
> LLM is producing slop, and really take a close look at the code, if
> you want to get it correct.

The problem is that I also assume that the code preceding the change is wrong
too. People often produce code that's even worse than what a properly guided
LLM produces.

Thanks

> 
>     Andrew