[PATCH v2 0/2] Add LPS0 check() for pinctrl-amd

[PATCH v2 0/2] Add LPS0 check() for pinctrl-amd

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

During suspend the pinctrl_amd driver disables the interrupts for
any GPIOs that are not marked as wake sources.

This however does not prevent them from changing the wake status
bit during suspend, it just stops the system from waking.

If the system wakes from hardware sleep for another reason (such
as plugging in the AC adapter) this wake bits might be active.

This could potentially cause problems with going back to hardware
sleep.  Add an extra debugging message when PM debugging is enabled
to help identify if this is happening.

Mario Limonciello (2):
  ACPI: Add missing prototype for non CONFIG_SUSPEND/CONFIG_X86 case
  pinctrl: amd: Add an LPS0 check() callback

 drivers/pinctrl/pinctrl-amd.c | 28 ++++++++++++++++++++++++++++
 include/linux/acpi.h          |  7 +++++++
 2 files changed, 35 insertions(+)

-- 
2.43.0

[PATCH v2 1/2] ACPI: Add missing prototype for non CONFIG_SUSPEND/CONFIG_X86 case

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

acpi_register_lps0_dev() and acpi_unregister_lps0_dev() may be used
in drivers that don't require CONFIG_SUSPEND or compile on !X86.

Add prototypes for those cases.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202502191627.fRgoBwcZ-lkp@intel.com/
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 include/linux/acpi.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4e495b29c640f..8e16ca5046e45 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1130,6 +1130,13 @@ static inline int acpi_get_lps0_constraint(struct device *dev)
 {
 	return ACPI_STATE_UNKNOWN;
 }
+static inline int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg)
+{
+	return -ENODEV;
+}
+static inline void acpi_unregister_lps0_dev(struct acpi_s2idle_dev_ops *arg)
+{
+}
 #endif /* CONFIG_SUSPEND && CONFIG_X86 */
 void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
 #else
-- 
2.43.0

[PATCH v2 2/2] pinctrl: amd: Add an LPS0 check() callback

[Mario Limonciello]

From: Mario Limonciello <mario.limonciello@amd.com>

During suspend the pinctrl_amd driver disables the interrupts for
any GPIOs that are not marked as wake sources.

This however does not prevent them from changing the wake status
bit during suspend, it just stops the system from waking.

If the system wakes from hardware sleep for another reason (such
as plugging in the AC adapter) this wake bits might be active.

This could potentially cause problems with going back to hardware
sleep.  Add an extra debugging message when PM debugging is enabled
to help identify if this is happening.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3929
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/pinctrl/pinctrl-amd.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
index 1d7fdcdec4c85..fdda8d1c4f344 100644
--- a/drivers/pinctrl/pinctrl-amd.c
+++ b/drivers/pinctrl/pinctrl-amd.c
@@ -37,6 +37,8 @@
 #include "pinctrl-utils.h"
 #include "pinctrl-amd.h"
 
+static struct amd_gpio *pinctrl_dev;
+
 static int amd_gpio_get_direction(struct gpio_chip *gc, unsigned offset)
 {
 	unsigned long flags;
@@ -909,6 +911,29 @@ static bool amd_gpio_should_save(struct amd_gpio *gpio_dev, unsigned int pin)
 	return false;
 }
 
+static void amd_gpio_check_pending(void)
+{
+	struct amd_gpio *gpio_dev = pinctrl_dev;
+	struct pinctrl_desc *desc = gpio_dev->pctrl->desc;
+	int i;
+
+	if (!pm_debug_messages_on)
+		return;
+
+	for (i = 0; i < desc->npins; i++) {
+		int pin = desc->pins[i].number;
+		u32 tmp;
+
+		tmp = readl(gpio_dev->base + pin * 4);
+		if (tmp & PIN_IRQ_PENDING)
+			pm_pr_dbg("%s: GPIO %d is active: 0x%x.\n", __func__, pin, tmp);
+	}
+}
+
+static struct acpi_s2idle_dev_ops pinctrl_amd_s2idle_dev_ops = {
+	.check = amd_gpio_check_pending,
+};
+
 static int amd_gpio_suspend_hibernate_common(struct device *dev, bool is_suspend)
 {
 	struct amd_gpio *gpio_dev = dev_get_drvdata(dev);
@@ -942,6 +967,7 @@ static int amd_gpio_suspend_hibernate_common(struct device *dev, bool is_suspend
 
 static int amd_gpio_suspend(struct device *dev)
 {
+	pinctrl_dev = dev_get_drvdata(dev);
 	return amd_gpio_suspend_hibernate_common(dev, true);
 }
 
@@ -1181,6 +1207,7 @@ static int amd_gpio_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, gpio_dev);
 	acpi_register_wakeup_handler(gpio_dev->irq, amd_gpio_check_wake, gpio_dev);
+	acpi_register_lps0_dev(&pinctrl_amd_s2idle_dev_ops);
 
 	dev_dbg(&pdev->dev, "amd gpio driver loaded\n");
 	return ret;
@@ -1199,6 +1226,7 @@ static void amd_gpio_remove(struct platform_device *pdev)
 
 	gpiochip_remove(&gpio_dev->gc);
 	acpi_unregister_wakeup_handler(amd_gpio_check_wake, gpio_dev);
+	acpi_unregister_lps0_dev(&pinctrl_amd_s2idle_dev_ops);
 }
 
 #ifdef CONFIG_ACPI
