All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gpio: Request interrupts after IRQ is initialized
@ 2022-04-14  2:57 Mario Limonciello
  2022-04-14 15:25 ` Andy Shevchenko
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Mario Limonciello @ 2022-04-14  2:57 UTC (permalink / raw)
  To: mario.limonciello, Linus Walleij, Bartosz Golaszewski,
	Andy Shevchenko, Shreeya Patel, open list:GPIO SUBSYSTEM,
	open list
  Cc: Basavaraj.Natikar, Richard.Gong, stable

commit 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before
initialization") attempted to fix a race condition that lead to a NULL
pointer, but in the process caused a regression for _AEI/_EVT declared
GPIOs. This manifests in messages showing deferred probing while trying
to allocate IRQs like so:

[    0.688318] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0000 to IRQ, err -517
[    0.688337] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x002C to IRQ, err -517
[    0.688348] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003D to IRQ, err -517
[    0.688359] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003E to IRQ, err -517
[    0.688369] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003A to IRQ, err -517
[    0.688379] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003B to IRQ, err -517
[    0.688389] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0002 to IRQ, err -517
[    0.688399] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0011 to IRQ, err -517
[    0.688410] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0012 to IRQ, err -517
[    0.688420] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0007 to IRQ, err -517

The code for walking _AEI doesn't handle deferred probing and so this leads
to non-functional GPIO interrupts.

Fix this issue by moving the call to `acpi_gpiochip_request_interrupts` to
occur after gc->irc.initialized is set.

Cc: Shreeya Patel <shreeya.patel@collabora.com>
Cc: stable@vger.kernel.org
Fixes: 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before initialization")
Reported-by: Mario Limonciello <mario.limonciello@amd.com>
Link: https://lore.kernel.org/linux-gpio/BL1PR12MB51577A77F000A008AA694675E2EF9@BL1PR12MB5157.namprd12.prod.outlook.com/T/#u
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpio/gpiolib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 085348e08986..b7694171655c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1601,8 +1601,6 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
 
 	gpiochip_set_irq_hooks(gc);
 
-	acpi_gpiochip_request_interrupts(gc);
-
 	/*
 	 * Using barrier() here to prevent compiler from reordering
 	 * gc->irq.initialized before initialization of above
@@ -1612,6 +1610,8 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
 
 	gc->irq.initialized = true;
 
+	acpi_gpiochip_request_interrupts(gc);
+
 	return 0;
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-14  2:57 [PATCH] gpio: Request interrupts after IRQ is initialized Mario Limonciello
@ 2022-04-14 15:25 ` Andy Shevchenko
  2022-04-14 19:11 ` Shreeya Patel
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Andy Shevchenko @ 2022-04-14 15:25 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Linus Walleij, Bartosz Golaszewski, Shreeya Patel,
	open list:GPIO SUBSYSTEM, open list, Basavaraj Natikar,
	Richard.Gong, Stable

On Thu, Apr 14, 2022 at 5:57 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> commit 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before
> initialization") attempted to fix a race condition that lead to a NULL
> pointer, but in the process caused a regression for _AEI/_EVT declared
> GPIOs. This manifests in messages showing deferred probing while trying
> to allocate IRQs like so:
>
> [    0.688318] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0000 to IRQ, err -517
> [    0.688337] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x002C to IRQ, err -517
> [    0.688348] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003D to IRQ, err -517
> [    0.688359] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003E to IRQ, err -517
> [    0.688369] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003A to IRQ, err -517
> [    0.688379] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003B to IRQ, err -517
> [    0.688389] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0002 to IRQ, err -517
> [    0.688399] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0011 to IRQ, err -517
> [    0.688410] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0012 to IRQ, err -517
> [    0.688420] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0007 to IRQ, err -517
>
> The code for walking _AEI doesn't handle deferred probing and so this leads
> to non-functional GPIO interrupts.
>
> Fix this issue by moving the call to `acpi_gpiochip_request_interrupts` to
> occur after gc->irc.initialized is set.

Good catch, Mario, and thanks for the prompt fix!
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Cc: Shreeya Patel <shreeya.patel@collabora.com>
> Cc: stable@vger.kernel.org
> Fixes: 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before initialization")
> Reported-by: Mario Limonciello <mario.limonciello@amd.com>
> Link: https://lore.kernel.org/linux-gpio/BL1PR12MB51577A77F000A008AA694675E2EF9@BL1PR12MB5157.namprd12.prod.outlook.com/T/#u
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/gpio/gpiolib.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 085348e08986..b7694171655c 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1601,8 +1601,6 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>
>         gpiochip_set_irq_hooks(gc);
>
> -       acpi_gpiochip_request_interrupts(gc);
> -
>         /*
>          * Using barrier() here to prevent compiler from reordering
>          * gc->irq.initialized before initialization of above
> @@ -1612,6 +1610,8 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>
>         gc->irq.initialized = true;
>
> +       acpi_gpiochip_request_interrupts(gc);
> +
>         return 0;
>  }
>
> --
> 2.34.1
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-14  2:57 [PATCH] gpio: Request interrupts after IRQ is initialized Mario Limonciello
  2022-04-14 15:25 ` Andy Shevchenko
@ 2022-04-14 19:11 ` Shreeya Patel
  2022-04-17 12:17 ` luke
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: Shreeya Patel @ 2022-04-14 19:11 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Basavaraj.Natikar, Richard.Gong, stable, open list,
	Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	open list:GPIO SUBSYSTEM, gabriel Krisman Bertazi,
	Collabora Kernel ML


On 14/04/22 08:27, Mario Limonciello wrote:
> commit 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before
> initialization") attempted to fix a race condition that lead to a NULL
> pointer, but in the process caused a regression for _AEI/_EVT declared
> GPIOs. This manifests in messages showing deferred probing while trying
> to allocate IRQs like so:
>
> [    0.688318] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0000 to IRQ, err -517
> [    0.688337] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x002C to IRQ, err -517
> [    0.688348] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003D to IRQ, err -517
> [    0.688359] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003E to IRQ, err -517
> [    0.688369] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003A to IRQ, err -517
> [    0.688379] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003B to IRQ, err -517
> [    0.688389] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0002 to IRQ, err -517
> [    0.688399] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0011 to IRQ, err -517
> [    0.688410] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0012 to IRQ, err -517
> [    0.688420] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0007 to IRQ, err -517
>
> The code for walking _AEI doesn't handle deferred probing and so this leads
> to non-functional GPIO interrupts.
>
> Fix this issue by moving the call to `acpi_gpiochip_request_interrupts` to
> occur after gc->irc.initialized is set.


