Messages in this thread Patch in this message | | | Date | Tue, 20 Dec 2022 22:39:15 +0100 | From | Michael Walle <> | Subject | Re: [PATCH net-next v1 4/4] net: phy: mxl-gpy: disable interrupts on GPY215 by default |
| |
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
| |