On Tue, Aug 30, 2022 at 8:14 AM Marek Maślanka <mm@xxxxxxxxxxxx> wrote: > > On 29.08.2022 16:12, Rafael J. Wysocki wrote: > > On Mon, Aug 29, 2022 at 12:21 PM Marek Maślanka <mm@xxxxxxxxxxxx> wrote: > >> > >> The GPE interrupts that are the wake-up source are "turned off" by clear > >> the “enable_for_wake” flag when the kernel resumes (suspend_enter() -> > >> acpi_s2idle_restore() -> acpi_disable_wakeup_devices() -> > >> acpi_set_gpe_wake_mask()). On the resume path the kernel also resumes > >> the interrupts (suspend_enter() -> dpm_resume_noirq() -> resume_irqs()) > >> which process the GPE interrupt that woke-up the kernel (... -> > >> acpi_irq() -> acpi_ev_sci_xrupt_handler() -> acpi_ev_gpe_detect() -> …). > >> The GPE interrupt routine stops in the acpi_ev_gpe_detect () function > >> when the "enable_for_wake" flag is cleared. > >> > >> As the interrupt servicing might work simultaneously on SMP, it’s > >> possible that the “enable_for_wake” flag can be cleared before the GPE > >> interrupt gets chances to be processed. It might happen when the CPU > >> processed other IRQ before the GPE IRQ that woke up the device. > >> > >> This issue is seen on low-end Chromebooks with two cores CPU when HPET > >> IRQ is triggered while resuming the device and is processed before the > >> ACPI GPE interrupt on the same CPU core. > >> > >> Before clear the enable_for_wake flag we need to make sure that the > >> specific wake-up GPE IRQ block was processed. > >> > >> Signed-off-by: Marek Maslanka <mm@xxxxxxxxxxxx> > > > > First off, if you want to modify ACPICA code, the way to do that is > > via the upstream ACPICA project on GitHub. > > > > Second, I'm not sure what the problem is. > > > > Yes, acpi_ev_gpe_detect() will bail out early when none of the GPEs in > > the given block is enabled either for wake or for run, but since the > > system has been woken up already and the GPE is now disabled, it will > > not trigger again until enabled next time. > > > > Is the problem that the GPE will signal wakeup spuriously on the next > > suspend or is it something else? > > In my cases the problem is the GPE STS flag that cannot be cleared > (acpi_ev_gpe_detect() -> acpi_ev_detect_gpe() -> acpi_ev_gpe_dispatch() > -> acpi_hw_clear_gpe()). If the status bit is not cleared before the > next suspend, the device will not wake up. Interesting. Have you considered modifying acpi_set_gpe_wake_mask() to clear the GPE status after clearing the bit in the enable_for_wake mask if the corresponding bit in enable_for_run is also unset?
