On Tue, May 24, 2022 at 03:08:07AM +0200, Andrew Lunn wrote:
> > @@ -976,6 +977,25 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
> > struct phy_driver *drv = phydev->drv;
> > irqreturn_t ret;
> >
> > + if (IS_ENABLED(CONFIG_PM_SLEEP) &&
> > + (phydev->mdio.dev.power.is_prepared ||
> > + phydev->mdio.dev.power.is_suspended)) {
> > + struct net_device *netdev = phydev->attached_dev;
> > +
> > + if (netdev) {
> > + struct device *parent = netdev->dev.parent;
> > +
> > + if (netdev->wol_enabled)
> > + pm_system_wakeup();
> > + else if (device_may_wakeup(&netdev->dev))
> > + pm_wakeup_dev_event(&netdev->dev, 0, true);
> > + else if (parent && device_may_wakeup(parent))
> > + pm_wakeup_dev_event(parent, 0, true);
> > + }
> > +
> > + return IRQ_HANDLED;
>
> I'm not sure you can just throw the interrupt away. There have been
> issues with WoL, where the WoL signal has been applied to a PMC, not
> an actual interrupt. Yet the PHY driver assumes it is an
> interrupt. And in order for WoL to work correctly, it needs the
> interrupt handler to be called. We said the hardware is broken, WoL
> cannot work for that setup.
>
> Here you have correct hardware, but you are throwing the interrupt
> away, which will have the same result. So i think you need to abort
> the suspend, get the bus working again, and call the interrupt
> handler. If this is a WoL interrupt you are supposed to be waking up
> anyway.
mdio_bus_phy_resume() does trigger the state machine via
phy_start_machine(), so link state changes *are* detected after wakeup.
But you're saying that's not sufficient and you really want the
PHY driver's IRQ handler to be called, do I understand that correctly?
That could be achieved with a flag indicating that the IRQ handler
needs to be rerun after resume. A simple invocation of irq_wake_thread()
will then achieve that.
It has also occurred to me that not clearing the IRQ may lead to
an interrupt storm if it's level-triggered. So I need to disable
the IRQ and re-enable it after the PHY has been resumed.
Back to the drawing board...
Thanks,
Lukas
