| Message ID | 20210608140701.17938-1-hui.wang@canonical.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v2] ACPI : don't always override the acpi irq | expand |
On Tue, Jun 8, 2021 at 4:07 PM Hui Wang <hui.wang@canonical.com> wrote: > > The laptop keyboard doesn't work on many MEDION notebooks, but the > keyboard works well under Windows and Unix. > > Through debugging, we found this log in the dmesg: > ACPI: IRQ 1 override to edge, high > pnp 00:03: Plug and Play ACPI device, IDs PNP0303 (active) > > And we checked the IRQ definition in the DSDT, it is: > IRQ (Level, ActiveLow, Exclusive, ) > {1} > > So the BIOS defines the keyboard irq to Level_Low, but the linux > kernel override it to Edge_High. If let the linux kernel skip the irq > override, the keyboard will work normally. > > From the existing comment in the acpi_dev_get_irqresource(), the > override function only needs to be called when BIOS defines IRQ() or > IRQNoFlags, and according to the Section 6.4.2.1 of ACPI 6.4 spec [1], > if IRQ() is empty or defines IRQNoFlags, the IRQ is High true, edge > sensitive and non-shareable. The linux ACPI driver (acpi_rs_set_irq[] > in rsirq.c) also assumes so. > > Here check 3 conditions to decide if the legacy is true or not, if it > is true, it means the IRQ() is empty or defines IRQNoFlags and need to > call irq_override(). > > Link: https://uefi.org/specs/ACPI/6.4/06_Device_Configuration/Device_Configuration.html#irq-descriptor # [1] > 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-and-tested-by: Manuel Krause <manuelkrause@netscape.net> > Signed-off-by: Hui Wang <hui.wang@canonical.com> > --- > drivers/acpi/resource.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c > index ee78a210c606..5e4341ca6790 100644 > --- a/drivers/acpi/resource.c > +++ b/drivers/acpi/resource.c > @@ -447,6 +447,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 irq_legacy; > > switch (ares->type) { > case ACPI_RESOURCE_TYPE_IRQ: > @@ -459,9 +460,13 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index, > irqresource_disabled(res, 0); > return false; > } > + > + irq_legacy = (irq->triggering == ACPI_EDGE_SENSITIVE && irq->polarity == > + ACPI_ACTIVE_HIGH && irq->shareable == ACPI_EXCLUSIVE); Now it would make sense to use a wrapper function for this: static bool irq_is_legacy(struct acpi_resource_irq *irq) { return irq->triggering == ACPI_EDGE_SENSITIVE && irq->polarity == ACPI_ACTIVE_HIGH && irq->shareable == ACPI_EXCLUSIVE; } > + > acpi_dev_get_irqresource(res, irq->interrupts[index], > irq->triggering, irq->polarity, > - irq->shareable, true); > + irq->shareable, irq_legacy); + irq->shareable, irq_is_legacy(irq)); > break; > case ACPI_RESOURCE_TYPE_EXTENDED_IRQ: > ext_irq = &ares->data.extended_irq; > --
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c index ee78a210c606..5e4341ca6790 100644 --- a/drivers/acpi/resource.c +++ b/drivers/acpi/resource.c @@ -447,6 +447,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 irq_legacy; switch (ares->type) { case ACPI_RESOURCE_TYPE_IRQ: @@ -459,9 +460,13 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index, irqresource_disabled(res, 0); return false; } + + irq_legacy = (irq->triggering == ACPI_EDGE_SENSITIVE && irq->polarity == + ACPI_ACTIVE_HIGH && irq->shareable == ACPI_EXCLUSIVE); + acpi_dev_get_irqresource(res, irq->interrupts[index], irq->triggering, irq->polarity, - irq->shareable, true); + irq->shareable, irq_legacy); break; case ACPI_RESOURCE_TYPE_EXTENDED_IRQ: ext_irq = &ares->data.extended_irq;
The laptop keyboard doesn't work on many MEDION notebooks, but the keyboard works well under Windows and Unix. Through debugging, we found this log in the dmesg: ACPI: IRQ 1 override to edge, high pnp 00:03: Plug and Play ACPI device, IDs PNP0303 (active) And we checked the IRQ definition in the DSDT, it is: IRQ (Level, ActiveLow, Exclusive, ) {1} So the BIOS defines the keyboard irq to Level_Low, but the linux kernel override it to Edge_High. If let the linux kernel skip the irq override, the keyboard will work normally. From the existing comment in the acpi_dev_get_irqresource(), the override function only needs to be called when BIOS defines IRQ() or IRQNoFlags, and according to the Section 6.4.2.1 of ACPI 6.4 spec [1], if IRQ() is empty or defines IRQNoFlags, the IRQ is High true, edge sensitive and non-shareable. The linux ACPI driver (acpi_rs_set_irq[] in rsirq.c) also assumes so. Here check 3 conditions to decide if the legacy is true or not, if it is true, it means the IRQ() is empty or defines IRQNoFlags and need to call irq_override(). Link: https://uefi.org/specs/ACPI/6.4/06_Device_Configuration/Device_Configuration.html#irq-descriptor # [1] 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-and-tested-by: Manuel Krause <manuelkrause@netscape.net> Signed-off-by: Hui Wang <hui.wang@canonical.com> --- drivers/acpi/resource.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)