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. https://lore.kernel.org/linux-acpi/20230313144710.3516-1-simon@xxxxxxxxxxxxxxxxxxxxxx/
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
