From: Basavaraj Natikar <bnatikar@amd.com>
To: Mario Limonciello <mario.limonciello@amd.com>,
Linus Walleij <linus.walleij@linaro.org>,
Basavaraj Natikar <Basavaraj.Natikar@amd.com>,
Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Cc: Mark Pearson <mpearson@lenovo.com>,
Pananchikkal Renjith <Renjith.Pananchikkal@amd.com>,
Kai-Heng Feng <kai.heng.feng@canonical.com>,
linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] pinctrl: amd: Add dynamic debugging for active GPIOs
Date: Thu, 13 Oct 2022 12:08:30 +0530 [thread overview]
Message-ID: <8fc979d4-089a-7293-d580-a2affc74d68d@amd.com> (raw)
In-Reply-To: <20221012221028.4817-2-mario.limonciello@amd.com>
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)))
next prev parent reply other threads:[~2022-10-13 6:38 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8fc979d4-089a-7293-d580-a2affc74d68d@amd.com \
--to=bnatikar@amd.com \
--cc=Basavaraj.Natikar@amd.com \
--cc=Renjith.Pananchikkal@amd.com \
--cc=Shyam-sundar.S-k@amd.com \
--cc=kai.heng.feng@canonical.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mario.limonciello@amd.com \
--cc=mpearson@lenovo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).