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.
> 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.
> +}
> +
> 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;
>
> --
