diff mbox series

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

Message ID 20210915130905.11196-1-hui.wang@canonical.com (mailing list archive)
State Awaiting Upstream
Delegated to: Rafael Wysocki
Headers show
Series [v3] ACPI: resources: add legacy irq override exception by DMI info | expand

Commit Message

Hui Wang Sept. 15, 2021, 1:09 p.m. UTC
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 | 49 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

Comments

Rafael J. Wysocki Sept. 15, 2021, 6:13 p.m. UTC | #1
On Wed, Sep 15, 2021 at 3:09 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 | 49 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index ee78a210c606..7bf38652e6ac 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,9 +381,51 @@ 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;
>
> @@ -401,7 +444,9 @@ 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 (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;
>
> --

Applied as 5.16 material under the subject "ACPI: resources: Add
DMI-based legacy IRQ override quirk" with some changelog edits.

Thanks!
Manuel Krause Sept. 15, 2021, 7:05 p.m. UTC | #2
On 15/09/2021 20:13, Rafael J. Wysocki wrote:
> On Wed, Sep 15, 2021 at 3:09 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 | 49 +++++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 47 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index ee78a210c606..7bf38652e6ac 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,9 +381,51 @@ 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;
>>
>> @@ -401,7 +444,9 @@ 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 (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;
>>
>> --
> 
> Applied as 5.16 material under the subject "ACPI: resources: Add
> DMI-based legacy IRQ override quirk" with some changelog edits.
> 
> Thanks!
> 

We have to thank you, Rafael, for investing your time and 
covering this issue + fix again!

I just want to add now, that also PATCH v3 works fine on here 
(with kernel v5.14.4 now).

I'm still not familiar with the kernel patch queueing mechanisms, 
so forgive my possibly bothering question:
Would it be possible (e.g. for you) to get that fix into earlier 
kernel versions? Simple reason to ask for it: Some people 
thinking over to return their newly bought machine of this type 
without proper out-of-the-box Linux support.
It shouldn't be my business, but I understand those folks' concerns.


TIA and best regards,

Manuel
Hui Wang Sept. 15, 2021, 11:04 p.m. UTC | #3
On 9/16/21 3:05 AM, Manuel Krause wrote:
> On 15/09/2021 20:13, Rafael J. Wysocki wrote:
>>
[...]
>>> +       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;
>>>
>>> -- 
>>
>> Applied as 5.16 material under the subject "ACPI: resources: Add
>> DMI-based legacy IRQ override quirk" with some changelog edits.
>>
>> Thanks!
>>
>
Thanks Rafael.
> We have to thank you, Rafael, for investing your time and covering 
> this issue + fix again!
>
> I just want to add now, that also PATCH v3 works fine on here (with 
> kernel v5.14.4 now).
>
> I'm still not familiar with the kernel patch queueing mechanisms, so 
> forgive my possibly bothering question:
> Would it be possible (e.g. for you) to get that fix into earlier 
> kernel versions? Simple reason to ask for it: Some people thinking 
> over to return their newly bought machine of this type without proper 
> out-of-the-box Linux support.
> It shouldn't be my business, but I understand those folks' concerns.
>
>
Hi Manuel,

After the patch is in the linux-next, at least I will backport the patch 
to ubuntu 20.04, 20.10 and 21.04 kernels.

Regards,

Hui.

> TIA and best regards,
>
> Manuel
diff mbox series

Patch

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index ee78a210c606..7bf38652e6ac 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,9 +381,51 @@  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;
 
@@ -401,7 +444,9 @@  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 (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;