Re: [PATCH] pinctrl: amd: Clear S4 wake bits when firmware has _AEI

[Shyam Sundar S K <Shyam-sundar.S-k@amd.com>]

On 02-10-2026 20:00, Mario Limonciello wrote:
> Commit ffe8a0c6b552 ("pinctrl-amd: Don't clear S4 wake bits at probe")
> preserves firmware-programmed S4 wake sources because some PCIe devices
> rely on them for Wake-on-LAN. The system that motivated that change does
> not expose ACPI GPIO events through _AEI.

This is definitely better than reverting or carrying per-board DMI quirks.

One corner case I can think of before this hits "stable:" on a system that
defines _AEI (say for a touchpad or buttons) but routes an onboard NIC
through a raw GPIO pin not listed in _AEI, this clears that pin's S4 bit
at probe. That could kill WoL from S4/S5 unless firmware re-arms it
during sleep transition. 

Low risk since nobody has reported that setup, but worth tracking if any WoL
regressions pop up.

A small point on the comment:

> 
> On a Chuwi CoreBook Plus, firmware exposes _AEI but also leaves the S4/S5
> wake bit set for GPIO 24. This causes the machine to power back on
> immediately after shutdown even though Wake-on-LAN is not enabled.
> 
> GPIO register dumps show that the only relevant difference is bit 15 on
> GPIO 24. Clearing that bit makes the machine remain off. However, GPIO 24
> is not itself listed in _AEI, so clearing only the pins described there
> would not fix the problem.
> 
> Use the presence of _AEI to distinguish the platform's GPIO event model.
> Continue clearing S0i3/S3 bits on every system. Clear S4 bits when firmware
> exposes _AEI, indicating that the platform uses OS-managed ACPI GPIO
> events, but preserve S4 bits when _AEI is absent for firmware-programmed
> sources such as PCIe PME that bypass the GPIO IRQ wake API.
> 
> Fixes: ffe8a0c6b552 ("pinctrl-amd: Don't clear S4 wake bits at probe")
> Reported-by: Olzhas Marat <marat.olzhas@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=222041
> Cc: stable@vger.kernel.org
> Tested-by: Olzhas Marat <marat.olzhas@gmail.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/pinctrl/pinctrl-amd.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
> index 15a398bb3be23..dca01c5d2231b 100644
> --- a/drivers/pinctrl/pinctrl-amd.c
> +++ b/drivers/pinctrl/pinctrl-amd.c
> @@ -880,12 +880,25 @@ static const struct pinconf_ops amd_pinconf_ops = {
>  static void amd_gpio_irq_init(struct amd_gpio *gpio_dev)
>  {
>  	const struct pinctrl_desc *desc = gpio_dev->pctrl->desc;
> +	acpi_handle handle = ACPI_HANDLE(&gpio_dev->pdev->dev);
>  	unsigned long flags;
>  	u32 pin_reg, mask;
>  	int i;
>  
>  	mask = BIT(WAKE_CNTRL_OFF_S0I3) | BIT(WAKE_CNTRL_OFF_S3);
>  
> +	/*
> +	 * _AEI indicates that the platform uses OS-managed ACPI GPIO events.
> +	 * Without it, preserve S4 wake bits for firmware-programmed sources
> +	 * such as PCIe PME that bypass the GPIO IRQ wake API.
> +	 */

On the Olzhas's Chuwi machine, pin 24 isn't in _AEI either, yet it's
the pin causing the issue. So _AEI isn't distinguishing which pins the
OS manages - it's more of a platform-level signal that happens to tell
the two boards apart.

Also, You could drop the outer "if (handle)" check though, since acpi_has_method() safely
returns false on a NULL handle:

Either way:

Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>

Thanks,
Shyam

> +	if (handle) {
> +#ifdef CONFIG_ACPI
> +		if (acpi_has_method(handle, "_AEI"))
> +			mask |= BIT(WAKE_CNTRL_OFF_S4);
> +#endif
> +	}
> +
>  	for (i = 0; i < desc->npins; i++) {
>  		int pin = desc->pins[i].number;
>  		const struct pin_desc *pd = pin_desc_get(gpio_dev->pctrl, pin);