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

[Hui Wang <hui.wang@xxxxxxxxxxxxx>]

On 9/14/21 1:10 AM, Rafael J. Wysocki wrote:
> On Sat, Sep 4, 2021 at 3:44 AM Hui Wang <hui.wang@xxxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> > Tested-by: Manuel Krause <manuelkrause@xxxxxxxxxxxx>
[...]
> > +                                        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;
> > --