Thanks Mario for sending a fix for this issue, I didn't realize 
gpiod_to_irq() was also
being called through acpi_gpiochip_request_interrupts().
Change looks good to me.
Reviewed-by: Shreeya Patel <shreeya.patel@collabora.com>

> Cc: Shreeya Patel <shreeya.patel@collabora.com>
> Cc: stable@vger.kernel.org
> Fixes: 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before initialization")
> Reported-by: Mario Limonciello <mario.limonciello@amd.com>
> Link: https://lore.kernel.org/linux-gpio/BL1PR12MB51577A77F000A008AA694675E2EF9@BL1PR12MB5157.namprd12.prod.outlook.com/T/#u
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/gpio/gpiolib.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 085348e08986..b7694171655c 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1601,8 +1601,6 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>   
>   	gpiochip_set_irq_hooks(gc);
>   
> -	acpi_gpiochip_request_interrupts(gc);
> -
>   	/*
>   	 * Using barrier() here to prevent compiler from reordering
>   	 * gc->irq.initialized before initialization of above
> @@ -1612,6 +1610,8 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>   
>   	gc->irq.initialized = true;
>   
> +	acpi_gpiochip_request_interrupts(gc);
> +
>   	return 0;
>   }
>   

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-14  2:57 [PATCH] gpio: Request interrupts after IRQ is initialized Mario Limonciello
  2022-04-14 15:25 ` Andy Shevchenko
  2022-04-14 19:11 ` Shreeya Patel
@ 2022-04-17 12:17 ` luke
  2022-04-17 12:24 ` firew4lker
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 18+ messages in thread
From: luke @ 2022-04-17 12:17 UTC (permalink / raw)
  To: mario.limonciello
  Cc: Basavaraj.Natikar, Richard.Gong, andy.shevchenko, brgl,
	linus.walleij, linux-gpio, linux-kernel, shreeya.patel, stable

Tested-By: lukeluk498@gmail.com Link: 
https://gitlab.freedesktop.org/drm/amd/-/issues/1976


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-14  2:57 [PATCH] gpio: Request interrupts after IRQ is initialized Mario Limonciello
                   ` (2 preceding siblings ...)
  2022-04-17 12:17 ` luke
@ 2022-04-17 12:24 ` firew4lker
  2022-04-18  4:34   ` Mario Limonciello
  2022-04-18 13:46 ` Thorsten Leemhuis
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: firew4lker @ 2022-04-17 12:24 UTC (permalink / raw)
  To: Mario Limonciello, Linus Walleij, Bartosz Golaszewski,
	Andy Shevchenko, Shreeya Patel, open list:GPIO SUBSYSTEM,
	open list
  Cc: Basavaraj.Natikar, Richard.Gong, stable

On 4/14/22 05:57, Mario Limonciello wrote:
> commit 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before
> initialization") attempted to fix a race condition that lead to a NULL
> pointer, but in the process caused a regression for _AEI/_EVT declared
> GPIOs. This manifests in messages showing deferred probing while trying
> to allocate IRQs like so:
>
> [    0.688318] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0000 to IRQ, err -517
> [    0.688337] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x002C to IRQ, err -517
> [    0.688348] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003D to IRQ, err -517
> [    0.688359] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003E to IRQ, err -517
> [    0.688369] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003A to IRQ, err -517
> [    0.688379] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003B to IRQ, err -517
> [    0.688389] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0002 to IRQ, err -517
> [    0.688399] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0011 to IRQ, err -517
> [    0.688410] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0012 to IRQ, err -517
> [    0.688420] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0007 to IRQ, err -517
>
> The code for walking _AEI doesn't handle deferred probing and so this leads
> to non-functional GPIO interrupts.
>
> Fix this issue by moving the call to `acpi_gpiochip_request_interrupts` to
> occur after gc->irc.initialized is set.
>
> Cc: Shreeya Patel <shreeya.patel@collabora.com>
> Cc: stable@vger.kernel.org
> Fixes: 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before initialization")
> Reported-by: Mario Limonciello <mario.limonciello@amd.com>
> Link: https://lore.kernel.org/linux-gpio/BL1PR12MB51577A77F000A008AA694675E2EF9@BL1PR12MB5157.namprd12.prod.outlook.com/T/#u
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/gpio/gpiolib.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 085348e08986..b7694171655c 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1601,8 +1601,6 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>   
>   	gpiochip_set_irq_hooks(gc);
>   
> -	acpi_gpiochip_request_interrupts(gc);
> -
>   	/*
>   	 * Using barrier() here to prevent compiler from reordering
>   	 * gc->irq.initialized before initialization of above
> @@ -1612,6 +1610,8 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>   
>   	gc->irq.initialized = true;
>   
> +	acpi_gpiochip_request_interrupts(gc);
> +
>   	return 0;
>   }
>   

Tested-By:firew4lker@gmail.com

This patch addresses the issue. Tested on a Lenovo T14 with AMD Ryzen 5 
PRO 4650U with Radeon Graphics Graphics.

Without this patch the laptop is impossible to wake from S3 and S0ix.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-17 12:24 ` firew4lker
@ 2022-04-18  4:34   ` Mario Limonciello
  2022-04-18 11:42     ` Andy Shevchenko
                       ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Mario Limonciello @ 2022-04-18  4:34 UTC (permalink / raw)
  To: firew4lker, Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Shreeya Patel, open list:GPIO SUBSYSTEM, open list
  Cc: Basavaraj.Natikar, Richard.Gong, stable

On 4/17/22 07:24, firew4lker wrote:
> On 4/14/22 05:57, Mario Limonciello wrote:
>> commit 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members 
>> before
>> initialization") attempted to fix a race condition that lead to a NULL
>> pointer, but in the process caused a regression for _AEI/_EVT declared
>> GPIOs. This manifests in messages showing deferred probing while trying
>> to allocate IRQs like so:
>>
>> [    0.688318] amd_gpio AMDI0030:00: Failed to translate GPIO pin 
>> 0x0000 to IRQ, err -517
>> [    0.688337] amd_gpio AMDI0030:00: Failed to translate GPIO pin 
>> 0x002C to IRQ, err -517
>> [    0.688348] amd_gpio AMDI0030:00: Failed to translate GPIO pin 
>> 0x003D to IRQ, err -517
>> [    0.688359] amd_gpio AMDI0030:00: Failed to translate GPIO pin 
>> 0x003E to IRQ, err -517
>> [    0.688369] amd_gpio AMDI0030:00: Failed to translate GPIO pin 
>> 0x003A to IRQ, err -517
>> [    0.688379] amd_gpio AMDI0030:00: Failed to translate GPIO pin 
>> 0x003B to IRQ, err -517
>> [    0.688389] amd_gpio AMDI0030:00: Failed to translate GPIO pin 
>> 0x0002 to IRQ, err -517
>> [    0.688399] amd_gpio AMDI0030:00: Failed to translate GPIO pin 
>> 0x0011 to IRQ, err -517
>> [    0.688410] amd_gpio AMDI0030:00: Failed to translate GPIO pin 
>> 0x0012 to IRQ, err -517
>> [    0.688420] amd_gpio AMDI0030:00: Failed to translate GPIO pin 
>> 0x0007 to IRQ, err -517
>>
>> The code for walking _AEI doesn't handle deferred probing and so this 
>> leads
>> to non-functional GPIO interrupts.
>>
>> Fix this issue by moving the call to 
>> `acpi_gpiochip_request_interrupts` to
>> occur after gc->irc.initialized is set.
>>
>> Cc: Shreeya Patel <shreeya.patel@collabora.com>
>> Cc: stable@vger.kernel.org
>> Fixes: 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members 
>> before initialization")
>> Reported-by: Mario Limonciello <mario.limonciello@amd.com>
>> Link: 
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flinux-gpio%2FBL1PR12MB51577A77F000A008AA694675E2EF9%40BL1PR12MB5157.namprd12.prod.outlook.com%2FT%2F%23u&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7C96ec39c78488493fd5ca08da206d3c7b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637857951204650754%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=xZbNC%2F50JqlNwcTYAtGLn6z0%2FEPbfCKKOc%2BlZlMh0EQ%3D&amp;reserved=0 
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/gpio/gpiolib.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>> index 085348e08986..b7694171655c 100644
>> --- a/drivers/gpio/gpiolib.c
>> +++ b/drivers/gpio/gpiolib.c
>> @@ -1601,8 +1601,6 @@ static int gpiochip_add_irqchip(struct gpio_chip 
>> *gc,
>>       gpiochip_set_irq_hooks(gc);
>> -    acpi_gpiochip_request_interrupts(gc);
>> -
>>       /*
>>        * Using barrier() here to prevent compiler from reordering
>>        * gc->irq.initialized before initialization of above
>> @@ -1612,6 +1610,8 @@ static int gpiochip_add_irqchip(struct gpio_chip 
>> *gc,
>>       gc->irq.initialized = true;
>> +    acpi_gpiochip_request_interrupts(gc);
>> +
>>       return 0;
>>   }
> 
> Tested-By:firew4lker@gmail.com
> 
> This patch addresses the issue. Tested on a Lenovo T14 with AMD Ryzen 5 
> PRO 4650U with Radeon Graphics Graphics.
> 
> Without this patch the laptop is impossible to wake from S3 and S0ix.
> 

Thanks for testing it!

Linus Walleij,

As this is backported to 5.15.y, 5.16.y, 5.17.y and those all had point 
releases a bunch of people are hitting it now.  If you choose to adopt 
this patch instead of revert the broken one, you can add to the commit 
message too:

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1976

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-18  4:34   ` Mario Limonciello
@ 2022-04-18 11:42     ` Andy Shevchenko
  2022-04-18 13:58       ` Thorsten Leemhuis
  2022-04-19 13:03     ` Limonciello, Mario
  2022-04-19 22:02     ` Linus Walleij
  2 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2022-04-18 11:42 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: firew4lker, Linus Walleij, Bartosz Golaszewski, Shreeya Patel,
	open list:GPIO SUBSYSTEM, open list, Basavaraj Natikar,
	Richard.Gong, Stable

On Mon, Apr 18, 2022 at 7:34 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:
> On 4/17/22 07:24, firew4lker wrote:

...

> Linus Walleij,
>
> As this is backported to 5.15.y, 5.16.y, 5.17.y and those all had point
> releases a bunch of people are hitting it now.  If you choose to adopt
> this patch instead of revert the broken one, you can add to the commit
> message too:
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1976

I prefer to explicitly tell that this is a link to a bug report, hence BugLink:.
But this is just my 2 cents.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-14  2:57 [PATCH] gpio: Request interrupts after IRQ is initialized Mario Limonciello
                   ` (3 preceding siblings ...)
  2022-04-17 12:24 ` firew4lker
@ 2022-04-18 13:46 ` Thorsten Leemhuis
  2022-04-19  9:40 ` Samuel Čavoj
  2022-04-19 21:59 ` Linus Walleij
  6 siblings, 0 replies; 18+ messages in thread
From: Thorsten Leemhuis @ 2022-04-18 13:46 UTC (permalink / raw)
  To: Mario Limonciello, Linus Walleij, Bartosz Golaszewski,
	Andy Shevchenko, Shreeya Patel, open list:GPIO SUBSYSTEM,
	open list
  Cc: Basavaraj.Natikar, Richard.Gong, stable, Greg KH

Hi, this is your Linux kernel regression tracker. Top-posting for once,
to make this easily accessible to everyone.

Greg, this seems to be a regression that made the news
https://www.reddit.com/r/linux/comments/u5hbk6/psa_linux_5173_on_dell_amd_laptops_might_cause/

That made me wonder "how can we get issues like this fixed really
quickly in stable". Are you in cases like this maybe willing to drop the
backport of 5467801f1fcb quickly (in this case maybe even for the new
stable kernel versions that just were sent out as rc1) and then reapply
it later together with below fix once that was reviewed and merged to
mainline? Or is that too much of a hassle even for special case like this?

Ciao, Thorsten


On 14.04.22 04:57, Mario Limonciello wrote:
> commit 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before
> initialization") attempted to fix a race condition that lead to a NULL
> pointer, but in the process caused a regression for _AEI/_EVT declared
> GPIOs. This manifests in messages showing deferred probing while trying
> to allocate IRQs like so:
> 
> [    0.688318] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0000 to IRQ, err -517
> [    0.688337] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x002C to IRQ, err -517
> [    0.688348] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003D to IRQ, err -517
> [    0.688359] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003E to IRQ, err -517
> [    0.688369] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003A to IRQ, err -517
> [    0.688379] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003B to IRQ, err -517
> [    0.688389] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0002 to IRQ, err -517
> [    0.688399] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0011 to IRQ, err -517
> [    0.688410] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0012 to IRQ, err -517
> [    0.688420] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0007 to IRQ, err -517
> 
> The code for walking _AEI doesn't handle deferred probing and so this leads
> to non-functional GPIO interrupts.
> 
> Fix this issue by moving the call to `acpi_gpiochip_request_interrupts` to
> occur after gc->irc.initialized is set.
> 
> Cc: Shreeya Patel <shreeya.patel@collabora.com>
> Cc: stable@vger.kernel.org
> Fixes: 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before initialization")
> Reported-by: Mario Limonciello <mario.limonciello@amd.com>
> Link: https://lore.kernel.org/linux-gpio/BL1PR12MB51577A77F000A008AA694675E2EF9@BL1PR12MB5157.namprd12.prod.outlook.com/T/#u
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/gpio/gpiolib.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 085348e08986..b7694171655c 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1601,8 +1601,6 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>  
>  	gpiochip_set_irq_hooks(gc);
>  
> -	acpi_gpiochip_request_interrupts(gc);
> -
>  	/*
>  	 * Using barrier() here to prevent compiler from reordering
>  	 * gc->irq.initialized before initialization of above
> @@ -1612,6 +1610,8 @@ static int gpiochip_add_irqchip(struct gpio_chip *gc,
>  
>  	gc->irq.initialized = true;
>  
> +	acpi_gpiochip_request_interrupts(gc);
> +
>  	return 0;
>  }
>  


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-18 11:42     ` Andy Shevchenko
@ 2022-04-18 13:58       ` Thorsten Leemhuis
  2022-04-18 14:17         ` Andy Shevchenko
  0 siblings, 1 reply; 18+ messages in thread
From: Thorsten Leemhuis @ 2022-04-18 13:58 UTC (permalink / raw)
  To: Andy Shevchenko, Mario Limonciello
  Cc: firew4lker, Linus Walleij, Bartosz Golaszewski, Shreeya Patel,
	open list:GPIO SUBSYSTEM, open list, Basavaraj Natikar,
	Richard.Gong, Stable

On 18.04.22 13:42, Andy Shevchenko wrote:
> On Mon, Apr 18, 2022 at 7:34 AM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>> On 4/17/22 07:24, firew4lker wrote:
> 
> ...
> 
>> Linus Walleij,
>>
>> As this is backported to 5.15.y, 5.16.y, 5.17.y and those all had point
>> releases a bunch of people are hitting it now.  If you choose to adopt
>> this patch instead of revert the broken one, you can add to the commit
>> message too:
>>
>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1976
> 
> I prefer to explicitly tell that this is a link to a bug report, hence BugLink:.
> But this is just my 2 cents.

Please use "Link:" as explained by the kernel's documentation in
Documentation/process/submitting-patches.rst
Documentation/process/5.Posting.rst (disclaimer: I recently made this
more explicit, but the concept it old). That's important, as people have
tools that rely on it -- I for example run one to track regressions, but
I might not be the only one running a tool that relies on proper tags.

And FWIW: I'm all for making this more explicit, but people already use
various different tags (BugLink is just one of them) for that and that
just results in a mess. I proposed consistent tags, but that didn't get
much feedback. Maybe I should try again. Makes me wonder: where does
BugLink come from? Is that something that people are used to from
GitLab, GitHub, or something?

Ciao, Thorsten


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-18 13:58       ` Thorsten Leemhuis
@ 2022-04-18 14:17         ` Andy Shevchenko
  2022-04-18 15:29           ` Thorsten Leemhuis
  0 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2022-04-18 14:17 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: Mario Limonciello, firew4lker, Linus Walleij,
	Bartosz Golaszewski, Shreeya Patel, open list:GPIO SUBSYSTEM,
	open list, Basavaraj Natikar, Richard.Gong, Stable

On Mon, Apr 18, 2022 at 4:58 PM Thorsten Leemhuis
<regressions@leemhuis.info> wrote:
>
> On 18.04.22 13:42, Andy Shevchenko wrote:
> > On Mon, Apr 18, 2022 at 7:34 AM Mario Limonciello
> > <mario.limonciello@amd.com> wrote:
> >> On 4/17/22 07:24, firew4lker wrote:
> >
> > ...
> >
> >> Linus Walleij,
> >>
> >> As this is backported to 5.15.y, 5.16.y, 5.17.y and those all had point
> >> releases a bunch of people are hitting it now.  If you choose to adopt
> >> this patch instead of revert the broken one, you can add to the commit
> >> message too:
> >>
> >> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1976
> >
> > I prefer to explicitly tell that this is a link to a bug report, hence BugLink:.
> > But this is just my 2 cents.
>
> Please use "Link:" as explained by the kernel's documentation in
> Documentation/process/submitting-patches.rst
> Documentation/process/5.Posting.rst (disclaimer: I recently made this
> more explicit, but the concept it old). That's important, as people have
> tools that rely on it -- I for example run one to track regressions, but
> I might not be the only one running a tool that relies on proper tags.

To me it looks like a documentation confusion since Link is what is
added automatically by `b4` tool. Having Link from the patch thread
(and not always the one with the discussion) as well as link to the
issue will be confusing.

> And FWIW: I'm all for making this more explicit, but people already use
> various different tags (BugLink is just one of them) for that and that
> just results in a mess.

Nope, it results otherwise. The Link is Link to the thread, which you
may find a lot in the kernel history. Making bug report links and
links to the patch threads that's what results in a mess.

> I proposed consistent tags, but that didn't get
> much feedback. Maybe I should try again. Makes me wonder: where does
> BugLink come from? Is that something that people are used to from
> GitLab, GitHub, or something?

It comes from kernel history :-)


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-18 14:17         ` Andy Shevchenko
@ 2022-04-18 15:29           ` Thorsten Leemhuis
  0 siblings, 0 replies; 18+ messages in thread
From: Thorsten Leemhuis @ 2022-04-18 15:29 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mario Limonciello, firew4lker, Linus Walleij,
	Bartosz Golaszewski, Shreeya Patel, open list:GPIO SUBSYSTEM,
	open list, Basavaraj Natikar, Richard.Gong, Stable

On 18.04.22 16:17, Andy Shevchenko wrote:
> On Mon, Apr 18, 2022 at 4:58 PM Thorsten Leemhuis
> <regressions@leemhuis.info> wrote:
>>
>> On 18.04.22 13:42, Andy Shevchenko wrote:
>>> On Mon, Apr 18, 2022 at 7:34 AM Mario Limonciello
>>> <mario.limonciello@amd.com> wrote:
>>>> On 4/17/22 07:24, firew4lker wrote:
>>>
>>> ...
>>>
>>>> Linus Walleij,
>>>>
>>>> As this is backported to 5.15.y, 5.16.y, 5.17.y and those all had point
>>>> releases a bunch of people are hitting it now.  If you choose to adopt
>>>> this patch instead of revert the broken one, you can add to the commit
>>>> message too:
>>>>
>>>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1976
>>>
>>> I prefer to explicitly tell that this is a link to a bug report, hence BugLink:.
>>> But this is just my 2 cents.
>>
>> Please use "Link:" as explained by the kernel's documentation in
>> Documentation/process/submitting-patches.rst
>> Documentation/process/5.Posting.rst (disclaimer: I recently made this
>> more explicit, but the concept it old). That's important, as people have
>> tools that rely on it -- I for example run one to track regressions, but
>> I might not be the only one running a tool that relies on proper tags.
> 
> To me it looks like a documentation confusion since Link is what is
> added automatically by `b4` tool.

Since some time now, yes, but the "Link:" tags are way older and used to
 link to all sorts of places that are relevant.

> Having Link from the patch thread
> (and not always the one with the discussion) as well as link to the
> issue will be confusing.

Yup, but that's how it is for years already (and in the muscle memory of
some -- that's why I might make sense to teach b4 to set something else,
but that's a different story). Linus himself does it like that. Recent
commits showing that are for example 901c7280ca0d or 0313bc278dac. And
for links bug trackers, too, as 80d47f5de5e3 or 14e3e989f6a5 show.

>> And FWIW: I'm all for making this more explicit, but people already use
>> various different tags (BugLink is just one of them) for that and that
>> just results in a mess.
> Nope, it results otherwise. The Link is Link to the thread, which you
> may find a lot in the kernel history. Making bug report links and
> links to the patch threads that's what results in a mess.

Yeah, but we are in that mess already and people inventing different
tags; some of the DRM people for example use(d?) "References", but there
were others iirc.

>> I proposed consistent tags, but that didn't get
>> much feedback. Maybe I should try again. Makes me wonder: where does
>> BugLink come from? Is that something that people are used to from
>> GitLab, GitHub, or something?
> It comes from kernel history :-)

Okay, thx, had just been wondering if people are used to it from some
platform.

Ciao, Thorsten

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-14  2:57 [PATCH] gpio: Request interrupts after IRQ is initialized Mario Limonciello
                   ` (4 preceding siblings ...)
  2022-04-18 13:46 ` Thorsten Leemhuis
@ 2022-04-19  9:40 ` Samuel Čavoj
  2022-04-19 21:59 ` Linus Walleij
  6 siblings, 0 replies; 18+ messages in thread
From: Samuel Čavoj @ 2022-04-19  9:40 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Shreeya Patel, linux-gpio, linux-kernel, Basavaraj.Natikar,
	Richard.Gong, stable

Thanks,

this fixes the issue on an ASUS UM325 laptop.

On 2022-04-14 04:57, Mario Limonciello wrote:
> commit 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members 
> before
> initialization") attempted to fix a race condition that lead to a NULL
> pointer, but in the process caused a regression for _AEI/_EVT declared
> GPIOs. This manifests in messages showing deferred probing while trying
> to allocate IRQs like so:
> 
> [    0.688318] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> 0x0000 to IRQ, err -517
> [    0.688337] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> 0x002C to IRQ, err -517
> [    0.688348] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> 0x003D to IRQ, err -517
> [    0.688359] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> 0x003E to IRQ, err -517
> [    0.688369] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> 0x003A to IRQ, err -517
> [    0.688379] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> 0x003B to IRQ, err -517
> [    0.688389] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> 0x0002 to IRQ, err -517
> [    0.688399] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> 0x0011 to IRQ, err -517
> [    0.688410] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> 0x0012 to IRQ, err -517
> [    0.688420] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> 0x0007 to IRQ, err -517
> 
> The code for walking _AEI doesn't handle deferred probing and so this 
> leads
> to non-functional GPIO interrupts.
> 
> Fix this issue by moving the call to `acpi_gpiochip_request_interrupts` 
> to
> occur after gc->irc.initialized is set.
> 
> Cc: Shreeya Patel <shreeya.patel@collabora.com>
> Cc: stable@vger.kernel.org
> Fixes: 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members
> before initialization")
> Reported-by: Mario Limonciello <mario.limonciello@amd.com>
> Link:
> https://lore.kernel.org/linux-gpio/BL1PR12MB51577A77F000A008AA694675E2EF9@BL1PR12MB5157.namprd12.prod.outlook.com/T/#u
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Tested-By: Samuel Čavoj <samuel@cavoj.net>

> ---
>  drivers/gpio/gpiolib.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 085348e08986..b7694171655c 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1601,8 +1601,6 @@ static int gpiochip_add_irqchip(struct gpio_chip 
> *gc,
> 
>  	gpiochip_set_irq_hooks(gc);
> 
> -	acpi_gpiochip_request_interrupts(gc);
> -
>  	/*
>  	 * Using barrier() here to prevent compiler from reordering
>  	 * gc->irq.initialized before initialization of above
> @@ -1612,6 +1610,8 @@ static int gpiochip_add_irqchip(struct gpio_chip 
> *gc,
> 
>  	gc->irq.initialized = true;
> 
> +	acpi_gpiochip_request_interrupts(gc);
> +
>  	return 0;
>  }

Sam

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-18  4:34   ` Mario Limonciello
  2022-04-18 11:42     ` Andy Shevchenko
@ 2022-04-19 13:03     ` Limonciello, Mario
  2022-04-19 22:02     ` Linus Walleij
  2 siblings, 0 replies; 18+ messages in thread
From: Limonciello, Mario @ 2022-04-19 13:03 UTC (permalink / raw)
  To: firew4lker, Linus Walleij, Bartosz Golaszewski, Andy Shevchenko,
	Shreeya Patel, open list:GPIO SUBSYSTEM, open list
  Cc: Natikar, Basavaraj, Gong, Richard, stable

[Public]



> -----Original Message-----
> From: Limonciello, Mario
> Sent: Sunday, April 17, 2022 23:35
> To: firew4lker <firew4lker@gmail.com>; Linus Walleij
> <linus.walleij@linaro.org>; Bartosz Golaszewski <brgl@bgdev.pl>; Andy
> Shevchenko <andy.shevchenko@gmail.com>; Shreeya Patel
> <shreeya.patel@collabora.com>; open list:GPIO SUBSYSTEM <linux-
> gpio@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
> Cc: Natikar, Basavaraj <Basavaraj.Natikar@amd.com>; Gong, Richard
> <Richard.Gong@amd.com>; stable@vger.kernel.org
> Subject: Re: [PATCH] gpio: Request interrupts after IRQ is initialized
> 
> On 4/17/22 07:24, firew4lker wrote:
> > On 4/14/22 05:57, Mario Limonciello wrote:
> >> commit 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members
> >> before
> >> initialization") attempted to fix a race condition that lead to a NULL
> >> pointer, but in the process caused a regression for _AEI/_EVT declared
> >> GPIOs. This manifests in messages showing deferred probing while trying
> >> to allocate IRQs like so:
> >>
> >> [    0.688318] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> >> 0x0000 to IRQ, err -517
> >> [    0.688337] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> >> 0x002C to IRQ, err -517
> >> [    0.688348] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> >> 0x003D to IRQ, err -517
> >> [    0.688359] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> >> 0x003E to IRQ, err -517
> >> [    0.688369] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> >> 0x003A to IRQ, err -517
> >> [    0.688379] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> >> 0x003B to IRQ, err -517
> >> [    0.688389] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> >> 0x0002 to IRQ, err -517
> >> [    0.688399] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> >> 0x0011 to IRQ, err -517
> >> [    0.688410] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> >> 0x0012 to IRQ, err -517
> >> [    0.688420] amd_gpio AMDI0030:00: Failed to translate GPIO pin
> >> 0x0007 to IRQ, err -517
> >>
> >> The code for walking _AEI doesn't handle deferred probing and so this
> >> leads
> >> to non-functional GPIO interrupts.
> >>
> >> Fix this issue by moving the call to
> >> `acpi_gpiochip_request_interrupts` to
> >> occur after gc->irc.initialized is set.
> >>
> >> Cc: Shreeya Patel <shreeya.patel@collabora.com>
> >> Cc: stable@vger.kernel.org
> >> Fixes: 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members
> >> before initialization")
> >> Reported-by: Mario Limonciello <mario.limonciello@amd.com>
> >> Link:
> >>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> kernel.org%2Flinux-
> gpio%2FBL1PR12MB51577A77F000A008AA694675E2EF9%40BL1PR12MB5157.
> namprd12.prod.outlook.com%2FT%2F%23u&amp;data=04%7C01%7Cmario.li
> monciello%40amd.com%7C96ec39c78488493fd5ca08da206d3c7b%7C3dd8961
> fe4884e608e11a82d994e183d%7C0%7C0%7C637857951204650754%7CUnkno
> wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1
> haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=xZbNC%2F50JqlNwcTYAtGLn6
> z0%2FEPbfCKKOc%2BlZlMh0EQ%3D&amp;reserved=0
> >>
> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> >> ---
> >>   drivers/gpio/gpiolib.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> >> index 085348e08986..b7694171655c 100644
> >> --- a/drivers/gpio/gpiolib.c
> >> +++ b/drivers/gpio/gpiolib.c
> >> @@ -1601,8 +1601,6 @@ static int gpiochip_add_irqchip(struct gpio_chip
> >> *gc,
> >>       gpiochip_set_irq_hooks(gc);
> >> -    acpi_gpiochip_request_interrupts(gc);
> >> -
> >>       /*
> >>        * Using barrier() here to prevent compiler from reordering
> >>        * gc->irq.initialized before initialization of above
> >> @@ -1612,6 +1610,8 @@ static int gpiochip_add_irqchip(struct gpio_chip
> >> *gc,
> >>       gc->irq.initialized = true;
> >> +    acpi_gpiochip_request_interrupts(gc);
> >> +
> >>       return 0;
> >>   }
> >
> > Tested-By:firew4lker@gmail.com
> >
> > This patch addresses the issue. Tested on a Lenovo T14 with AMD Ryzen 5
> > PRO 4650U with Radeon Graphics Graphics.
> >
> > Without this patch the laptop is impossible to wake from S3 and S0ix.
> >
> 
> Thanks for testing it!
> 
> Linus Walleij,
> 
> As this is backported to 5.15.y, 5.16.y, 5.17.y and those all had point
> releases a bunch of people are hitting it now.  If you choose to adopt
> this patch instead of revert the broken one, you can add to the commit
> message too:
> 
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1976

Here's a few more links to add.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1979

Link: https://bugzilla.kernel.org/show_bug.cgi?id=215850

Thanks,

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-14  2:57 [PATCH] gpio: Request interrupts after IRQ is initialized Mario Limonciello
                   ` (5 preceding siblings ...)
  2022-04-19  9:40 ` Samuel Čavoj
@ 2022-04-19 21:59 ` Linus Walleij
  6 siblings, 0 replies; 18+ messages in thread
From: Linus Walleij @ 2022-04-19 21:59 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: Bartosz Golaszewski, Andy Shevchenko, Shreeya Patel,
	open list:GPIO SUBSYSTEM, open list, Basavaraj.Natikar,
	Richard.Gong, stable

On Thu, Apr 14, 2022 at 4:57 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:

> commit 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before
> initialization") attempted to fix a race condition that lead to a NULL
> pointer, but in the process caused a regression for _AEI/_EVT declared
> GPIOs. This manifests in messages showing deferred probing while trying
> to allocate IRQs like so:
>
> [    0.688318] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0000 to IRQ, err -517
> [    0.688337] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x002C to IRQ, err -517
> [    0.688348] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003D to IRQ, err -517
> [    0.688359] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003E to IRQ, err -517
> [    0.688369] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003A to IRQ, err -517
> [    0.688379] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x003B to IRQ, err -517
> [    0.688389] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0002 to IRQ, err -517
> [    0.688399] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0011 to IRQ, err -517
> [    0.688410] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0012 to IRQ, err -517
> [    0.688420] amd_gpio AMDI0030:00: Failed to translate GPIO pin 0x0007 to IRQ, err -517
>
> The code for walking _AEI doesn't handle deferred probing and so this leads
> to non-functional GPIO interrupts.
>
> Fix this issue by moving the call to `acpi_gpiochip_request_interrupts` to
> occur after gc->irc.initialized is set.
>
> Cc: Shreeya Patel <shreeya.patel@collabora.com>
> Cc: stable@vger.kernel.org
> Fixes: 5467801f1fcb ("gpio: Restrict usage of GPIO chip irq members before initialization")
> Reported-by: Mario Limonciello <mario.limonciello@amd.com>
> Link: https://lore.kernel.org/linux-gpio/BL1PR12MB51577A77F000A008AA694675E2EF9@BL1PR12MB5157.namprd12.prod.outlook.com/T/#u
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

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

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-18  4:34   ` Mario Limonciello
  2022-04-18 11:42     ` Andy Shevchenko
  2022-04-19 13:03     ` Limonciello, Mario
@ 2022-04-19 22:02     ` Linus Walleij
  2022-04-21 16:07       ` Thorsten Leemhuis
  2 siblings, 1 reply; 18+ messages in thread
From: Linus Walleij @ 2022-04-19 22:02 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: firew4lker, Bartosz Golaszewski, Andy Shevchenko, Shreeya Patel,
	open list:GPIO SUBSYSTEM, open list, Basavaraj.Natikar,
	Richard.Gong, stable

On Mon, Apr 18, 2022 at 6:34 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:

> Linus Walleij,
>
> As this is backported to 5.15.y, 5.16.y, 5.17.y and those all had point
> releases a bunch of people are hitting it now.  If you choose to adopt
> this patch instead of revert the broken one, you can add to the commit
> message too:
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1976

I am on parental leave kind of, but Bartosz knows what to do,
in this case, since it is ACPI-related, Andy knows best what
to do, and I see he also replied.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-19 22:02     ` Linus Walleij
@ 2022-04-21 16:07       ` Thorsten Leemhuis
  2022-04-22 12:18         ` Thorsten Leemhuis
  0 siblings, 1 reply; 18+ messages in thread
From: Thorsten Leemhuis @ 2022-04-21 16:07 UTC (permalink / raw)
  To: Linus Walleij, Mario Limonciello
  Cc: firew4lker, Bartosz Golaszewski, Andy Shevchenko, Shreeya Patel,
	open list:GPIO SUBSYSTEM, open list, Basavaraj.Natikar,
	Richard.Gong, stable, Greg KH, regressions

On 20.04.22 00:02, Linus Walleij wrote:
> On Mon, Apr 18, 2022 at 6:34 AM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
> 
>> Linus Walleij,
>>
>> As this is backported to 5.15.y, 5.16.y, 5.17.y and those all had point
>> releases a bunch of people are hitting it now.  If you choose to adopt
>> this patch instead of revert the broken one, you can add to the commit
>> message too:
>>
>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1976
> 
> I am on parental leave kind of, but Bartosz knows what to do,
> in this case, since it is ACPI-related, Andy knows best what
> to do, and I see he also replied.

Bartosz, Andy, what's the status here? It looks like the patch didn't
make any progress in the past few days (or did I miss it?). I'd really
like to see this patch or a revert of 5467801f1fcb ("gpio: Restrict
usage of GPIO chip irq members before initialization") mainlined by rc4,
so Greg (CCed) can fix it in the next round of stable updates, as it
seems quite a few people are affected by the problem.

Reminder: this is one of those issue that we IMHO really should fix
quickly, as explained by a text recently added to the documentation:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/handling-regressions.rst#n131

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-21 16:07       ` Thorsten Leemhuis
@ 2022-04-22 12:18         ` Thorsten Leemhuis
  2022-04-22 13:15           ` Limonciello, Mario
  0 siblings, 1 reply; 18+ messages in thread
From: Thorsten Leemhuis @ 2022-04-22 12:18 UTC (permalink / raw)
  To: Mario Limonciello
  Cc: firew4lker, Bartosz Golaszewski, Andy Shevchenko, Shreeya Patel,
	open list:GPIO SUBSYSTEM, open list, Basavaraj.Natikar,
	Richard.Gong, stable, Greg KH, regressions, Linus Walleij

On 21.04.22 18:07, Thorsten Leemhuis wrote:
> On 20.04.22 00:02, Linus Walleij wrote:
>> On Mon, Apr 18, 2022 at 6:34 AM Mario Limonciello
>> <mario.limonciello@amd.com> wrote:
>>
>>> Linus Walleij,
>>>
>>> As this is backported to 5.15.y, 5.16.y, 5.17.y and those all had point
>>> releases a bunch of people are hitting it now.  If you choose to adopt
>>> this patch instead of revert the broken one, you can add to the commit
>>> message too:
>>>
>>> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1976
>>
>> I am on parental leave kind of, but Bartosz knows what to do,
>> in this case, since it is ACPI-related, Andy knows best what
>> to do, and I see he also replied.
> 
> Bartosz, Andy, what's the status here? It looks like the patch didn't
> make any progress in the past few days (or did I miss it?). I'd really
> like to see this patch or a revert of 5467801f1fcb ("gpio: Restrict
> usage of GPIO chip irq members before initialization") mainlined by rc4,
> so Greg (CCed) can fix it in the next round of stable updates, as it
> seems quite a few people are affected by the problem.

Mario, are you aware if this patch made any progress towards getting
merged? If not, I wonder if we (you?) maybe should ask Linus to pick
this up directly giving the circumstances to speed things up (or maybe a
v2 that incorporates all the Reviewed-by/ACKs that accumulated).

Ciao, Thorsten

> Reminder: this is one of those issue that we IMHO really should fix
> quickly, as explained by a text recently added to the documentation:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/handling-regressions.rst#n131
> 
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> 
> P.S.: As the Linux kernel's regression tracker I deal with a lot of
> reports and sometimes miss something important when writing mails like
> this. If that's the case here, don't hesitate to tell me in a public
> reply, it's in everyone's interest to set the public record straight.
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH] gpio: Request interrupts after IRQ is initialized
  2022-04-22 12:18         ` Thorsten Leemhuis
@ 2022-04-22 13:15           ` Limonciello, Mario
  0 siblings, 0 replies; 18+ messages in thread
From: Limonciello, Mario @ 2022-04-22 13:15 UTC (permalink / raw)
  To: Thorsten Leemhuis
  Cc: firew4lker, Bartosz Golaszewski, Andy Shevchenko, Shreeya Patel,
	open list:GPIO SUBSYSTEM, open list, Natikar, Basavaraj, Gong,
	Richard, stable, Greg KH, regressions, Linus Walleij

[Public]

> -----Original Message-----
> From: Thorsten Leemhuis <regressions@leemhuis.info>
> Sent: Friday, April 22, 2022 07:19
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: firew4lker <firew4lker@gmail.com>; Bartosz Golaszewski
> <brgl@bgdev.pl>; Andy Shevchenko <andy.shevchenko@gmail.com>;
> Shreeya Patel <shreeya.patel@collabora.com>; open list:GPIO SUBSYSTEM
> <linux-gpio@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>;
> Natikar, Basavaraj <Basavaraj.Natikar@amd.com>; Gong, Richard
> <Richard.Gong@amd.com>; stable@vger.kernel.org; Greg KH
> <gregkh@linuxfoundation.org>; regressions@lists.linux.dev; Linus Walleij
> <linus.walleij@linaro.org>
> Subject: Re: [PATCH] gpio: Request interrupts after IRQ is initialized
> 
> On 21.04.22 18:07, Thorsten Leemhuis wrote:
> > On 20.04.22 00:02, Linus Walleij wrote:
> >> On Mon, Apr 18, 2022 at 6:34 AM Mario Limonciello
> >> <mario.limonciello@amd.com> wrote:
> >>
> >>> Linus Walleij,
> >>>
> >>> As this is backported to 5.15.y, 5.16.y, 5.17.y and those all had point
> >>> releases a bunch of people are hitting it now.  If you choose to adopt
> >>> this patch instead of revert the broken one, you can add to the commit
> >>> message too:
> >>>
> >>> Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
> b.freedesktop.org%2Fdrm%2Famd%2F-
> %2Fissues%2F1976&amp;data=05%7C01%7Cmario.limonciello%40amd.com%
> 7C1bfce4a8186b4b1a4ef208da245a4411%7C3dd8961fe4884e608e11a82d994e
> 183d%7C0%7C0%7C637862267399285803%7CUnknown%7CTWFpbGZsb3d8ey
> JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%
> 7C3000%7C%7C%7C&amp;sdata=cJvkTifRPkrl8kRaYffMmb0RGkoHb0krYMkj2
> Ufsg5k%3D&amp;reserved=0
> >>
> >> I am on parental leave kind of, but Bartosz knows what to do,
> >> in this case, since it is ACPI-related, Andy knows best what
> >> to do, and I see he also replied.
> >
> > Bartosz, Andy, what's the status here? It looks like the patch didn't
> > make any progress in the past few days (or did I miss it?). I'd really
> > like to see this patch or a revert of 5467801f1fcb ("gpio: Restrict
> > usage of GPIO chip irq members before initialization") mainlined by rc4,
> > so Greg (CCed) can fix it in the next round of stable updates, as it
> > seems quite a few people are affected by the problem.
> 
> Mario, are you aware if this patch made any progress towards getting
> merged? If not, I wonder if we (you?) maybe should ask Linus to pick
> this up directly giving the circumstances to speed things up (or maybe a
> v2 that incorporates all the Reviewed-by/ACKs that accumulated).

I don't see it in Bartosz or Andy's trees.
I'm fine to send up a v2 directly to Linus with this audience on CC.

> 
> Ciao, Thorsten
> 
> > Reminder: this is one of those issue that we IMHO really should fix
> > quickly, as explained by a text recently added to the documentation:
> >
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.k
> ernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%
> 2Ftree%2FDocumentation%2Fprocess%2Fhandling-
> regressions.rst%23n131&amp;data=05%7C01%7Cmario.limonciello%40amd.c
> om%7C1bfce4a8186b4b1a4ef208da245a4411%7C3dd8961fe4884e608e11a82d
> 994e183d%7C0%7C0%7C637862267399285803%7CUnknown%7CTWFpbGZsb3
> d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D%7C3000%7C%7C%7C&amp;sdata=ttOhIvDHhi8fIjrpuNcgXKeZaM%2F%2
> BaosUhn2mGEaC67M%3D&amp;reserved=0
> >
> > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> >
> > P.S.: As the Linux kernel's regression tracker I deal with a lot of
> > reports and sometimes miss something important when writing mails like
> > this. If that's the case here, don't hesitate to tell me in a public
> > reply, it's in everyone's interest to set the public record straight.
> >

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-04-22 13:15 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-14  2:57 [PATCH] gpio: Request interrupts after IRQ is initialized Mario Limonciello
2022-04-14 15:25 ` Andy Shevchenko
2022-04-14 19:11 ` Shreeya Patel
2022-04-17 12:17 ` luke
2022-04-17 12:24 ` firew4lker
2022-04-18  4:34   ` Mario Limonciello
2022-04-18 11:42     ` Andy Shevchenko
2022-04-18 13:58       ` Thorsten Leemhuis
2022-04-18 14:17         ` Andy Shevchenko
2022-04-18 15:29           ` Thorsten Leemhuis
2022-04-19 13:03     ` Limonciello, Mario
2022-04-19 22:02     ` Linus Walleij
2022-04-21 16:07       ` Thorsten Leemhuis
2022-04-22 12:18         ` Thorsten Leemhuis
2022-04-22 13:15           ` Limonciello, Mario
2022-04-18 13:46 ` Thorsten Leemhuis
2022-04-19  9:40 ` Samuel Čavoj
2022-04-19 21:59 ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.