-- 
2.43.0

Re: [PATCH v2 1/2] ACPI: Add missing prototype for non CONFIG_SUSPEND/CONFIG_X86 case

[Rafael J. Wysocki]

On Wed, Feb 19, 2025 at 3:54 PM Mario Limonciello <superm1@kernel.org> wrote:
>
> From: Mario Limonciello <mario.limonciello@amd.com>
>
> acpi_register_lps0_dev() and acpi_unregister_lps0_dev() may be used
> in drivers that don't require CONFIG_SUSPEND or compile on !X86.
>
> Add prototypes for those cases.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202502191627.fRgoBwcZ-lkp@intel.com/
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  include/linux/acpi.h | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 4e495b29c640f..8e16ca5046e45 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1130,6 +1130,13 @@ static inline int acpi_get_lps0_constraint(struct device *dev)
>  {
>         return ACPI_STATE_UNKNOWN;
>  }
> +static inline int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg)
> +{
> +       return -ENODEV;
> +}
> +static inline void acpi_unregister_lps0_dev(struct acpi_s2idle_dev_ops *arg)
> +{
> +}
>  #endif /* CONFIG_SUSPEND && CONFIG_X86 */
>  void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
>  #else
> --

Applied as 6.15 material, thanks!

Re: [PATCH v2 1/2] ACPI: Add missing prototype for non CONFIG_SUSPEND/CONFIG_X86 case

[Rafael J. Wysocki]

On Thu, Feb 20, 2025 at 9:05 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Wed, Feb 19, 2025 at 3:54 PM Mario Limonciello <superm1@kernel.org> wrote:
> >
> > From: Mario Limonciello <mario.limonciello@amd.com>
> >
> > acpi_register_lps0_dev() and acpi_unregister_lps0_dev() may be used
> > in drivers that don't require CONFIG_SUSPEND or compile on !X86.
> >
> > Add prototypes for those cases.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202502191627.fRgoBwcZ-lkp@intel.com/
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> >  include/linux/acpi.h | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index 4e495b29c640f..8e16ca5046e45 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -1130,6 +1130,13 @@ static inline int acpi_get_lps0_constraint(struct device *dev)
> >  {
> >         return ACPI_STATE_UNKNOWN;
> >  }
> > +static inline int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg)
> > +{
> > +       return -ENODEV;
> > +}
> > +static inline void acpi_unregister_lps0_dev(struct acpi_s2idle_dev_ops *arg)
> > +{
> > +}
> >  #endif /* CONFIG_SUSPEND && CONFIG_X86 */
> >  void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
> >  #else
> > --
>
> Applied as 6.15 material, thanks!

And dropped due to build issues:

https://lore.kernel.org/linux-acpi/202502211223.IKdRg7rk-lkp@intel.com/

Re: [PATCH v2 2/2] pinctrl: amd: Add an LPS0 check() callback

[Linus Walleij]

Hi Mario,

thanks for your patch!

On Wed, Feb 19, 2025 at 3:54 PM Mario Limonciello <superm1@kernel.org> wrote:

> From: Mario Limonciello <mario.limonciello@amd.com>
>
> During suspend the pinctrl_amd driver disables the interrupts for
> any GPIOs that are not marked as wake sources.
>
> This however does not prevent them from changing the wake status
> bit during suspend, it just stops the system from waking.
>
> If the system wakes from hardware sleep for another reason (such
> as plugging in the AC adapter) this wake bits might be active.
>
> This could potentially cause problems with going back to hardware
> sleep.  Add an extra debugging message when PM debugging is enabled
> to help identify if this is happening.
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3929
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

I don't understand the ACPI part of this patch, and I think I was
not even CC:ed on 1/2 so I haven't seen it.

Anyway:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

if you want to apply it in the ACPI tree.

Yours,
Linus Walleij

Re: [PATCH v2 2/2] pinctrl: amd: Add an LPS0 check() callback

[Mario Limonciello]

On 2/27/2025 14:34, Linus Walleij wrote:
> Hi Mario,
> 
> thanks for your patch!
> 
> On Wed, Feb 19, 2025 at 3:54 PM Mario Limonciello <superm1@kernel.org> wrote:
> 
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> During suspend the pinctrl_amd driver disables the interrupts for
>> any GPIOs that are not marked as wake sources.
>>
>> This however does not prevent them from changing the wake status
>> bit during suspend, it just stops the system from waking.
>>
>> If the system wakes from hardware sleep for another reason (such
>> as plugging in the AC adapter) this wake bits might be active.
>>
>> This could potentially cause problems with going back to hardware
>> sleep.  Add an extra debugging message when PM debugging is enabled
>> to help identify if this is happening.
>>
>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3929
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> 
> I don't understand the ACPI part of this patch, and I think I was
> not even CC:ed on 1/2 so I haven't seen it.
> 
> Anyway:
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
> if you want to apply it in the ACPI tree.
> 
> Yours,
> Linus Walleij

Thanks, there was a problem reported by LKP robot on v2 for the ACPI patch.

I have a v3 but was waiting for feedback on this one.
Will add your tag and send v3, thanks!