lkml.org 
[lkml]   [2022]   [Aug]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH 0/8] acpi: i2c: Use SharedAndWake and ExclusiveAndWake to enable wake irq
    From
    Hi,

    On 8/31/22 16:37, Raul Rangel wrote:
    > Interesting... The patch series is here:
    > https://patchwork.kernel.org/project/linux-input/cover/20220830231541.1135813-1-rrangel@chromium.org/
    >
    > I'll look into why you only got added to 2 of the emails.

    FWIW I also received the full series without problems.

    I'll try to reply to this soon-ish, but I have a bit of
    a patch backlog to process and I'm trying to process
    the backlog in FIFO order and this is one of the last
    series in the backlog ...

    Regards,

    Hans


    >
    > On Wed, Aug 31, 2022 at 5:52 AM Andy Shevchenko
    > <andriy.shevchenko@linux.intel.com> 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
    >>
    >>
    >

    \
     
     \ /
      Last update: 2022-08-31 17:20    [W:5.405 / U:0.156 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site