[PATCH v2] ACPI: resources: add legacy irq override exception by DMI info

[PATCH v2] ACPI: resources: add legacy irq override exception by DMI info

[Hui Wang]

After the commit 0ec4e55e9f57 ("ACPI: resources: Add checks for ACPI
IRQ override") is reverted, the keyboard of those Medion laptops can't
work again.

To fix the keyboard issue, here adding an override check by DMI info,
this will not affect other machines and this design refers to
the prt_quirks[] in the drivers/acpi/pci_irq.c.

If we meet similar issues on other platforms, we could expand the
table of skip_override_table[] or medion_laptop[].

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213031
BugLink: http://bugs.launchpad.net/bugs/1909814
Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reported-by: Manuel Krause <manuelkrause@netscape.net>
Tested-by: Manuel Krause <manuelkrause@netscape.net>
Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 drivers/acpi/resource.c | 51 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index ee78a210c606..676254ccac4d 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -16,6 +16,7 @@
 #include <linux/ioport.h>
 #include <linux/slab.h>
 #include <linux/irq.h>
+#include <linux/dmi.h>
 
 #ifdef CONFIG_X86
 #define valid_IRQ(i) (((i) != 0) && ((i) != 2))
@@ -380,17 +381,63 @@ unsigned int acpi_dev_get_irq_type(int triggering, int polarity)
 }
 EXPORT_SYMBOL_GPL(acpi_dev_get_irq_type);
 
+static const struct dmi_system_id medion_laptop[] = {
+	{
+		.ident = "MEDION P15651",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "MEDION"),
+			DMI_MATCH(DMI_BOARD_NAME, "M15T"),
+		},
+	},
+	{ }
+};
+
+struct irq_override_cmp {
+	const struct dmi_system_id *system;
+	unsigned char irq;
+	unsigned char triggering;
+	unsigned char polarity;
+	unsigned char shareable;
+};
+
+static const struct irq_override_cmp skip_override_table[] = {
+	{ medion_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0 },
+};
+
+static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
+				  u8 shareable)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(skip_override_table); i++) {
+		const struct irq_override_cmp *entry = &skip_override_table[i];
+
+		if (dmi_check_system(entry->system) &&
+		    entry->irq == gsi &&
+		    entry->triggering == triggering &&
+		    entry->polarity == polarity &&
+		    entry->shareable == shareable)
+			return false;
+	}
+
+	return true;
+}
+
 static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
 				     u8 triggering, u8 polarity, u8 shareable,
