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

Alex Hung alex.hung at canonical.com
Tue Sep 29 19:54:19 UTC 2020


On 2020-09-29 2:03 a.m., Stefan Bader wrote:
> On 28.09.20 21: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)
> 
> This is not really what regression potential should be about[1]. And beside of
> that, my experience with any changes to EC code is that there is never a low
> chance of something going wrong.
> 
> From what I understand, all HW with general purpose events would not be handled
> differently. Assume this is the majority or is that changed by now. For the
> reduced HW platforms, how could potential problems look like? Could you imagine

Thanks to pointing out.

ACPI spec states in EC's_GPE "This method is not required on
Hardware-reduced ACPI platforms", so hardware-reduced systems probably
use different approaches. That's likely the reason to skip _GPE for
hardware-reduced systems.

> something to be worse off if acquiring the gpio interrupt fails? Sounds like the
> last patch mitigates that by always falling back to polling. Which probably
> cannot do special keys but other things...
> 
> [1] https://wiki.ubuntu.com/StableReleaseUpdates/#SRU_Bug_Template
>>
>> 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(-)
>>
> Acked-by: Stefan Bader <stefan.bader at canonical.com>
> 


-- 
Cheers,
Alex Hung



More information about the kernel-team mailing list