pinctrl: amd: Add IRQF_ONESHOT to the interrupt request

Message ID 20240123180818.3994-1-mario.limonciello@amd.com
State New
Headers
Series pinctrl: amd: Add IRQF_ONESHOT to the interrupt request |

Commit Message

Mario Limonciello Jan. 23, 2024, 6:08 p.m. UTC
  On some systems the interrupt is shared between GPIO controller
and ACPI SCI. When the interrupt is shared with the ACPI SCI the
flags need to be identical.

This should fix the GPIO controller failing to work after commit
7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI").
```
[    0.417335] genirq: Flags mismatch irq 9. 00000088 (pinctrl_amd) vs. 00002080 (acpi)
[    0.420073] amd_gpio: probe of AMDI0030:00 failed with error -16
```

Cc: Rafael J. Wysocki <rafael@kernel.org>
Reported-by: Christian Heusel <christian@heusel.eu>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218407
Fixes: 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI")
Link: https://lore.kernel.org/linux-acpi/CAJZ5v0iRqUXeuKmC_+dAJtDBLWQ3x15n4gRH48y7MEaLoXF+UA@mail.gmail.com/T/#mc5506014141b61e472b24e095889535a04458083
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/pinctrl/pinctrl-amd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Rafael J. Wysocki Jan. 23, 2024, 6:20 p.m. UTC | #1
On Tue, Jan 23, 2024 at 7:08 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> On some systems the interrupt is shared between GPIO controller
> and ACPI SCI. When the interrupt is shared with the ACPI SCI the
> flags need to be identical.
>
> This should fix the GPIO controller failing to work after commit
> 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI").
> ```
> [    0.417335] genirq: Flags mismatch irq 9. 00000088 (pinctrl_amd) vs. 00002080 (acpi)
> [    0.420073] amd_gpio: probe of AMDI0030:00 failed with error -16
> ```
>
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> Reported-by: Christian Heusel <christian@heusel.eu>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218407
> Fixes: 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI")
> Link: https://lore.kernel.org/linux-acpi/CAJZ5v0iRqUXeuKmC_+dAJtDBLWQ3x15n4gRH48y7MEaLoXF+UA@mail.gmail.com/T/#mc5506014141b61e472b24e095889535a04458083
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Acked-by: Rafael J. Wysocki <rafael@kernel.org>

> ---
>  drivers/pinctrl/pinctrl-amd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
> index ca4a247c2cd1..6a33b584976c 100644
> --- a/drivers/pinctrl/pinctrl-amd.c
> +++ b/drivers/pinctrl/pinctrl-amd.c
> @@ -1162,7 +1162,7 @@ static int amd_gpio_probe(struct platform_device *pdev)
>         }
>
>         ret = devm_request_irq(&pdev->dev, gpio_dev->irq, amd_gpio_irq_handler,
> -                              IRQF_SHARED, KBUILD_MODNAME, gpio_dev);
> +                              IRQF_SHARED | IRQF_ONESHOT, KBUILD_MODNAME, gpio_dev);

Or if adding IRQF_ONESHOT here is not acceptable, IRQF_PROBE_SHARED
can be used too.

>         if (ret)
>                 goto out2;
>
> --
  
Christian Heusel Jan. 23, 2024, 7:05 p.m. UTC | #2
On 24/01/23 12:08PM, Mario Limonciello wrote:
> This should fix the GPIO controller failing to work after commit
> 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI").
> ```
> [    0.417335] genirq: Flags mismatch irq 9. 00000088 (pinctrl_amd) vs. 00002080 (acpi)
> [    0.420073] amd_gpio: probe of AMDI0030:00 failed with error -16
> ```

I have just applied this on top of 6.8-rc1 and the error in dmesg went
away, so from my perspective this fix looks good.

Tested-by: Christian Heusel <christian@heusel.eu>
  