-				     bool legacy)
+				     bool check_override)
 {
 	int irq, p, t;
+	bool override;
 
 	if (!valid_IRQ(gsi)) {
 		irqresource_disabled(res, gsi);
 		return;
 	}
 
+	override = check_override && acpi_dev_irq_override(gsi, triggering,
+							   polarity, shareable);
+
 	/*
 	 * In IO-APIC mode, use overridden attribute. Two reasons:
 	 * 1. BIOS bug in DSDT
@@ -401,7 +448,7 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
 	 * using extended IRQ descriptors we take the IRQ configuration
 	 * from _CRS directly.
 	 */
-	if (legacy && !acpi_get_override_irq(gsi, &t, &p)) {
+	if (override && !acpi_get_override_irq(gsi, &t, &p)) {
 		u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
 		u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
 
-- 
2.25.1

Re: [PATCH v2] ACPI: resources: add legacy irq override exception by DMI info

[Manuel Krause]

Hi Rafael, Hui and all others,

thank you very much for your time, thoughts and work, again!

The new [PATCH v2] works as well on here as the previous one. 
Keep my "Tested-by" attribute.

@Rafael: Do you see risks with this patch or the need for further 
improvements? If not, can you, please, push this patch towards 
kernel inclusion?

@Hui: Are you going to publish the updated patch on bugzilla or 
should I upload it? -- Many thanks for your recent quick actions!

Best regards,

Manuel


On 14/09/2021 15:34, Hui Wang wrote:
> After the commit 0ec4e55e9f57 ("ACPI: resources: Add checks for ACPI
> IRQ override") is reverted, the keyboard of those Medion laptops can't
> work again.
> 
> To fix the keyboard issue, here adding an override check by DMI info,
> this will not affect other machines and this design refers to
> the prt_quirks[] in the drivers/acpi/pci_irq.c.
> 
> If we meet similar issues on other platforms, we could expand the
> table of skip_override_table[] or medion_laptop[].
> 
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213031
> BugLink: http://bugs.launchpad.net/bugs/1909814
> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reported-by: Manuel Krause <manuelkrause@netscape.net>
> Tested-by: Manuel Krause <manuelkrause@netscape.net>
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>   drivers/acpi/resource.c | 51 +++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index ee78a210c606..676254ccac4d 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
[...]

Re: [PATCH v2] ACPI: resources: add legacy irq override exception by DMI info

[Hui Wang]

On 9/15/21 12:01 AM, Manuel Krause wrote:
> Hi Rafael, Hui and all others,
>
> thank you very much for your time, thoughts and work, again!
>
> The new [PATCH v2] works as well on here as the previous one. Keep my 
> "Tested-by" attribute.
>
> @Rafael: Do you see risks with this patch or the need for further 
> improvements? If not, can you, please, push this patch towards kernel 
> inclusion?
>
> @Hui: Are you going to publish the updated patch on bugzilla or should 
> I upload it? -- Many thanks for your recent quick actions!

OK, will do it.

Thanks,

Hui.

>
> Best regards,
>
> Manuel
>
>
> On 14/09/2021 15:34, Hui Wang wrote:
>> After the commit 0ec4e55e9f57 ("ACPI: resources: Add checks for ACPI
>> IRQ override") is reverted, the keyboard of those Medion laptops can't
>> work again.
>>
>> To fix the keyboard issue, here adding an override check by DMI info,
>> this will not affect other machines and this design refers to
>> the prt_quirks[] in the drivers/acpi/pci_irq.c.
>>
>> If we meet similar issues on other platforms, we could expand the
>> table of skip_override_table[] or medion_laptop[].
>>
>> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213031
>> BugLink: http://bugs.launchpad.net/bugs/1909814
>> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Reported-by: Manuel Krause <manuelkrause@netscape.net>
>> Tested-by: Manuel Krause <manuelkrause@netscape.net>
>> Signed-off-by: Hui Wang <hui.wang@canonical.com>
>> ---
>>   drivers/acpi/resource.c | 51 +++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 49 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index ee78a210c606..676254ccac4d 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
> [...]

Re: [PATCH v2] ACPI: resources: add legacy irq override exception by DMI info

[Rafael J. Wysocki]

On Tue, Sep 14, 2021 at 3:34 PM Hui Wang <hui.wang@canonical.com> wrote:
>
> After the commit 0ec4e55e9f57 ("ACPI: resources: Add checks for ACPI
> IRQ override") is reverted, the keyboard of those Medion laptops can't
> work again.
>
> To fix the keyboard issue, here adding an override check by DMI info,
> this will not affect other machines and this design refers to
> the prt_quirks[] in the drivers/acpi/pci_irq.c.
>
> If we meet similar issues on other platforms, we could expand the
> table of skip_override_table[] or medion_laptop[].
>
> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213031
> BugLink: http://bugs.launchpad.net/bugs/1909814
> Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reported-by: Manuel Krause <manuelkrause@netscape.net>
> Tested-by: Manuel Krause <manuelkrause@netscape.net>
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  drivers/acpi/resource.c | 51 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index ee78a210c606..676254ccac4d 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -16,6 +16,7 @@
>  #include <linux/ioport.h>
>  #include <linux/slab.h>
>  #include <linux/irq.h>
> +#include <linux/dmi.h>
>
>  #ifdef CONFIG_X86
>  #define valid_IRQ(i) (((i) != 0) && ((i) != 2))
> @@ -380,17 +381,63 @@ unsigned int acpi_dev_get_irq_type(int triggering, int polarity)
>  }
>  EXPORT_SYMBOL_GPL(acpi_dev_get_irq_type);
>
> +static const struct dmi_system_id medion_laptop[] = {
> +       {
> +               .ident = "MEDION P15651",
> +               .matches = {
> +                       DMI_MATCH(DMI_SYS_VENDOR, "MEDION"),
> +                       DMI_MATCH(DMI_BOARD_NAME, "M15T"),
> +               },
> +       },
> +       { }
> +};
> +
> +struct irq_override_cmp {
> +       const struct dmi_system_id *system;
> +       unsigned char irq;
> +       unsigned char triggering;
> +       unsigned char polarity;
> +       unsigned char shareable;
> +};
> +
> +static const struct irq_override_cmp skip_override_table[] = {
> +       { medion_laptop, 1, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_LOW, 0 },
> +};
> +
> +static bool acpi_dev_irq_override(u32 gsi, u8 triggering, u8 polarity,
> +                                 u8 shareable)
> +{
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(skip_override_table); i++) {
> +               const struct irq_override_cmp *entry = &skip_override_table[i];
> +
> +               if (dmi_check_system(entry->system) &&
> +                   entry->irq == gsi &&
> +                   entry->triggering == triggering &&
> +                   entry->polarity == polarity &&
> +                   entry->shareable == shareable)
> +                       return false;
> +       }
> +
> +       return true;
> +}
> +
>  static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>                                      u8 triggering, u8 polarity, u8 shareable,
> -                                    bool legacy)
> +                                    bool check_override)
>  {
>         int irq, p, t;
> +       bool override;
>
>         if (!valid_IRQ(gsi)) {
>                 irqresource_disabled(res, gsi);
>                 return;
>         }
>
> +       override = check_override && acpi_dev_irq_override(gsi, triggering,
> +                                                          polarity, shareable);
> +

This can be still made more straightforward.

The local bool variable is not necessary and check below can be
rearranged as follows below.

>         /*
>          * In IO-APIC mode, use overridden attribute. Two reasons:
>          * 1. BIOS bug in DSDT
> @@ -401,7 +448,7 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>          * using extended IRQ descriptors we take the IRQ configuration
>          * from _CRS directly.
>          */
> -       if (legacy && !acpi_get_override_irq(gsi, &t, &p)) {
> +       if (override && !acpi_get_override_irq(gsi, &t, &p)) {

if (check_override &&
    acpi_dev_irq_override(gsi, triggering, polarity, shareable) &&
    !acpi_get_override_irq(gsi, &t, &p)) {

>                 u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
>                 u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
>
> --
> 2.25.1
>

Re: [PATCH v2] ACPI: resources: add legacy irq override exception by DMI info

[Hui Wang]

On 9/15/21 7:53 PM, Rafael J. Wysocki wrote:
> On Tue, Sep 14, 2021 at 3:34 PM Hui Wang <hui.wang@canonical.com> wrote:
[...]
>>
>> +       override = check_override && acpi_dev_irq_override(gsi, triggering,
>> +                                                          polarity, shareable);
>> +
> This can be still made more straightforward.
>
> The local bool variable is not necessary and check below can be
> rearranged as follows below.
>
>>          /*
>>           * In IO-APIC mode, use overridden attribute. Two reasons:
>>           * 1. BIOS bug in DSDT
>> @@ -401,7 +448,7 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
>>           * using extended IRQ descriptors we take the IRQ configuration
>>           * from _CRS directly.
>>           */
>> -       if (legacy && !acpi_get_override_irq(gsi, &t, &p)) {
>> +       if (override && !acpi_get_override_irq(gsi, &t, &p)) {
> if (check_override &&
>      acpi_dev_irq_override(gsi, triggering, polarity, shareable) &&
>      !acpi_get_override_irq(gsi, &t, &p)) {

OK, got it. will change to it in the v3.

Thanks.


>>                  u8 trig = t ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
>>                  u8 pol = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
>>
>> --
>> 2.25.1
>>