Re: [PATCH] ACPI: s2idle: Don't ignore error when enabling wakeup IRQ

[Simon Gaiser <simon@invisiblethingslab.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1961 bytes --]

Rafael J. Wysocki:
> On Mon, Mar 13, 2023 at 1:54 PM Simon Gaiser
> <simon@invisiblethingslab.com> wrote:
>>
>> enable_irq_wake() can fail. Previously acpi_s2idle_prepare() silently
>> ignored it's return code, potentially leaving a system that never
>> wakes up.
>>
>> Discovered when trying to go into s2idle under Xen. This leads to a
>> system that can't be woken, since xen-pirq currently doesn't support
>> setting wakeup IRQs. Real s2idle support for Xen is another topic,
>> but now at least the user gets an error and the system doesn't needs
>> an hard reset.
>>
>> Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
>> ---
>>
>> Note that I'm unfamiliar with the code so when reviewing please
>> carefully check if ignoring the error was indeed unintended.
> 
> No, it wasn't.
> 
> First, in the majority of cases in which ACPI SCI is used, the IRQ
> chip in question has IRQCHIP_SKIP_SET_WAKE set, so enable_irq_wake()
> cannot fail for it.
> 
> Second, even if it could fail, it is preferred to let the system
> suspend anyway, as long as there is at least one other wakeup source
> in it and that is the case as a rule (for example, wakeup is enabled
> for the PS/2 keyboard IRQ for all systems that have it).
> 
>> If there are indeed cases where the error should be ignored I would
>> submit a patch that at least logs the error, although the error
>> message would be hard to see with broken wakeup.
> 
> Logging an error would be fine, but failing suspend may not be.  Of
> course, suspend should be aborted if there are no other usable (and
> enabled) wakeup sources, but currently there's no infrastructure for
> verifying that.

Ah, ok, that's a bit unfortunate. Anyway sent a v2 that only logs. That
way it should at least make debugging for someone else that runs into
such a case a bit easier.

https://lore.kernel.org/linux-acpi/20230313144710.3516-1-simon@invisiblethingslab.com/

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]