[PATCH net v11 3/4] net: phy: take the interrupt back from the bus on detach

[Aleksei Sviridkin]

A PHY whose own driver is a module on a filesystem that is not mounted
when the MAC probes gets the generic driver first. phy_probe() replaces
phydev->irq with PHY_POLL because that driver has no interrupt support,
nothing puts it back, and the PHY polls for the rest of the uptime once
its real driver takes over. That is where a Keenetic KN-1012 stands,
with an Airoha EN8811H behind an MT7531 port and its driver on the root
filesystem: the devicetree interrupt of the PHY maps to irq 15, and once
the real driver has taken over the field reads -1.

Take the number back in phy_detach(), from mdiobus->irq[], which is
where phy_device_create() seeded phydev->irq from and where the bus that
described the interrupt still holds it. Only under is_genphy_driven,
since that is the substitution being undone: elsewhere the field belongs
to whoever wrote it, a MAC installing PHY_MAC_INTERRUPT writes
phydev->irq alone, and a phy_request_interrupt() that failed leaves
PHY_POLL there while the bus table still holds the number that could not
be requested. Under the generic driver a PHY_MAC_INTERRUPT written into
phydev->irq alone is still replaced; bcmasp, genet and tsnep, the MACs
that write it that way, write it again after each connect.

Do it before device_release_driver() rather than after. That call
returns with the mdio device bindable and the device lock dropped, so
from then on a phy_probe() on another CPU is the other writer of this
field; before it, the generic driver is bound and the driver core turns
such a probe away with -EBUSY. That is ordering, not exclusion: nothing
on this side holds the device lock. Nor does the ordering hold after a
sysfs unbind of the generic driver under a consumer, which leaves
is_genphy_driven set with no driver bound, so the store can race or
follow the probe of a driver that binds meanwhile. The next
phy_attach_direct() applies the substitution again for whichever driver
it finds bound, so the number its callers request still matches that
driver.

Fixes: 00db8189d984 ("This patch adds a PHY Abstraction Layer to the Linux Kernel, enabling ethernet drivers to remain as ignorant as is reasonable of the connected PHY's design and operation details.")
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@xxxxxx>
---

Notes:
    Found and measured on a Keenetic KN-1012 (MT7981B, MT7531 switch) with an
    Airoha EN8811H behind lan4, whose interrupt the devicetree describes and
    whose driver is a module.
    
    The one condition arranged for the run is that the PHY driver module loads
    after the root filesystem rather than from the early boot list this
    distribution normally puts it in. The distribution's own late-PHY handling
    was also removed, that being the one patch which could have changed the
    outcome; upstream has nothing like it. The kernel is still a distribution
    one and its remaining patches to phylink and phy_device do run on these
    paths - none of them writes phydev->irq.
    
    DSA then sets the port up at 1.87 s, the generic driver is bound by hand,
    phy_probe() replaces the interrupt with PHY_POLL, and phylink rejects
    2500base-x against it:
    
      lan4 (uninitialized): validation of 2500base-x ... failed: -EINVAL
      lan4 (uninitialized): failed to connect to PHY: -EINVAL
    
    The real driver arrives between 13.4 and 13.6 s depending on the boot, and
    binds. phydev->irq then reads -1 without this patch and 15 with it, 15
    being what the devicetree gave that PHY. The three switch ports alongside
    read 79, 80 and 81 in both runs, so the reading distinguishes rather than
    printing one answer. The field has no sysfs attribute of its own, so it was
    read with a debug-only module parameter that walks the MDIO bus and prints
    it.
    
    The reading predates the is_genphy_driven guard the store now sits
    under, which should not be implied away. On the measured path the flag
    is set: phylink_fwnode_phy_connect(), which DSA reaches through
    phylink_of_phy_connect() for this port, calls phy_detach() from its own
    failure check, and the three writes of that flag are all in
    phy_device.c, none of them between the hand-bind and that call. So the
    guard passes and the store is the one the reading came from. Its other
    side, a detach with a real driver bound, was not measured.

 drivers/net/phy/phy_device.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 94b2e85e00a3..a9c71a286118 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1965,6 +1965,8 @@ void phy_detach(struct phy_device *phydev)
 	 * real driver could be loaded
 	 */
 	if (phydev->is_genphy_driven) {
+		/* The release below lets phy_probe() write this field. */
+		phydev->irq = phydev->mdio.bus->irq[phydev->mdio.addr];
 		device_release_driver(&phydev->mdio.dev);
 		phydev->is_genphy_driven = 0;
 	}
-- 
2.53.0