Re: [PATCH net v2 0/1] PHY interruptus horribilis

[Rafael J. Wysocki]

On Wed, Jun 8, 2022 at 11:52 AM Lukas Wunner <lukas@xxxxxxxxx> wrote:
>
> Andrew Lunn (PHY maintainer) asked me to resend this patch and cc the
> IRQ maintainer.  I'm also cc'ing PM maintainers for good measure.
>
> The patch addresses an issue with PHY interrupts occurring during a
> system sleep transition after the PHY has already been suspended.
>
> The IRQ subsystem uses an internal flag IRQD_WAKEUP_ARMED to avoid
> handling such interrupts, but it's not set until suspend_device_irqs()
> is called during the ->suspend_noirq() phase.  That's too late in this
> case as PHYs are suspended in the ->suspend() phase.  And there's
> no external interface to set the flag earlier.

Yes, it is not there intentionally.

Strictly speaking, IRQD_WAKEUP_ARMED is there to indicate to the IRQ
subsystem that the given IRQ is a system wakeup one and has been left
enabled specifically in order to signal system wakeup.  It allows the
IRQ to trigger between suspend_device_irqs() and resume_device_irqs()
exactly once, which causes the system to wake up from suspend-to-idle
(that's the primary use case for it) or aborts system suspends in
progress.

As you have noticed, it is set automatically by suspend_device_irqs()
if the given IRQ has IRQD_WAKEUP_STATE which is the case when it has
been enabled for system wakeup.

> As I'm lacking access to the flag, I'm open coding its functionality
> in this patch.  Is this the correct approach or should I instead look
> into providing an external interface to the flag?

The idea is that the regular IRQ "action" handler will run before
suspend_device_irqs(), so it should take care of signaling wakeup if
need be.  IRQD_WAKEUP_ARMED to trigger wakeup when IRQ "action"
handlers don't run.

> Side note: suspend_device_irqs() and resume_device_irqs() have been
> exported since forever even though there's no module user...

Well, that's a mistake.