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

[Hui Wang <hui.wang@canonical.com>]

On 9/14/21 1:10 AM, Rafael J. Wysocki wrote:
> On Sat, Sep 4, 2021 at 3:44 AM 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
>> Reported-by: Manuel Krause <manuelkrause@netscape.net>
>> Tested-by: Manuel Krause <manuelkrause@netscape.net>
[...]
>> +                                        u8 shareable)
>> +{
>> +       int i;
>> +       const struct irq_override_cmp *en;
> This can be declared inside the for () loop.  Also, if "en" means
> "entry", please call the variable "entry".
OK, got it.
>
>> +
>> +       for (i = 0; i < ARRAY_SIZE(skip_override_table); i++) {
>> +               en = &skip_override_table[i];
>> +
>> +               if (dmi_check_system(en->system) &&
>> +                   en->irq == gsi &&
>> +                   en->triggering == triggering &&
>> +                   en->polarity == polarity &&
>> +                   en->shareable == shareable)
>> +                       return false;
>> +       }
>> +
>> +       return true;
>> +}
>> +
>>   /**
>>    * acpi_dev_resource_interrupt - Extract ACPI interrupt resource information.
>>    * @ares: Input ACPI resource object.
>> @@ -447,6 +491,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>>   {
>>          struct acpi_resource_irq *irq;
>>          struct acpi_resource_extended_irq *ext_irq;
>> +       bool is_legacy;
>>
>>          switch (ares->type) {
>>          case ACPI_RESOURCE_TYPE_IRQ:
>> @@ -459,9 +504,14 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>>                          irqresource_disabled(res, 0);
>>                          return false;
>>                  }
>> +
>> +               is_legacy = acpi_dev_legacy_irq_override(irq->interrupts[index],
>> +                                                        irq->triggering, irq->polarity,
>> +                                                        irq->shareable);
>> +
>>                  acpi_dev_get_irqresource(res, irq->interrupts[index],
>>                                           irq->triggering, irq->polarity,
>> -                                        irq->shareable, true);
>> +                                        irq->shareable, is_legacy);
> Maybe we can rename the last argument of acpi_dev_get_irqresource() to
> check_override (or similar) and do the check in there when that is
> set?

OK will change it as the suggestion.

Thanks.

>>                  break;
>>          case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
>>                  ext_irq = &ares->data.extended_irq;
>> --