| Message ID | 20220414025705.598-1-mario.limonciello@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: Request interrupts after IRQ is initialized | expand |
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 >
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; > } >
Tested-By: lukeluk498@gmail.com Link:
https://gitlab.freedesktop.org/drm/amd/-/issues/1976
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.
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&data=04%7C01%7Cmario.limonciello%40amd.com%7C96ec39c78488493fd5ca08da206d3c7b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637857951204650754%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=xZbNC%2F50JqlNwcTYAtGLn6z0%2FEPbfCKKOc%2BlZlMh0EQ%3D&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
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.
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; > } >
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
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 :-)
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
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
[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&data=04%7C01%7Cmario.li > monciello%40amd.com%7C96ec39c78488493fd5ca08da206d3c7b%7C3dd8961 > fe4884e608e11a82d994e183d%7C0%7C0%7C637857951204650754%7CUnkno > wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1 > haWwiLCJXVCI6Mn0%3D%7C3000&sdata=xZbNC%2F50JqlNwcTYAtGLn6 > z0%2FEPbfCKKOc%2BlZlMh0EQ%3D&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,
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
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
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.
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. >
[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&data=05%7C01%7Cmario.limonciello%40amd.com% > 7C1bfce4a8186b4b1a4ef208da245a4411%7C3dd8961fe4884e608e11a82d994e > 183d%7C0%7C0%7C637862267399285803%7CUnknown%7CTWFpbGZsb3d8ey > JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D% > 7C3000%7C%7C%7C&sdata=cJvkTifRPkrl8kRaYffMmb0RGkoHb0krYMkj2 > Ufsg5k%3D&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&data=05%7C01%7Cmario.limonciello%40amd.c > om%7C1bfce4a8186b4b1a4ef208da245a4411%7C3dd8961fe4884e608e11a82d > 994e183d%7C0%7C0%7C637862267399285803%7CUnknown%7CTWFpbGZsb3 > d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0 > %3D%7C3000%7C%7C%7C&sdata=ttOhIvDHhi8fIjrpuNcgXKeZaM%2F%2 > BaosUhn2mGEaC67M%3D&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. > >
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; }
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(-)