Re: [PATCH] ACPI : don't always override the acpi irq

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 6/8/21 7:50 PM, Rafael J. Wysocki wrote:
On Tue, Jun 8, 2021 at 6:56 AM Hui Wang <hui.wang@xxxxxxxxxxxxx> 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 page 419 and 420 of the
ACPI_6_3_final_Jan30.pdf, if IRQ() is empty or defines IRQNoFlags,
Say "Section ... of ACPI 6.3" instead of referring directly to a PDF file.

And if you refer to ACPI 6.4 instead, you may use a Link tag to point
to the relevant section in the HTML format of the spec.
OK, go it.
the IRQ is High true, edge sensitive and non-shareable. The linux
ACPI driver (acpi_rs_set_irq[] in rsirq.c) also assumes so.

So here add a function to check 4 conditions, if all of them are true,
call override function. otherwise, it means IRQ descriptior in the
BIOS is not legacy or is not empty.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213031
BugLink: http://bugs.launchpad.net/bugs/1909814
Reported-and-tested-by: Manuel Krause <manuelkrause@xxxxxxxxxxxx>
Signed-off-by: Hui Wang <hui.wang@xxxxxxxxxxxxx>
---
  drivers/acpi/resource.c | 13 ++++++++++++-
  1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index ee78a210c606..d346aa24ffd6 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -380,6 +380,16 @@ unsigned int acpi_dev_get_irq_type(int triggering, int polarity)
  }
  EXPORT_SYMBOL_GPL(acpi_dev_get_irq_type);

+static bool acpi_dev_irq_empty_or_noflags(bool legacy, u8 triggering, u8 polarity,
+                                         u8 shareable)
+{
+       if (legacy && (triggering == ACPI_EDGE_SENSITIVE) &&
+           (polarity == ACPI_ACTIVE_HIGH) && (shareable == ACPI_EXCLUSIVE))
+               return true;
+       else
+               return false;
Because the function returns bool, you can do

   return legacy && triggering == ACPI_EDGE_SENSITIVE && polarity ==
ACPI_ACTIVE_HIGH && shareable == ACPI_EXCLUSIVE;

Also I'm not sure why a new function is needed for this at all, as the
check can be done in-line below just fine.

Moreover, as it stands, the only purpose of the "legacy" argument of
acpi_dev_get_irqresource() is whether or not to do the override, so
the triggering/polarity/shareable check can be used to determine the
value of "legacy" when calling that function from
acpi_dev_resource_interrupt() in the ACPI_RESOURCE_TYPE_IRQ case.

Got it, will do a change like that.

Thanks.


+}
+
  static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
                                      u8 triggering, u8 polarity, u8 shareable,
                                      bool legacy)
@@ -401,7 +411,8 @@ 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 (acpi_dev_irq_empty_or_noflags(legacy, 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;

--



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux