Re: [PATCH net-next v1 4/4] net: phy: mxl-gpy: disable interrupts on GPY215 by default

[Michael Walle]

Am 2022-12-20 14:33, schrieb Andrew Lunn:
> > > > > I think a better place for this test is in gpy_config_intr(), return
> > > > > -EOPNOTSUPP. phy_enable_interrupts() failing should then cause
> > > > > phy_request_interrupt() to use polling.
> > > >
> > > > Which will then print a warning, which might be misleading.
> > > > Or we disable the warning if -EOPNOTSUPP is returned?
> > >
> > > Disabling the warning is the right thing to do.
> >
> > There is more to this. .config_intr() is also used in
> > phy_init_hw() and phy_drv_supports_irq(). The latter would
> > still return true in our case. I'm not sure that is correct.
> >
> > After trying your suggestion, I'm still in favor of somehow
> > tell the phy core to force polling mode during probe() of the
> > driver.
>
> The problem is that the MAC can set the interrupt number after the PHY
> probe has been called. e.g.
>
> https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c#L524
>
> The interrupt needs to be set by the time the PHY is connected to the
> MAC, which is often in the MAC open method, much later than the PHY
> probe.

Ok, then phydev->irq should be updated within the phy_attach_direct().
Something like the following:

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e865be3d7f01..c6c5830f5214 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1537,6 +1537,14 @@ int phy_attach_direct(struct net_device *dev, 
struct phy_device *phydev,

        phydev->interrupts = PHY_INTERRUPT_DISABLED;

+       /* PHYs can request to use poll mode even though they have an
+        * associated interrupt line. This could be the case if they
+        * detect a broken interrupt handling.
+        */
+       if (phydev->drv->force_polling_mode &&
+           phydev->drv->force_polling_mode(phydev))
+               phydev->irq = PHY_POLL;
+
        /* Port is set to PORT_TP by default and the actual PHY driver 
will set
         * it to different value depending on the PHY configuration. If 
we have
         * the generic PHY driver we can't figure it out, thus set the 
old

That callback could be too specifc, I don't know. We could also have
phydev->drv->pre_attach() which then can update the phydev->irq itself.

Btw. the phy_attached_info() in the stmmac seems to be a leftover
from before the phylink conversion. phylink will print a similar info
but when the PHY is actually attached.

-michael