[<prev] [next>] [day] [month] [year] [list]
Message-ID: <df1c3d64-7255-0b7d-51a4-01a615a8c07b@lenovo.com>
Date: Thu, 13 Oct 2022 09:53:23 -0400
From: Mark Pearson <markpearson@...ovo.com>
To: Mario Limonciello <mario.limonciello@....com>
CC: Kai-Heng Feng <kai.heng.feng@...onical.com>,
<linus.walleij@...aro.org>, <Basavaraj.Natikar@....com>,
"S-k, Shyam-sundar" <Shyam-sundar.S-k@....com>,
Mark Pearson <mpearson@...ovo.com>,
<renjith.pananchikkal@....com>, <linux-gpio@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: Fw: [External] Re: [PATCH 1/1] pinctrl: amd: Add dynamic
debugging for active GPIOs
>>
>> Some laptops have been reported to wake up from s2idle when plugging
>> in the AC adapter or by closing the lid. This is a surprising
>> behavior that is further clarified by commit cb3e7d624c3ff ("PM:
>> wakeup: Add extra debugging statement for multiple active IRQs").
>>
>> With that commit in place the following interaction can be seen
>> when the lid is closed:
>>
>> [ 28.946038] PM: suspend-to-idle
>> [ 28.946083] ACPI: EC: ACPI EC GPE status set
>> [ 28.946101] ACPI: PM: Rearming ACPI SCI for wakeup
>> [ 28.950152] Timekeeping suspended for 3.320 seconds
>> [ 28.950152] PM: Triggering wakeup from IRQ 9
>> [ 28.950152] ACPI: EC: ACPI EC GPE status set
>> [ 28.950152] ACPI: EC: ACPI EC GPE dispatched
>> [ 28.995057] ACPI: EC: ACPI EC work flushed
>> [ 28.995075] ACPI: PM: Rearming ACPI SCI for wakeup
>> [ 28.995131] PM: Triggering wakeup from IRQ 9
>> [ 28.995271] ACPI: EC: ACPI EC GPE status set
>> [ 28.995291] ACPI: EC: ACPI EC GPE dispatched
>> [ 29.098556] ACPI: EC: ACPI EC work flushed
>> [ 29.207020] ACPI: EC: ACPI EC work flushed
>> [ 29.207037] ACPI: PM: Rearming ACPI SCI for wakeup
>> [ 29.211095] Timekeeping suspended for 0.739 seconds
>> [ 29.211095] PM: Triggering wakeup from IRQ 9
>> [ 29.211079] PM: Triggering wakeup from IRQ 7
>> [ 29.211095] ACPI: PM: ACPI non-EC GPE wakeup
>> [ 29.211095] PM: resume from suspend-to-idle
>>
>> * IRQ9 on this laptop is used for the ACPI SCI.
>> * IRQ7 on this laptop is used for the GPIO controller.
>>
>> What has occurred is when the lid was closed the EC woke up the
>> SoC from it's deepest sleep state and the kernel's s2idle loop
>> processed all EC events. When it was finished processing EC events,
>> it checked for any other reasons to wake (break the s2idle loop).
>>
>> The IRQ for the GPIO controller was active so the loop broke, and
>> then this IRQ was processed. This is not a kernel bug but it is
>> certainly a surprising behavior, and to better debug it we should
>> have a dynamic debugging message that we can enact to catch it.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>
> This is very useful, thanks for the patch!
>
> Acked-by: Kai-Heng Feng <kai.heng.feng@...onical.com>
>
Seconded! Helped tracking down some issues on our platforms. Thanks Mario
and AMD team.
Acked-by: markpearson@...ovo.com
>> ---
>> drivers/pinctrl/pinctrl-amd.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
>> index 4691a33bc374f..8378b4115ec0d 100644
>> --- a/drivers/pinctrl/pinctrl-amd.c
>> +++ b/drivers/pinctrl/pinctrl-amd.c
>> @@ -628,13 +628,16 @@ static bool do_amd_gpio_irq_handler(int irq, void *dev_id)
>> /* Each status bit covers four pins */
>> for (i = 0; i < 4; i++) {
>> regval = readl(regs + i);
>> - /* caused wake on resume context for shared IRQ */
>> - if (irq < 0 && (regval & BIT(WAKE_STS_OFF))) {
>> +
>> + if (regval & BIT(WAKE_STS_OFF) ||
>> + regval & BIT(INTERRUPT_STS_OFF))
>> dev_dbg(&gpio_dev->pdev->dev,
>> - "Waking due to GPIO %d: 0x%x",
>> + "GPIO %d is active: 0x%x",
>> irqnr + i, regval);
>> +
>> + /* caused wake on resume context for shared IRQ */
>> + if (irq < 0 && (regval & BIT(WAKE_STS_OFF)))
>> return true;
>> - }
>>
>> if (!(regval & PIN_IRQ_PENDING) ||
>> !(regval & BIT(INTERRUPT_MASK_OFF)))
>> --
>> 2.34.1
>>
Powered by blists - more mailing lists