Interesting... The patch series is here: https://patchwork.kernel.org/project/linux-input/cover/20220830231541.1135813-1-rrangel@xxxxxxxxxxxx/ I'll look into why you only got added to 2 of the emails. On Wed, Aug 31, 2022 at 5:52 AM Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > On Tue, Aug 30, 2022 at 05:15:33PM -0600, Raul E Rangel wrote: > > Today, i2c drivers are making the assumption that their IRQs can also > > be used as wake IRQs. This isn't always the case and it can lead to > > spurious wakes. This has recently started to affect AMD Chromebooks. > > With the introduction of > > d62bd5ce12d7 ("pinctrl: amd: Implement irq_set_wake"), the AMD GPIO > > controller gained the capability to set the wake bit on each GPIO. The > > ACPI specification defines two ways to inform the system if a device is > > wake capable: > > 1) The _PRW object defines the GPE that can be used to wake the system. > > 2) Setting ExclusiveAndWake or SharedAndWake in the _CRS GpioInt. > > > > Currently only the first method is supported. The i2c drivers don't have > > any indication that the IRQ is wake capable, so they guess. This causes > > spurious interrupts, for example: > > * We have an ACPI HID device that has `_PR0` and `_PR3`. It doesn't have > > `_PRW` or `ExclusiveAndWake` so that means the device can't wake the > > system. > > * The IRQ line is active level low for this device and is pulled up by > > the power resource defined in `_PR0`/`_PR3`. > > * The i2c driver will (incorrectly) arm the GPIO for wake by calling > > `enable_irq_wake` as part of its suspend hook. > > * ACPI will power down the device since it doesn't have a wake GPE > > associated with it. > > * When the device is powered down, the IRQ line will drop, and it will > > trigger a wake event. > > > > See the following debug log: > > [ 42.335804] PM: Suspending system (s2idle) > > [ 42.340186] amd_gpio AMD0030:00: RX: Setting wake for pin 89 to enable > > [ 42.467736] power-0416 __acpi_power_off : Power resource [PR00] turned off > > [ 42.467739] device_pm-0280 device_set_power : Device [H05D] transitioned to D3cold > > [ 42.475210] PM: pm_system_irq_wakeup: 11 triggered pinctrl_amd > > [ 42.535293] PM: Wakeup unrelated to ACPI SCI > > [ 42.535294] PM: resume from suspend-to-idle > > > > In order to fix this, we need to take into account the wake capable bit > > defined on the GpioInt. This is accomplished by: > > * Migrating some of the i2c drivers over to using the PM subsystem to > > manage the wake IRQ. max8925-i2c, elants_i2c, and raydium_i2c_ts still > > need to be migrated, I can do that depending on the feedback to this > > patch series. > > * Expose the wake_capable bit from the ACPI GpioInt resource to the > > i2c core. > > * Use the wake_capable bit in the i2c core to call > > `dev_pm_set_wake_irq`. This reuses the existing device tree flow. > > * Make the i2c drivers stop calling `dev_pm_set_wake_irq` since it's now > > handled by the i2c core. > > * Make the ACPI device PM system aware of the wake_irq. This is > > necessary so the device doesn't incorrectly get powered down when a > > wake_irq is enabled. > > > > I've tested this code with various combinations of having _PRW, > > ExclusiveAndWake and power resources all defined or not defined, but it > > would be great if others could test this out on their hardware. > > I have got only cover letter and a single patch (#3). What's going on? > > Note: I'm also reviewer of I涎 DesignWare driver, you really have to > fix your tools / submission process and try again. No review for this > series. > > -- > With Best Regards, > Andy Shevchenko > >