Linux regression tracking (Thorsten Leemhuis) Jan. 26, 2024, 4:19 p.m. UTC | #3
On 23.01.24 19:08, Mario Limonciello wrote:
> On some systems the interrupt is shared between GPIO controller
> and ACPI SCI. When the interrupt is shared with the ACPI SCI the
> flags need to be identical.
> 
> This should fix the GPIO controller failing to work after commit
> 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI").
> ```
> [    0.417335] genirq: Flags mismatch irq 9. 00000088 (pinctrl_amd) vs. 00002080 (acpi)
> [    0.420073] amd_gpio: probe of AMDI0030:00 failed with error -16
> ```
> 
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> Reported-by: Christian Heusel <christian@heusel.eu>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218407
> Fixes: 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI")

I'm affected by this regression myself on my Thinkpad T14s Gen1 AMD
(power button does not work, but I guess there might be more). Haven't
tested this patch yet, seemed like this was pretty clear case and
Christian already tested it. But if it makes anyone happy I can do that.

> Link: https://lore.kernel.org/linux-acpi/CAJZ5v0iRqUXeuKmC_+dAJtDBLWQ3x15n4gRH48y7MEaLoXF+UA@mail.gmail.com/T/#mc5506014141b61e472b24e095889535a04458083

Side note: might be wise to change that to something slightly shorter:

Link:
https://lore.kernel.org/linux-acpi/kvoclxvyhmdmrfpfgwfjr33bdltej3upw5qcnazc4xakwdgg2b@krewjw2uk42k/

Ciao, Thorsten

> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/pinctrl/pinctrl-amd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
> index ca4a247c2cd1..6a33b584976c 100644
> --- a/drivers/pinctrl/pinctrl-amd.c
> +++ b/drivers/pinctrl/pinctrl-amd.c
> @@ -1162,7 +1162,7 @@ static int amd_gpio_probe(struct platform_device *pdev)
>  	}
>  
>  	ret = devm_request_irq(&pdev->dev, gpio_dev->irq, amd_gpio_irq_handler,
> -			       IRQF_SHARED, KBUILD_MODNAME, gpio_dev);
> +			       IRQF_SHARED | IRQF_ONESHOT, KBUILD_MODNAME, gpio_dev);
>  	if (ret)
>  		goto out2;
>  

P.S.: Let me add this to the tracking while at it:

#regzbot introduced 7a36b901a6eb ^
https://bugzilla.kernel.org/show_bug.cgi?id=218407
#regzbot title ACPI: OSL:/pinctrl: GPIO controller failing to work
#regzbot fix pinctrl: amd: Add IRQF_ONESHOT to the interrupt request
#regzbot ignore-activity
  
Linus Walleij Jan. 31, 2024, 9:06 a.m. UTC | #4
On Tue, Jan 23, 2024 at 7:08 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:

> On some systems the interrupt is shared between GPIO controller
> and ACPI SCI. When the interrupt is shared with the ACPI SCI the
> flags need to be identical.
>
> This should fix the GPIO controller failing to work after commit
> 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI").
> ```
> [    0.417335] genirq: Flags mismatch irq 9. 00000088 (pinctrl_amd) vs. 00002080 (acpi)
> [    0.420073] amd_gpio: probe of AMDI0030:00 failed with error -16
> ```
>
> Cc: Rafael J. Wysocki <rafael@kernel.org>
> Reported-by: Christian Heusel <christian@heusel.eu>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218407
> Fixes: 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI")
> Link: https://lore.kernel.org/linux-acpi/CAJZ5v0iRqUXeuKmC_+dAJtDBLWQ3x15n4gRH48y7MEaLoXF+UA@mail.gmail.com/T/#mc5506014141b61e472b24e095889535a04458083
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Patch applied for fixes!

Thanks for dealing with these issues Mario!

Yours,
Linus Walleij
  
Christian Heusel Feb. 4, 2024, 3:21 p.m. UTC | #5
On 24/01/23 12:08PM, Mario Limonciello wrote:
> This should fix the GPIO controller failing to work after commit
> 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI").
> ```
> [    0.417335] genirq: Flags mismatch irq 9. 00000088 (pinctrl_amd) vs. 00002080 (acpi)
> [    0.420073] amd_gpio: probe of AMDI0030:00 failed with error -16
> ```
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218407
> Fixes: 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI")

