[PATCH v2 0/1] Fix regression in 5.18 for GPIO

[PATCH v2 0/1] Fix regression in 5.18 for GPIO

[Mario Limonciello]

Linus,

This patch is being sent directly to you because there has been
a regression in 5.18 that I identified and sent a fix up that has been
reviewed/tested/acked for nearly a week but the current subsystem
maintainer (Bartosz) hasn't picked it up to send to you.

It's a severe problem; anyone who hits it:
1) Power button doesn't work anymore
2) Can't resume their laptop from S3 or s2idle

Because the original patch was cc stable@, it landed in stable releases
and has been breaking people left and right as distros track the stable
channels.  The patch is well tested. Would you please consider to pick
this up directly to fix that regression?

Thanks,

Mario Limonciello (1):
  gpio: Request interrupts after IRQ is initialized

 drivers/gpio/gpiolib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH v2 1/1] gpio: Request interrupts after IRQ is initialized

[Mario Limonciello]

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>
Reviewed-by: Shreeya Patel <shreeya.patel@collabora.com>
Tested-By: Samuel Čavoj <samuel@cavoj.net>
Tested-By: lukeluk498@gmail.com Link:
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1979
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215850
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1976
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
v1->v2:
 * Pick up acked-by/reviewed-by/link/tested-by

 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

Re: [PATCH v2 1/1] gpio: Request interrupts after IRQ is initialized

[Limonciello, Mario]

On 4/22/2022 08:14, 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>
> Reviewed-by: Shreeya Patel <shreeya.patel@collabora.com>
> Tested-By: Samuel Čavoj <samuel@cavoj.net>
> Tested-By: lukeluk498@gmail.com Link:
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1979
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215850
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1976
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>

Takashi hit this as well recently and provided a few more tags.

BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1198697
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Tested-by: Takashi Iwai <tiwai@suse.de>


> ---
> v1->v2:
>   * Pick up acked-by/reviewed-by/link/tested-by
> 
>   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;
>   }
>   

Re: [PATCH v2 0/1] Fix regression in 5.18 for GPIO

[Linus Torvalds]

On Fri, Apr 22, 2022 at 6:15 AM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> This patch is being sent directly to you because there has been
> a regression in 5.18 that I identified and sent a fix up that has been
> reviewed/tested/acked for nearly a week but the current subsystem
> maintainer (Bartosz) hasn't picked it up to send to you.

Applied.

I'm not sure the "cc: stable" makes much sense since the bug was
introduced in this release, but I assume you added it because the
problem commit was also marked for stable.

The "Fixes:" tag should take care of it, but I left that cc:stable alone.

           Linus

Re: [PATCH v2 0/1] Fix regression in 5.18 for GPIO

[Bartosz Golaszewski]

On Fri, Apr 22, 2022 at 3:15 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> Linus,
>
> This patch is being sent directly to you because there has been
> a regression in 5.18 that I identified and sent a fix up that has been
> reviewed/tested/acked for nearly a week but the current subsystem
> maintainer (Bartosz) hasn't picked it up to send to you.
>

Hi Mario!

I don't have any previous submission in my inbox. Are you sure to have
used my current address (brgl@bgdev.pl)?

Bart

> It's a severe problem; anyone who hits it:
> 1) Power button doesn't work anymore
> 2) Can't resume their laptop from S3 or s2idle
>
> Because the original patch was cc stable@, it landed in stable releases
> and has been breaking people left and right as distros track the stable
> channels.  The patch is well tested. Would you please consider to pick
> this up directly to fix that regression?
>
> Thanks,
>
> Mario Limonciello (1):
>   gpio: Request interrupts after IRQ is initialized
>
>  drivers/gpio/gpiolib.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> --
> 2.34.1
>

Re: [PATCH v2 0/1] Fix regression in 5.18 for GPIO

[Bartosz Golaszewski]

