From: Hans de Goede <hdegoede@redhat.com>
To: Mario Limonciello <mario.limonciello@amd.com>,
Thomas Gleixner <tglx@linutronix.de>,
kys@microsoft.com, hpa@linux.intel.com
Cc: x86@kernel.org, LKML <linux-kernel@vger.kernel.org>,
Borislav Petkov <bp@alien8.de>
Subject: Re: PIC probing code from e179f6914152 failing
Date: Fri, 20 Oct 2023 17:16:47 +0200 [thread overview]
Message-ID: <789ff693-a4f0-eef7-7991-59c031fefe49@redhat.com> (raw)
In-Reply-To: <e79dea49-0c07-4ca2-b359-97dd1bc579c8@amd.com>
Hi Mario,
On 10/19/23 23:20, Mario Limonciello wrote:
> On 10/18/2023 17:50, Thomas Gleixner wrote:
<snip>
>> But that brings up an interesting question. How are those affected
>> machines even reaching a state where the user notices that just the
>> keyboard and the GPIO are not working? Why?
>
> So the GPIO controller driver (pinctrl-amd) uses platform_get_irq() to try to discover the IRQ to use.
>
> This calls acpi_irq_get() which isn't implemented on x86 (hardcodes -EINVAL).
>
> I can "work around it" by:
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 76bfcba25003..2b4b436c65d8 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -187,7 +187,8 @@ int platform_get_irq_optional(struct platform_device *dev, unsigned int num)
> }
>
> r = platform_get_resource(dev, IORESOURCE_IRQ, num);
> - if (has_acpi_companion(&dev->dev)) {
> + if (IS_ENABLED(CONFIG_ACPI_GENERIC_GSI) &&
> + has_acpi_companion(&dev->dev)) {
> if (r && r->flags & IORESOURCE_DISABLED) {
> ret = acpi_irq_get(ACPI_HANDLE(&dev->dev), num, r);
> if (ret)
>
> but the resource that is returned from the next hunk has the resource flags set wrong in the NULL pic case:
>
> NULL case:
> r: AMDI0030:00 flags: 0x30000418
> PIC case:
> r: AMDI0030:00 flags: 0x418
>
> IOW NULL pic case has IORESOURCE_DISABLED / IORESOURCE_UNSET
>
> This then later the GPIO controller interrupts are not actually working.
> For example the attn pin for my I2C touchpad doesn't work.
Right the issue is that with the legacy-pic path disabled /
with nr_legacy_irqs() returning 0 them there is no mapping
added for the Legacy ISA IRQs which causes this problem.
My hack to set nr_legacy_irqs to 16 also for the NULL PIC from:
https://bugzilla.kernel.org/show_bug.cgi?id=218003
Does cause the Legacy ISA IRQ mappings to get added and makes
the GPIO controller actually work, as can be seen from:
https://bugzilla.kernel.org/attachment.cgi?id=305241&action=edit
Which is a dmesg with that hack and it does NOT have this error:
[ 0.276113] amd_gpio AMDI0030:00: error -EINVAL: IRQ index 0 not found
[ 0.278464] amd_gpio: probe of AMDI0030:00 failed with error -22
and the reporter also reports the touchpad works with this patch.
As Thomas already said the legayc PIC really is not necessary,
but what is still necessary on these laptops with the legacy PIC
not initialized is to have the Legacy ISA IRQ mappings added
by the kernel itself since these are missing from the MADT
(if I have my ACPI/IOAPIC terminology correct).
This quick hack (which is the one from the working dmesg)
does this:
--- a/arch/x86/kernel/i8259.c
+++ a/arch/x86/kernel/i8259.c
@@ -394,7 +394,7 @@ static int legacy_pic_probe(void)
}
struct legacy_pic null_legacy_pic = {
- .nr_legacy_irqs = 0,
+ .nr_legacy_irqs = NR_IRQS_LEGACY,
.chip = &dummy_irq_chip,
.mask = legacy_pic_uint_noop,
.unmask = legacy_pic_uint_noop,
But I believe this will break things when there are actually
non legacy ISA IRQs / GSI-s using GSI numbers < NR_IRQS_LEGACY
Thomas, I'm not at all familiar with this area of the kernel,
but would checking if the MADT defines any non ISA GSIs under
16 and if NOT use nr_legacy_irqs = NR_IRQS_LEGACY for the
NULL PIC be an option?
Or maybe some sort of DMI (sys_vendor == Lenovo) quirk to
set nr_legacy_irqs = NR_IRQS_LEGACY for the NULL PIC ?
Regards,
Hans
next prev parent reply other threads:[~2023-10-20 15:17 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-18 18:50 PIC probing code from e179f6914152 failing Mario Limonciello
2023-10-18 22:50 ` Thomas Gleixner
2023-10-19 16:39 ` David Lazăr
2023-10-19 21:20 ` Mario Limonciello
2023-10-20 3:43 ` Mario Limonciello
2023-10-20 15:16 ` Hans de Goede [this message]
2023-10-20 17:13 ` Mario Limonciello
2023-10-23 15:59 ` Thomas Gleixner
2023-10-23 16:17 ` Mario Limonciello
2023-10-23 17:50 ` Thomas Gleixner
2023-10-23 17:59 ` Mario Limonciello
2023-10-25 9:23 ` Thomas Gleixner
2023-10-25 14:41 ` Mario Limonciello
2023-10-25 15:25 ` Mario Limonciello
2023-10-25 15:25 ` David Lazar
2023-10-25 17:31 ` Thomas Gleixner
2023-10-25 17:37 ` Rafael J. Wysocki
2023-10-25 21:04 ` [PATCH] x86/i8259: Skip probing when ACPI/MADT advertises PCAT compatibility, Thomas Gleixner
2023-10-25 22:11 ` Mario Limonciello
2023-10-26 9:27 ` Re: Thomas Gleixner
2023-10-26 8:17 ` [PATCH] x86/i8259: Skip probing when ACPI/MADT advertises PCAT compatibility Hans de Goede
2023-10-26 9:39 ` [tip: x86/urgent] " tip-bot2 for Thomas Gleixner
2023-10-27 18:46 ` tip-bot2 for Thomas Gleixner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=789ff693-a4f0-eef7-7991-59c031fefe49@redhat.com \
--to=hdegoede@redhat.com \
--cc=bp@alien8.de \
--cc=hpa@linux.intel.com \
--cc=kys@microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mario.limonciello@amd.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.