Just a friendly poke regarding this patch as the bug is still present in
rc3.

Cheers,
chris
  
Linux regression tracking (Thorsten Leemhuis) Feb. 7, 2024, 8:07 a.m. UTC | #6
On 31.01.24 10:06, Linus Walleij wrote:
> On Tue, Jan 23, 2024 at 7:08 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
> 
>> On some systems the interrupt is shared between GPIO controller
>> and ACPI SCI. When the interrupt is shared with the ACPI SCI the
>> flags need to be identical.
>>
>> This should fix the GPIO controller failing to work after commit
>> 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI").
>> ```
>> [    0.417335] genirq: Flags mismatch irq 9. 00000088 (pinctrl_amd) vs. 00002080 (acpi)
>> [    0.420073] amd_gpio: probe of AMDI0030:00 failed with error -16
>> ```
>>
>> Cc: Rafael J. Wysocki <rafael@kernel.org>
>> Reported-by: Christian Heusel <christian@heusel.eu>
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218407
>> Fixes: 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler for SCI")
>> Link: https://lore.kernel.org/linux-acpi/CAJZ5v0iRqUXeuKmC_+dAJtDBLWQ3x15n4gRH48y7MEaLoXF+UA@mail.gmail.com/T/#mc5506014141b61e472b24e095889535a04458083
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> 
> Patch applied for fixes!

Hmm, Linus, that was a week ago and I still can't spot the fix in -next
or
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git
; am I missing something or did something come up?

Furthermore Christian seems to grow impatient -- and I do as well, as I
recently couldn't wake up my laptop due to this while on the road. :-/
As at least one of the affected machines is quite popular in Linux
community, I'd prefer if we can fix this rather sooner than later.
Linus, if you are busy, I could ask the other Linus to pick this up
straight from the list if that's okay for you.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
  
Linus Walleij Feb. 7, 2024, 10:40 a.m. UTC | #7
On Wed, Feb 7, 2024 at 9:07 AM Linux regression tracking (Thorsten
Leemhuis) <regressions@leemhuis.info> wrote:

> [me]
> > Patch applied for fixes!
>
> Hmm, Linus, that was a week ago and I still can't spot the fix in -next
> or
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git
> ; am I missing something or did something come up?

Something came up but it's just illnesses travels and other such
excuses.

I have merged down fixed to for-next and the patch should appear
in linux-next and after that I can send it to Torvalds.

> Furthermore Christian seems to grow impatient -- and I do as well, as I
> recently couldn't wake up my laptop due to this while on the road. :-/

Sorry, my fault :(

I will try to get this upstream as quick as possible.

Thanks for chasing me!

Yours,
Linus Walleij
  
Thorsten Leemhuis Feb. 8, 2024, 5:45 p.m. UTC | #8
On 07.02.24 11:40, Linus Walleij wrote:
> On Wed, Feb 7, 2024 at 9:07 AM Linux regression tracking (Thorsten
> Leemhuis) <regressions@leemhuis.info> wrote:
>>> Patch applied for fixes!
>> Hmm, Linus, that was a week ago and I still can't spot the fix in -next
>> or
>> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git
>> ; am I missing something or did something come up?
> 
> Something came up but it's just illnesses travels and other such
> excuses.

Happens, no worries.

> I have merged down fixed to for-next and the patch should appear
> in linux-next and after that I can send it to Torvalds.

Great, thx! Saw the pull request, so should land soon.

Ciao, Thorsten
  

Patch

diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
index ca4a247c2cd1..6a33b584976c 100644
--- a/drivers/pinctrl/pinctrl-amd.c
+++ b/drivers/pinctrl/pinctrl-amd.c
@@ -1162,7 +1162,7 @@  static int amd_gpio_probe(struct platform_device *pdev)
 	}
 
 	ret = devm_request_irq(&pdev->dev, gpio_dev->irq, amd_gpio_irq_handler,
-			       IRQF_SHARED, KBUILD_MODNAME, gpio_dev);
+			       IRQF_SHARED | IRQF_ONESHOT, KBUILD_MODNAME, gpio_dev);
 	if (ret)
 		goto out2;