On Mon, Apr 25, 2022 at 8:41 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> On Fri, Apr 22, 2022 at 3:15 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
> >
> > Linus,
> >
> > This patch is being sent directly to you because there has been
> > a regression in 5.18 that I identified and sent a fix up that has been
> > reviewed/tested/acked for nearly a week but the current subsystem
> > maintainer (Bartosz) hasn't picked it up to send to you.
> >
>
> Hi Mario!
>
> I don't have any previous submission in my inbox. Are you sure to have
> used my current address (brgl@bgdev.pl)?
>

Nevermind, found it in spam. Sorry, this sometimes happens in gmail.

Anyway - it's only  been 3 days and I've been travelling. Sometimes
reviews take a couple days.

Bart

> Bart
>
> > It's a severe problem; anyone who hits it:
> > 1) Power button doesn't work anymore
> > 2) Can't resume their laptop from S3 or s2idle
> >
> > Because the original patch was cc stable@, it landed in stable releases
> > and has been breaking people left and right as distros track the stable
> > channels.  The patch is well tested. Would you please consider to pick
> > this up directly to fix that regression?
> >
> > Thanks,
> >
> > Mario Limonciello (1):
> >   gpio: Request interrupts after IRQ is initialized
> >
> >  drivers/gpio/gpiolib.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > --
> > 2.34.1
> >

RE: [PATCH v2 0/1] Fix regression in 5.18 for GPIO

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Bartosz Golaszewski <brgl@bgdev.pl>
> Sent: Monday, April 25, 2022 13:43
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>; Natikar, Basavaraj
> <Basavaraj.Natikar@amd.com>; Gong, Richard <Richard.Gong@amd.com>;
> regressions@lists.linux.dev; Thorsten Leemhuis
> <regressions@leemhuis.info>; Greg KH <gregkh@linuxfoundation.org>;
> stable <stable@vger.kernel.org>; Linus Walleij <linus.walleij@linaro.org>;
> Shreeya Patel <shreeya.patel@collabora.com>; Andy Shevchenko
> <andy.shevchenko@gmail.com>; open list:GPIO SUBSYSTEM <linux-
> gpio@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH v2 0/1] Fix regression in 5.18 for GPIO
> 
> On Mon, Apr 25, 2022 at 8:41 PM Bartosz Golaszewski <brgl@bgdev.pl>
> wrote:
> >
> > On Fri, Apr 22, 2022 at 3:15 PM Mario Limonciello
> > <mario.limonciello@amd.com> wrote:
> > >
> > > Linus,
> > >
> > > This patch is being sent directly to you because there has been
> > > a regression in 5.18 that I identified and sent a fix up that has been
> > > reviewed/tested/acked for nearly a week but the current subsystem
> > > maintainer (Bartosz) hasn't picked it up to send to you.
> > >
> >
> > Hi Mario!
> >
> > I don't have any previous submission in my inbox. Are you sure to have
> > used my current address (brgl@bgdev.pl)?
> >
> 
> Nevermind, found it in spam. Sorry, this sometimes happens in gmail.

OK glad you found it.

> 
> Anyway - it's only  been 3 days and I've been travelling. Sometimes
> reviews take a couple days.

If it was just in the normal development release in an RC I'd agree there wasn't
a lot of urgency, but stable picked it up and caused severe regressions.  There
wasn't an obvious willingness to revert the problematic commit in stable is why
Thorsten was making noise about it and suggested me to send it directly to Linus.

Anyway - I'm glad it's sorted now.

> 
> Bart
> 
> > Bart
> >
> > > It's a severe problem; anyone who hits it:
> > > 1) Power button doesn't work anymore
> > > 2) Can't resume their laptop from S3 or s2idle
> > >
> > > Because the original patch was cc stable@, it landed in stable releases
> > > and has been breaking people left and right as distros track the stable
> > > channels.  The patch is well tested. Would you please consider to pick
> > > this up directly to fix that regression?
> > >
> > > Thanks,
> > >
> > > Mario Limonciello (1):
> > >   gpio: Request interrupts after IRQ is initialized
> > >
> > >  drivers/gpio/gpiolib.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > --
> > > 2.34.1
> > >