* [PATCH 0/1] Improved debugging for active GPIO causing wakeup
@ 2022-10-12 22:10 Mario Limonciello
2022-10-12 22:10 ` [PATCH 1/1] pinctrl: amd: Add dynamic debugging for active GPIOs Mario Limonciello
0 siblings, 1 reply; 5+ messages in thread
From: Mario Limonciello @ 2022-10-12 22:10 UTC (permalink / raw)
To: Linus Walleij, linux-gpio
Cc: Mark Pearson, Natikar Basavaraj, Pananchikkal Renjith,
Shyam Sundar S K, Kai-Heng Feng, Mario Limonciello, linux-kernel
Some laptops have been reported to wake up from s2idle when plugging
in the AC adapter or by closing the lid. This series adds support
to better debug what is going on.
With this patch in place we can see that this laptop woke up from
the following GPIO (which on this design happens to also be mirrored
to GPE0xe):
amd_gpio AMDI0030:00: GPIO 18 is active: 0x30057a00
The use of this particular GPIO is OEM design specific.
Knowing these details, it's possible to workaround this problem with
the following on the kernel command line using the parameter for
gpiolib_acpi introduced in commit 6b6af7bd5718f ("gpiolib: acpi:
Add support to ignore programming an interrupt")
acpi_mask_gpe=0x0e gpiolib_acpi.ignore_interrupt=AMDI0030:00@18
Mario Limonciello (1):
pinctrl: amd: Add dynamic debugging for active GPIOs
drivers/pinctrl/pinctrl-amd.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/1] pinctrl: amd: Add dynamic debugging for active GPIOs
2022-10-12 22:10 [PATCH 0/1] Improved debugging for active GPIO causing wakeup Mario Limonciello
@ 2022-10-12 22:10 ` Mario Limonciello
2022-10-13 6:38 ` Basavaraj Natikar
2022-10-13 13:45 ` Kai-Heng Feng
0 siblings, 2 replies; 5+ messages in thread
From: Mario Limonciello @ 2022-10-12 22:10 UTC (permalink / raw)
To: Linus Walleij, Basavaraj Natikar, Shyam Sundar S K
Cc: Mark Pearson, Pananchikkal Renjith, Kai-Heng Feng,
Mario Limonciello, linux-gpio, linux-kernel
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@amd.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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] pinctrl: amd: Add dynamic debugging for active GPIOs
2022-10-12 22:10 ` [PATCH 1/1] pinctrl: amd: Add dynamic debugging for active GPIOs Mario Limonciello
@ 2022-10-13 6:38 ` Basavaraj Natikar
2022-10-13 13:45 ` Kai-Heng Feng
1 sibling, 0 replies; 5+ messages in thread
From: Basavaraj Natikar @ 2022-10-13 6:38 UTC (permalink / raw)
To: Mario Limonciello, Linus Walleij, Basavaraj Natikar, Shyam Sundar S K
Cc: Mark Pearson, Pananchikkal Renjith, Kai-Heng Feng, linux-gpio,
linux-kernel
On 10/13/2022 3:40 AM, Mario Limonciello wrote:
> 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@amd.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))
Above if can be simplified as "if (regval & PIN_IRQ_PENDING)"
with that change,
Acked-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Thanks,
--
Basavaraj
> 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)))
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] pinctrl: amd: Add dynamic debugging for active GPIOs
2022-10-12 22:10 ` [PATCH 1/1] pinctrl: amd: Add dynamic debugging for active GPIOs Mario Limonciello
2022-10-13 6:38 ` Basavaraj Natikar
@ 2022-10-13 13:45 ` Kai-Heng Feng
[not found] ` <TYZPR03MB59942B6A03B43132DADF1C5CBD259@TYZPR03MB5994.apcprd03.prod.outlook.com>
1 sibling, 1 reply; 5+ messages in thread
From: Kai-Heng Feng @ 2022-10-13 13:45 UTC (permalink / raw)
To: Mario Limonciello
Cc: Linus Walleij, Basavaraj Natikar, Shyam Sundar S K, Mark Pearson,
Pananchikkal Renjith, linux-gpio, linux-kernel
On Thu, Oct 13, 2022 at 6:10 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> 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@amd.com>
This is very useful, thanks for the patch!
Acked-by: Kai-Heng Feng <kai.heng.feng@canonical.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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-13 14:03 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-12 22:10 [PATCH 0/1] Improved debugging for active GPIO causing wakeup Mario Limonciello
2022-10-12 22:10 ` [PATCH 1/1] pinctrl: amd: Add dynamic debugging for active GPIOs Mario Limonciello
2022-10-13 6:38 ` Basavaraj Natikar
2022-10-13 13:45 ` Kai-Heng Feng
[not found] ` <TYZPR03MB59942B6A03B43132DADF1C5CBD259@TYZPR03MB5994.apcprd03.prod.outlook.com>
2022-10-13 13:53 ` Fw: [External] " Mark Pearson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).