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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


Rafael J. Wysocki:
> On Mon, Mar 13, 2023 at 1:54 PM Simon Gaiser
> <simon@xxxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxx>
>> ---
>> 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.

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux