[PATCH 0/4][SRU][Focal] fix ACPI EC initisation failures

[Alex Hung]

On 2020-09-29 2:48 a.m., Colin Ian King wrote:
> On 28/09/2020 20:14, Alex Hung wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1896482
>>
>> [Impact]
>>
>> The bug causes the EC driver to fail and ACPI events are no longer
>> handled by Linux kernel, i.e lid close no longer triggers suspends.
>>
>> [Fix]
>>
>> Upstream commit 03e9a0e05739cf reworks ec_install_handlers to avoid
>> initialisation failures.
>>
>> The other three patches are prerequisite to apply 03e9a0e05739cf on
>> focal kernel.
>>
>> [Test]
>>
>> Tested with Lenovo ThinkPad X1 Carbon Gen 8
>>
>> [Regression Potential]
>>
>> Low. The patches were cherry-picked from mainline kernel (two since 5.5
>> and two since 5.7)
>>
>> Daniel Drake (2):
>>   ACPI: EC: tweak naming in preparation for GpioInt support
>>   ACPI: EC: add support for hardware-reduced systems
>>
>> Rafael J. Wysocki (2):
>>   ACPI: EC: Avoid passing redundant argument to functions
>>   ACPI: EC: Consolidate event handler installation code
>>
>>  drivers/acpi/ec.c       | 218 +++++++++++++++++++++++++++++++++---------------
>>  drivers/acpi/internal.h |   3 +-
>>  2 files changed, 154 insertions(+), 67 deletions(-)
>>
> 
> The first two patches have been around for a year now so have bedded in
> a bit. I checked and found the following commit fixes an issue in the
> 2nd commit:
> 
> commit c823c17a8ea4db4943152388a0beb9a556715716
> Author: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
> Date:   Thu Feb 27 22:51:36 2020 +0100
> 
>     ACPI: EC: Avoid printing confusing messages in acpi_ec_setup()
> 
> ..is this required too?


This was not included because it is about system messages but I only
included as few patches as possible.


> 
> 
> Also I see the comment in acpi_ec_add():
> 
> +       /*
> +        * Ignore the GPE value on Reduced Hardware platforms.
> +        * Some products have this set to an erroneous value.
> +        */
> 
> I guess that's the usual "firmware is defined incorrectly" problem, so
> what happens then?

EC's _GPE is used to specify which general purpose event pin is used and
thus the GPE interrupt can be enabled accordingly.

Judging by the ACPI spec (says "This method is not required on
Hardware-reduced ACPI platforms.) and the code, this mechanism is not
available on "reduced Hardware platforms", aka ARM, so I would say some
weird error messages or undefined behaviours will be observed when
enabling GPE fails.

> 
> Finally, as a side note, is this something we have checks for in fwts?

Yes a check of GPE v.s. reduced hardware can be included in fwts's
"method_test_GPE_return" in src/acpi/devices/ec/ec.c

I will submit a fwts patch later.

> 
> Colin
> 


-- 
Cheers,
Alex Hung