"EC: Install address space handler at the namespace root" causing issues for some users

"EC: Install address space handler at the namespace root" causing issues for some users

[Hans de Goede]

Hi Rafael,

There are 2 bug reports:

1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/

2. EC related ACPI errors and bad performance:
https://bugzilla.redhat.com/show_bug.cgi?id=2298938

Both of which started with 6.9.7 which has the 2 commits related to "EC: Install
address space handler at the namespace root" from 6.10 backported:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/acpi?h=v6.9.7&id=2b2b0ac1533d790690d6d28899f01a2924d54d4d
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/acpi?h=v6.9.7&id=9750135f2f326879889ed60ccd68b96572dfa6ee

i have build a test 6.9.9 kernel with these 2 reverted and 1. is confirmed to be fixed
by reverting these 2 commits. Although the user does report an IRQ storm on the ACPI IRQ
(IRQ 9) related to thunderbolt after this.

I have not yet got confirmation that the second bug is also resolved by the commits.

Either way it looks like we need to dig into this and figure out what is causing
these EC related regressions.

Regards,

Hans

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Hans de Goede]

Hi,

On 7/29/24 1:15 PM, Hans de Goede wrote:
> Hi Rafael,
> 
> There are 2 bug reports:
> 
> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
> 
> 2. EC related ACPI errors and bad performance:
> https://bugzilla.redhat.com/show_bug.cgi?id=2298938
> 
> Both of which started with 6.9.7 which has the 2 commits related to "EC: Install
> address space handler at the namespace root" from 6.10 backported:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/acpi?h=v6.9.7&id=2b2b0ac1533d790690d6d28899f01a2924d54d4d
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/acpi?h=v6.9.7&id=9750135f2f326879889ed60ccd68b96572dfa6ee
> 
> i have build a test 6.9.9 kernel with these 2 reverted and 1. is confirmed to be fixed
> by reverting these 2 commits. Although the user does report an IRQ storm on the ACPI IRQ
> (IRQ 9) related to thunderbolt after this.
> 
> I have not yet got confirmation that the second bug is also resolved by the commits.

... resolved by *reverting* the commits.

> Either way it looks like we need to dig into this and figure out what is causing
> these EC related regressions.

I'm wondering if maybe this is also somewhat related to commit e2ffcda16290 ("ACPI: OSL:
Allow Notify () handlers to run on all CPUs") ?

I guess not though since this started in 6.9.7 and 6.9.6 is fine.

I did recently submit an unrelated fix caused by e2ffcda16290:

https://lore.kernel.org/platform-driver-x86/20240729110030.8016-1-hdegoede@redhat.com/

Regards,

Hans

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Rafael J. Wysocki]

Hi Hans,

On Mon, Jul 29, 2024 at 1:29 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 7/29/24 1:15 PM, Hans de Goede wrote:
> > Hi Rafael,
> >
> > There are 2 bug reports:
> >
> > 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
> > https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
> >
> > 2. EC related ACPI errors and bad performance:
> > https://bugzilla.redhat.com/show_bug.cgi?id=2298938
> >
> > Both of which started with 6.9.7 which has the 2 commits related to "EC: Install
> > address space handler at the namespace root" from 6.10 backported:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/acpi?h=v6.9.7&id=2b2b0ac1533d790690d6d28899f01a2924d54d4d
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/acpi?h=v6.9.7&id=9750135f2f326879889ed60ccd68b96572dfa6ee
> >
> > i have build a test 6.9.9 kernel with these 2 reverted and 1. is confirmed to be fixed
> > by reverting these 2 commits. Although the user does report an IRQ storm on the ACPI IRQ
> > (IRQ 9) related to thunderbolt after this.
> >
> > I have not yet got confirmation that the second bug is also resolved by the commits.
>
> ... resolved by *reverting* the commits.
>
> > Either way it looks like we need to dig into this and figure out what is causing
> > these EC related regressions.

Right, so I looked at the dmesg output in 2. and saw that the EC
errors were reported right after enabling the EC for the first time in
acpi_ec_dsdt_probe().

Because acpi_ec_dsdt_probe() passes true as the last argument to
acpi_ec_setup(), it will evaluate _REG everywhere at this point, but
previously it only evaluated _REG in the EC scope.

In the ECDT case, the _REG evaluation is deferred until the EC has
been found in the namespace, so maybe that's the right time to
evaluate EC opregions _REG in general.

So one thing to try may be to pass "false" to acpi_ec_setup() in
acpi_ec_dsdt_probe().

> I'm wondering if maybe this is also somewhat related to commit e2ffcda16290 ("ACPI: OSL:
> Allow Notify () handlers to run on all CPUs") ?
>
> I guess not though since this started in 6.9.7 and 6.9.6 is fine.
>
> I did recently submit an unrelated fix caused by e2ffcda16290:
>
> https://lore.kernel.org/platform-driver-x86/20240729110030.8016-1-hdegoede@redhat.com/

None of the above look like they are processor-related, though.

Thanks!

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 2384 bytes --]

Hi Rafael,

On 7/29/24 2:20 PM, Rafael J. Wysocki wrote:
> Hi Hans,
> 
> On Mon, Jul 29, 2024 at 1:29 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 7/29/24 1:15 PM, Hans de Goede wrote:
>>> Hi Rafael,
>>>
>>> There are 2 bug reports:
>>>
>>> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
>>> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
>>>
>>> 2. EC related ACPI errors and bad performance:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=2298938
>>>
>>> Both of which started with 6.9.7 which has the 2 commits related to "EC: Install
>>> address space handler at the namespace root" from 6.10 backported:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/acpi?h=v6.9.7&id=2b2b0ac1533d790690d6d28899f01a2924d54d4d
>>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/acpi?h=v6.9.7&id=9750135f2f326879889ed60ccd68b96572dfa6ee
>>>
>>> i have build a test 6.9.9 kernel with these 2 reverted and 1. is confirmed to be fixed
>>> by reverting these 2 commits. Although the user does report an IRQ storm on the ACPI IRQ
>>> (IRQ 9) related to thunderbolt after this.
>>>
>>> I have not yet got confirmation that the second bug is also resolved by the commits.
>>
>> ... resolved by *reverting* the commits.
>>
>>> Either way it looks like we need to dig into this and figure out what is causing
>>> these EC related regressions.
> 
> Right, so I looked at the dmesg output in 2. and saw that the EC
> errors were reported right after enabling the EC for the first time in
> acpi_ec_dsdt_probe().
> 
> Because acpi_ec_dsdt_probe() passes true as the last argument to
> acpi_ec_setup(), it will evaluate _REG everywhere at this point, but
> previously it only evaluated _REG in the EC scope.
> 
> In the ECDT case, the _REG evaluation is deferred until the EC has
> been found in the namespace, so maybe that's the right time to
> evaluate EC opregions _REG in general.
> 
> So one thing to try may be to pass "false" to acpi_ec_setup() in
> acpi_ec_dsdt_probe().

That is a good idea. I have written the attached patch for this and done
a Fedora kernel test build with that patch added for the reporter of:

https://bugzilla.redhat.com/show_bug.cgi?id=2298938

To test.

Regards,

Hans


[-- Attachment #2: 0001-ACPI-EC-Delay-running-_REG-handlers-until-the-EC-acp.patch --]
[-- Type: text/x-patch, Size: 2905 bytes --]

From ecaed52d8eaa6a302ed8c216598a58ca45f3c148 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Thu, 1 Aug 2024 11:27:54 +0200
Subject: [PATCH] ACPI: EC: Delay running _REG handlers until the EC
 acpi_device is enumerated

Since commit 60fa6ae6e6d0 ("ACPI: EC: Install address space handler at
the namespace root") on systems without an ECDT any EC _REG handlers are
now run as soon as acpi_ec_dsdt_probe() runs, where as before only
the _REG handler under the EC acpi_device was run.

This is causing EC related ACPI errors and thermalzone issues on Acer
Aspire ES1-572 laptops:

jul 19 17:33:41 kernel: ACPI: EC: EC started
jul 19 17:33:41 kernel: ACPI: EC: interrupt blocked
jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC.ECMD due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FNCL due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FN00._OFF due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC._REG due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
jul 19 17:33:41 kernel: ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
jul 19 17:33:41 kernel: ACPI: \_SB_.PCI0.LPCB.EC0_: Boot DSDT EC used to handle transactions
jul 19 17:33:41 kernel: ACPI: Interpreter enabled

For ECDT declared ECs the calling of _REG is delayed till the acpi_device
for the EC gets enumerated (till acpi_ec_add() is called). Make the path
for ECs which are discovered early on through the DSDT consistent with this
and all also delay the calling of _REG till acpi_device enumeration time
there.

This fixes the mentioned ACPI EC errors and thermalzone issues.

Fixes: 60fa6ae6e6d0 ("ACPI: EC: Install address space handler at the namespace root")
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2298938
Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/ec.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 299ec653388c..8929f9215b70 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1788,13 +1788,12 @@ void __init acpi_ec_dsdt_probe(void)
 	}
 
 	/*
-	 * When the DSDT EC is available, always re-configure boot EC to
-	 * have _REG evaluated. _REG can only be evaluated after the
-	 * namespace initialization.
+	 * Delay _REG evaluation until the EC is found by regular ACPI device
+	 * hierarchy parsing and acpi_ec_add() is called for the EC.
 	 * At this point, the GPE is not fully initialized, so do not to
 	 * handle the events.
 	 */
-	ret = acpi_ec_setup(ec, NULL, true);
+	ret = acpi_ec_setup(ec, NULL, false);
 	if (ret) {
 		acpi_ec_free(ec);
 		return;
-- 
2.45.2

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Hans de Goede]

Hi,

On 7/29/24 1:15 PM, Hans de Goede wrote:
> Hi Rafael,
> 
> There are 2 bug reports:
> 
> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/

I have filed:

https://bugzilla.redhat.com/show_bug.cgi?id=2302253

to track this now and an acpidump of the troublesome LG laptop
is attached there. I have also requested dmesg output of
a non working kernel to be attached there.

As a reminder this is the bug where it has been confirmed that
reverting "EC: Install address space handler at the namespace root"
helps, with the caveat that there is a Thunderbolt related IRQ
storm on the ACPI event IRQ after the revert ...

Regards,

Hans

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Hans de Goede]

Hi,

On 8/1/24 4:28 PM, Hans de Goede wrote:
> Hi,
> 
> On 7/29/24 1:15 PM, Hans de Goede wrote:
>> Hi Rafael,
>>
>> There are 2 bug reports:
>>
>> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
>> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
> 
> I have filed:
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=2302253
> 
> to track this now and an acpidump of the troublesome LG laptop
> is attached there. I have also requested dmesg output of
> a non working kernel to be attached there.
> 
> As a reminder this is the bug where it has been confirmed that
> reverting "EC: Install address space handler at the namespace root"
> helps, with the caveat that there is a Thunderbolt related IRQ
> storm on the ACPI event IRQ after the revert ...

Ok, so the bugzilla now has 2 different dmesg outputs:

1. 6.9.6, this kernel works without problems

2. 6.9.12 with the following patch you suggested on top:

--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1788,7 +1788,7 @@ void __init acpi_ec_dsdt_probe(void)
 	 * At this point, the GPE is not fully initialized, so do not to
 	 * handle the events.
 	 */
-	ret = acpi_ec_setup(ec, NULL, true);
+	ret = acpi_ec_setup(ec, NULL, false);
 	if (ret) {
 		acpi_ec_free(ec);
 		return;

Unfortunately this does not help. dmesg shows some EC _REG errors, which
are now (with the above diff applied) shown just before the
"Boot DSDT EC initialization complete" message, which shows that _REG now
runs from acpi_ec_add() rather then before:

[    1.007566] ACPI BIOS Error (bug): Could not resolve symbol [\_TZ.FN00._OFF], AE_NOT_FOUND (20230628/psargs-330)
[    1.007576] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC.EREG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
[    1.007580] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC._REG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
[    1.007639] ACPI: EC: interrupt unblocked
[    1.007640] ACPI: EC: event unblocked
[    1.007675] ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
[    1.007676] ACPI: EC: GPE=0x6e
[    1.007677] ACPI: \_SB_.PC00.LPCB.LGEC: Boot DSDT EC initialization complete
[    1.007679] ACPI: \_SB_.PC00.LPCB.LGEC: EC: Used to handle transactions and events

Note that the errors are from calling _REG on \_SB.PC00.LPCB.H_EC, where as the actual
EC (and the only acpi_device on which _REG would get called for the EC Opregion before) is:
\_SB_.PC00.LPCB.LGEC.

Looking at the DSDT it seems that the H_EC is not used and is leftover from a copy/paste
from some reference design DSDT. Its _REG however does write to the EC before hitting the error
and I think that that write may be causing the issue...

The H_EC device does have an _STA method and looking closer the troublesome EREG method is
also called from _INI. So I guess that _STA is returning 0 causing _INI to not run and
that is the reason why we are not seeing the same EREG errors with kernel 6.9.6 where _REG is
only called for the EC opregion on \_SB_.PC00.LPCB.LGEC and not for the entire ACPI device
hierarchy as is done with >= 6.9.7 .

Maybe we should only call _REG for the EC opregion on present devices (and devices without
a _STA)?

Also note that both LGEC and H_EC use the same cmd + data ports.

I'll go and ask the reporter to retreive the status of both LGEC and H_EC and then see
from there.

Regards,

Hans

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Hans de Goede]

Hi,

On 8/5/24 1:28 PM, Hans de Goede wrote:
> Hi,
> 
> On 8/1/24 4:28 PM, Hans de Goede wrote:
>> Hi,
>>
>> On 7/29/24 1:15 PM, Hans de Goede wrote:
>>> Hi Rafael,
>>>
>>> There are 2 bug reports:
>>>
>>> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
>>> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
>>
>> I have filed:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=2302253
>>
>> to track this now and an acpidump of the troublesome LG laptop
>> is attached there. I have also requested dmesg output of
>> a non working kernel to be attached there.
>>
>> As a reminder this is the bug where it has been confirmed that
>> reverting "EC: Install address space handler at the namespace root"
>> helps, with the caveat that there is a Thunderbolt related IRQ
>> storm on the ACPI event IRQ after the revert ...
>  
> Ok, so the bugzilla now has 2 different dmesg outputs:
> 
> 1. 6.9.6, this kernel works without problems
> 
> 2. 6.9.12 with the following patch you suggested on top:
> 
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -1788,7 +1788,7 @@ void __init acpi_ec_dsdt_probe(void)
>  	 * At this point, the GPE is not fully initialized, so do not to
>  	 * handle the events.
>  	 */
> -	ret = acpi_ec_setup(ec, NULL, true);
> +	ret = acpi_ec_setup(ec, NULL, false);
>  	if (ret) {
>  		acpi_ec_free(ec);
>  		return;
> 
> Unfortunately this does not help. dmesg shows some EC _REG errors, which
> are now (with the above diff applied) shown just before the
> "Boot DSDT EC initialization complete" message, which shows that _REG now
> runs from acpi_ec_add() rather then before:
> 
> [    1.007566] ACPI BIOS Error (bug): Could not resolve symbol [\_TZ.FN00._OFF], AE_NOT_FOUND (20230628/psargs-330)
> [    1.007576] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC.EREG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
> [    1.007580] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC._REG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
> [    1.007639] ACPI: EC: interrupt unblocked
> [    1.007640] ACPI: EC: event unblocked
> [    1.007675] ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
> [    1.007676] ACPI: EC: GPE=0x6e
> [    1.007677] ACPI: \_SB_.PC00.LPCB.LGEC: Boot DSDT EC initialization complete
> [    1.007679] ACPI: \_SB_.PC00.LPCB.LGEC: EC: Used to handle transactions and events
> 
> Note that the errors are from calling _REG on \_SB.PC00.LPCB.H_EC, where as the actual
> EC (and the only acpi_device on which _REG would get called for the EC Opregion before) is:
> \_SB_.PC00.LPCB.LGEC.
> 
> Looking at the DSDT it seems that the H_EC is not used and is leftover from a copy/paste
> from some reference design DSDT. Its _REG however does write to the EC before hitting the error
> and I think that that write may be causing the issue...
> 
> The H_EC device does have an _STA method and looking closer the troublesome EREG method is
> also called from _INI. So I guess that _STA is returning 0 causing _INI to not run and
> that is the reason why we are not seeing the same EREG errors with kernel 6.9.6 where _REG is
> only called for the EC opregion on \_SB_.PC00.LPCB.LGEC and not for the entire ACPI device
> hierarchy as is done with >= 6.9.7 .
> 
> Maybe we should only call _REG for the EC opregion on present devices (and devices without
> a _STA)?
> 
> Also note that both LGEC and H_EC use the same cmd + data ports.
> 
> I'll go and ask the reporter to retrieve the status of both LGEC and H_EC and then see
> from there.

The reporter has confirmed that of the 2 EC devices ( H_EC / LGEC ) only LGEC returns
a _STA of non 0:

> Here it is, with kernel 6.9.6:
> 
> $ cat /sys/bus/acpi/devices/PNP0C09\:00/path
> \_SB_.PC00.LPCB.H_EC
> $ cat /sys/bus/acpi/devices/PNP0C09\:00/status
> 0
> $ cat /sys/bus/acpi/devices/PNP0C09\:01/path
> \_SB_.PC00.LPCB.LGEC
> $ cat /sys/bus/acpi/devices/PNP0C09\:01/status
> 15

And taking a second look at the other bug:
https://bugzilla.redhat.com/show_bug.cgi?id=2298938

That one also has 2 EC ACPI devices and the errors come from calling _REG on the one
which is not picked as the boot_ec :

jul 19 17:33:41 kernel: ACPI: EC: EC started
jul 19 17:33:41 kernel: ACPI: EC: interrupt blocked
jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC.ECMD due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FNCL due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FN00._OFF due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC._REG due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
jul 19 17:33:41 kernel: ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
jul 19 17:33:41 kernel: ACPI: \_SB_.PCI0.LPCB.EC0_: Boot DSDT EC used to handle transactions

Note the H_EC vs EC0_ in the errors vs the "Boot DSDT EC used to
handle transactions" message.

So the issue in both cases seems to be calling _REG on an unused EC acpi_device.
Not sure how to best fix this though ...

Regards,

Hans

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Rafael J. Wysocki]

Hi,

On Mon, Aug 5, 2024 at 2:47 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 8/5/24 1:28 PM, Hans de Goede wrote:
> > Hi,
> >
> > On 8/1/24 4:28 PM, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 7/29/24 1:15 PM, Hans de Goede wrote:
> >>> Hi Rafael,
> >>>
> >>> There are 2 bug reports:
> >>>
> >>> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
> >>> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
> >>
> >> I have filed:
> >>
> >> https://bugzilla.redhat.com/show_bug.cgi?id=2302253
> >>
> >> to track this now and an acpidump of the troublesome LG laptop
> >> is attached there. I have also requested dmesg output of
> >> a non working kernel to be attached there.
> >>
> >> As a reminder this is the bug where it has been confirmed that
> >> reverting "EC: Install address space handler at the namespace root"
> >> helps, with the caveat that there is a Thunderbolt related IRQ
> >> storm on the ACPI event IRQ after the revert ...
> >
> > Ok, so the bugzilla now has 2 different dmesg outputs:
> >
> > 1. 6.9.6, this kernel works without problems
> >
> > 2. 6.9.12 with the following patch you suggested on top:
> >
> > --- a/drivers/acpi/ec.c
> > +++ b/drivers/acpi/ec.c
> > @@ -1788,7 +1788,7 @@ void __init acpi_ec_dsdt_probe(void)
> >        * At this point, the GPE is not fully initialized, so do not to
> >        * handle the events.
> >        */
> > -     ret = acpi_ec_setup(ec, NULL, true);
> > +     ret = acpi_ec_setup(ec, NULL, false);
> >       if (ret) {
> >               acpi_ec_free(ec);
> >               return;
> >
> > Unfortunately this does not help. dmesg shows some EC _REG errors, which
> > are now (with the above diff applied) shown just before the
> > "Boot DSDT EC initialization complete" message, which shows that _REG now
> > runs from acpi_ec_add() rather then before:
> >
> > [    1.007566] ACPI BIOS Error (bug): Could not resolve symbol [\_TZ.FN00._OFF], AE_NOT_FOUND (20230628/psargs-330)
> > [    1.007576] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC.EREG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
> > [    1.007580] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC._REG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
> > [    1.007639] ACPI: EC: interrupt unblocked
> > [    1.007640] ACPI: EC: event unblocked
> > [    1.007675] ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
> > [    1.007676] ACPI: EC: GPE=0x6e
> > [    1.007677] ACPI: \_SB_.PC00.LPCB.LGEC: Boot DSDT EC initialization complete
> > [    1.007679] ACPI: \_SB_.PC00.LPCB.LGEC: EC: Used to handle transactions and events
> >
> > Note that the errors are from calling _REG on \_SB.PC00.LPCB.H_EC, where as the actual
> > EC (and the only acpi_device on which _REG would get called for the EC Opregion before) is:
> > \_SB_.PC00.LPCB.LGEC.
> >
> > Looking at the DSDT it seems that the H_EC is not used and is leftover from a copy/paste
> > from some reference design DSDT. Its _REG however does write to the EC before hitting the error
> > and I think that that write may be causing the issue...
> >
> > The H_EC device does have an _STA method and looking closer the troublesome EREG method is
> > also called from _INI. So I guess that _STA is returning 0 causing _INI to not run and
> > that is the reason why we are not seeing the same EREG errors with kernel 6.9.6 where _REG is
> > only called for the EC opregion on \_SB_.PC00.LPCB.LGEC and not for the entire ACPI device
> > hierarchy as is done with >= 6.9.7 .
> >
> > Maybe we should only call _REG for the EC opregion on present devices (and devices without
> > a _STA)?
> >
> > Also note that both LGEC and H_EC use the same cmd + data ports.
> >
> > I'll go and ask the reporter to retrieve the status of both LGEC and H_EC and then see
> > from there.
>
> The reporter has confirmed that of the 2 EC devices ( H_EC / LGEC ) only LGEC returns
> a _STA of non 0:
>
> > Here it is, with kernel 6.9.6:
> >
> > $ cat /sys/bus/acpi/devices/PNP0C09\:00/path
> > \_SB_.PC00.LPCB.H_EC
> > $ cat /sys/bus/acpi/devices/PNP0C09\:00/status
> > 0
> > $ cat /sys/bus/acpi/devices/PNP0C09\:01/path
> > \_SB_.PC00.LPCB.LGEC
> > $ cat /sys/bus/acpi/devices/PNP0C09\:01/status
> > 15
>
> And taking a second look at the other bug:
> https://bugzilla.redhat.com/show_bug.cgi?id=2298938
>
> That one also has 2 EC ACPI devices and the errors come from calling _REG on the one
> which is not picked as the boot_ec :
>
> jul 19 17:33:41 kernel: ACPI: EC: EC started
> jul 19 17:33:41 kernel: ACPI: EC: interrupt blocked
> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC.ECMD due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FNCL due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FN00._OFF due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC._REG due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> jul 19 17:33:41 kernel: ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
> jul 19 17:33:41 kernel: ACPI: \_SB_.PCI0.LPCB.EC0_: Boot DSDT EC used to handle transactions
>
> Note the H_EC vs EC0_ in the errors vs the "Boot DSDT EC used to
> handle transactions" message.
>
> So the issue in both cases seems to be calling _REG on an unused EC acpi_device.
> Not sure how to best fix this though ...

I have created an experimental acpi-ec-fixes branch:

https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=acpi-ec-fixes

for this which illustrates my idea (untested so far).

The underlying observation is that _REG only needs to be evaluated for
EC operation regions located in the scopes of ACPI device objects
representing valid devices, so it is better to do it for each of these
objects individually in acpi_bus_attach().

For the EC itself, it is better to do what was done before the
$subject commit, so evaluate _REG for the EC operation regions in the
EC scope (including the "orphan" _REG).

Accordingly, commit 0e6b6dedf168 ("Revert "ACPI: EC: Evaluate orphan
_REG under EC device") is reverted, acpi_execute_reg_methods() is
modified to take an additional depth argument and it is called for
each device object representing a valid device with that argument
equal to 1.

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Rafael J. Wysocki]

On Thu, Aug 8, 2024 at 7:22 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> Hi,
>
> On Mon, Aug 5, 2024 at 2:47 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >
> > Hi,
> >
> > On 8/5/24 1:28 PM, Hans de Goede wrote:
> > > Hi,
> > >
> > > On 8/1/24 4:28 PM, Hans de Goede wrote:
> > >> Hi,
> > >>
> > >> On 7/29/24 1:15 PM, Hans de Goede wrote:
> > >>> Hi Rafael,
> > >>>
> > >>> There are 2 bug reports:
> > >>>
> > >>> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
> > >>> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
> > >>
> > >> I have filed:
> > >>
> > >> https://bugzilla.redhat.com/show_bug.cgi?id=2302253
> > >>
> > >> to track this now and an acpidump of the troublesome LG laptop
> > >> is attached there. I have also requested dmesg output of
> > >> a non working kernel to be attached there.
> > >>
> > >> As a reminder this is the bug where it has been confirmed that
> > >> reverting "EC: Install address space handler at the namespace root"
> > >> helps, with the caveat that there is a Thunderbolt related IRQ
> > >> storm on the ACPI event IRQ after the revert ...
> > >
> > > Ok, so the bugzilla now has 2 different dmesg outputs:
> > >
> > > 1. 6.9.6, this kernel works without problems
> > >
> > > 2. 6.9.12 with the following patch you suggested on top:
> > >
> > > --- a/drivers/acpi/ec.c
> > > +++ b/drivers/acpi/ec.c
> > > @@ -1788,7 +1788,7 @@ void __init acpi_ec_dsdt_probe(void)
> > >        * At this point, the GPE is not fully initialized, so do not to
> > >        * handle the events.
> > >        */
> > > -     ret = acpi_ec_setup(ec, NULL, true);
> > > +     ret = acpi_ec_setup(ec, NULL, false);
> > >       if (ret) {
> > >               acpi_ec_free(ec);
> > >               return;
> > >
> > > Unfortunately this does not help. dmesg shows some EC _REG errors, which
> > > are now (with the above diff applied) shown just before the
> > > "Boot DSDT EC initialization complete" message, which shows that _REG now
> > > runs from acpi_ec_add() rather then before:
> > >
> > > [    1.007566] ACPI BIOS Error (bug): Could not resolve symbol [\_TZ.FN00._OFF], AE_NOT_FOUND (20230628/psargs-330)
> > > [    1.007576] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC.EREG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
> > > [    1.007580] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC._REG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
> > > [    1.007639] ACPI: EC: interrupt unblocked
> > > [    1.007640] ACPI: EC: event unblocked
> > > [    1.007675] ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
> > > [    1.007676] ACPI: EC: GPE=0x6e
> > > [    1.007677] ACPI: \_SB_.PC00.LPCB.LGEC: Boot DSDT EC initialization complete
> > > [    1.007679] ACPI: \_SB_.PC00.LPCB.LGEC: EC: Used to handle transactions and events
> > >
> > > Note that the errors are from calling _REG on \_SB.PC00.LPCB.H_EC, where as the actual
> > > EC (and the only acpi_device on which _REG would get called for the EC Opregion before) is:
> > > \_SB_.PC00.LPCB.LGEC.
> > >
> > > Looking at the DSDT it seems that the H_EC is not used and is leftover from a copy/paste
> > > from some reference design DSDT. Its _REG however does write to the EC before hitting the error
> > > and I think that that write may be causing the issue...
> > >
> > > The H_EC device does have an _STA method and looking closer the troublesome EREG method is
> > > also called from _INI. So I guess that _STA is returning 0 causing _INI to not run and
> > > that is the reason why we are not seeing the same EREG errors with kernel 6.9.6 where _REG is
> > > only called for the EC opregion on \_SB_.PC00.LPCB.LGEC and not for the entire ACPI device
> > > hierarchy as is done with >= 6.9.7 .
> > >
> > > Maybe we should only call _REG for the EC opregion on present devices (and devices without
> > > a _STA)?
> > >
> > > Also note that both LGEC and H_EC use the same cmd + data ports.
> > >
> > > I'll go and ask the reporter to retrieve the status of both LGEC and H_EC and then see
> > > from there.
> >
> > The reporter has confirmed that of the 2 EC devices ( H_EC / LGEC ) only LGEC returns
> > a _STA of non 0:
> >
> > > Here it is, with kernel 6.9.6:
> > >
> > > $ cat /sys/bus/acpi/devices/PNP0C09\:00/path
> > > \_SB_.PC00.LPCB.H_EC
> > > $ cat /sys/bus/acpi/devices/PNP0C09\:00/status
> > > 0
> > > $ cat /sys/bus/acpi/devices/PNP0C09\:01/path
> > > \_SB_.PC00.LPCB.LGEC
> > > $ cat /sys/bus/acpi/devices/PNP0C09\:01/status
> > > 15
> >
> > And taking a second look at the other bug:
> > https://bugzilla.redhat.com/show_bug.cgi?id=2298938
> >
> > That one also has 2 EC ACPI devices and the errors come from calling _REG on the one
> > which is not picked as the boot_ec :
> >
> > jul 19 17:33:41 kernel: ACPI: EC: EC started
> > jul 19 17:33:41 kernel: ACPI: EC: interrupt blocked
> > jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC.ECMD due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> > jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FNCL due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> > jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FN00._OFF due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> > jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC._REG due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> > jul 19 17:33:41 kernel: ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
> > jul 19 17:33:41 kernel: ACPI: \_SB_.PCI0.LPCB.EC0_: Boot DSDT EC used to handle transactions
> >
> > Note the H_EC vs EC0_ in the errors vs the "Boot DSDT EC used to
> > handle transactions" message.
> >
> > So the issue in both cases seems to be calling _REG on an unused EC acpi_device.
> > Not sure how to best fix this though ...
>
> I have created an experimental acpi-ec-fixes branch:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=acpi-ec-fixes
>
> for this which illustrates my idea (untested so far).

This has just been updated to actually register the EC _REG for all
devices (which would have been skipped for the majority of them due to
the previous location of the relevant function call).

> The underlying observation is that _REG only needs to be evaluated for
> EC operation regions located in the scopes of ACPI device objects
> representing valid devices, so it is better to do it for each of these
> objects individually in acpi_bus_attach().
>
> For the EC itself, it is better to do what was done before the
> $subject commit, so evaluate _REG for the EC operation regions in the
> EC scope (including the "orphan" _REG).
>
> Accordingly, commit 0e6b6dedf168 ("Revert "ACPI: EC: Evaluate orphan
> _REG under EC device") is reverted, acpi_execute_reg_methods() is
> modified to take an additional depth argument and it is called for
> each device object representing a valid device with that argument
> equal to 1.

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Armin Wolf]

Am 09.08.24 um 15:17 schrieb Rafael J. Wysocki:

> On Thu, Aug 8, 2024 at 7:22 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>> Hi,
>>
>> On Mon, Aug 5, 2024 at 2:47 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> On 8/5/24 1:28 PM, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 8/1/24 4:28 PM, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 7/29/24 1:15 PM, Hans de Goede wrote:
>>>>>> Hi Rafael,
>>>>>>
>>>>>> There are 2 bug reports:
>>>>>>
>>>>>> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
>>>>>> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
>>>>> I have filed:
>>>>>
>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=2302253
>>>>>
>>>>> to track this now and an acpidump of the troublesome LG laptop
>>>>> is attached there. I have also requested dmesg output of
>>>>> a non working kernel to be attached there.
>>>>>
>>>>> As a reminder this is the bug where it has been confirmed that
>>>>> reverting "EC: Install address space handler at the namespace root"
>>>>> helps, with the caveat that there is a Thunderbolt related IRQ
>>>>> storm on the ACPI event IRQ after the revert ...
>>>> Ok, so the bugzilla now has 2 different dmesg outputs:
>>>>
>>>> 1. 6.9.6, this kernel works without problems
>>>>
>>>> 2. 6.9.12 with the following patch you suggested on top:
>>>>
>>>> --- a/drivers/acpi/ec.c
>>>> +++ b/drivers/acpi/ec.c
>>>> @@ -1788,7 +1788,7 @@ void __init acpi_ec_dsdt_probe(void)
>>>>         * At this point, the GPE is not fully initialized, so do not to
>>>>         * handle the events.
>>>>         */
>>>> -     ret = acpi_ec_setup(ec, NULL, true);
>>>> +     ret = acpi_ec_setup(ec, NULL, false);
>>>>        if (ret) {
>>>>                acpi_ec_free(ec);
>>>>                return;
>>>>
>>>> Unfortunately this does not help. dmesg shows some EC _REG errors, which
>>>> are now (with the above diff applied) shown just before the
>>>> "Boot DSDT EC initialization complete" message, which shows that _REG now
>>>> runs from acpi_ec_add() rather then before:
>>>>
>>>> [    1.007566] ACPI BIOS Error (bug): Could not resolve symbol [\_TZ.FN00._OFF], AE_NOT_FOUND (20230628/psargs-330)
>>>> [    1.007576] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC.EREG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
>>>> [    1.007580] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC._REG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
>>>> [    1.007639] ACPI: EC: interrupt unblocked
>>>> [    1.007640] ACPI: EC: event unblocked
>>>> [    1.007675] ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
>>>> [    1.007676] ACPI: EC: GPE=0x6e
>>>> [    1.007677] ACPI: \_SB_.PC00.LPCB.LGEC: Boot DSDT EC initialization complete
>>>> [    1.007679] ACPI: \_SB_.PC00.LPCB.LGEC: EC: Used to handle transactions and events
>>>>
>>>> Note that the errors are from calling _REG on \_SB.PC00.LPCB.H_EC, where as the actual
>>>> EC (and the only acpi_device on which _REG would get called for the EC Opregion before) is:
>>>> \_SB_.PC00.LPCB.LGEC.
>>>>
>>>> Looking at the DSDT it seems that the H_EC is not used and is leftover from a copy/paste
>>>> from some reference design DSDT. Its _REG however does write to the EC before hitting the error
>>>> and I think that that write may be causing the issue...
>>>>
>>>> The H_EC device does have an _STA method and looking closer the troublesome EREG method is
>>>> also called from _INI. So I guess that _STA is returning 0 causing _INI to not run and
>>>> that is the reason why we are not seeing the same EREG errors with kernel 6.9.6 where _REG is
>>>> only called for the EC opregion on \_SB_.PC00.LPCB.LGEC and not for the entire ACPI device
>>>> hierarchy as is done with >= 6.9.7 .
>>>>
>>>> Maybe we should only call _REG for the EC opregion on present devices (and devices without
>>>> a _STA)?
>>>>
>>>> Also note that both LGEC and H_EC use the same cmd + data ports.
>>>>
>>>> I'll go and ask the reporter to retrieve the status of both LGEC and H_EC and then see
>>>> from there.
>>> The reporter has confirmed that of the 2 EC devices ( H_EC / LGEC ) only LGEC returns
>>> a _STA of non 0:
>>>
>>>> Here it is, with kernel 6.9.6:
>>>>
>>>> $ cat /sys/bus/acpi/devices/PNP0C09\:00/path
>>>> \_SB_.PC00.LPCB.H_EC
>>>> $ cat /sys/bus/acpi/devices/PNP0C09\:00/status
>>>> 0
>>>> $ cat /sys/bus/acpi/devices/PNP0C09\:01/path
>>>> \_SB_.PC00.LPCB.LGEC
>>>> $ cat /sys/bus/acpi/devices/PNP0C09\:01/status
>>>> 15
>>> And taking a second look at the other bug:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=2298938
>>>
>>> That one also has 2 EC ACPI devices and the errors come from calling _REG on the one
>>> which is not picked as the boot_ec :
>>>
>>> jul 19 17:33:41 kernel: ACPI: EC: EC started
>>> jul 19 17:33:41 kernel: ACPI: EC: interrupt blocked
>>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC.ECMD due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FNCL due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FN00._OFF due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC._REG due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>>> jul 19 17:33:41 kernel: ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
>>> jul 19 17:33:41 kernel: ACPI: \_SB_.PCI0.LPCB.EC0_: Boot DSDT EC used to handle transactions
>>>
>>> Note the H_EC vs EC0_ in the errors vs the "Boot DSDT EC used to
>>> handle transactions" message.
>>>
>>> So the issue in both cases seems to be calling _REG on an unused EC acpi_device.
>>> Not sure how to best fix this though ...
>> I have created an experimental acpi-ec-fixes branch:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=acpi-ec-fixes
>>
>> for this which illustrates my idea (untested so far).
> This has just been updated to actually register the EC _REG for all
> devices (which would have been skipped for the majority of them due to
> the previous location of the relevant function call).

Hi,

i will ask the reporter https://bugzilla.kernel.org/show_bug.cgi?id=219075 to try out your patches.
His LG Gram seems to be affected by this problem (it has two overlapping EC devices ...).

Thanks,
Armin Wolf

>> The underlying observation is that _REG only needs to be evaluated for
>> EC operation regions located in the scopes of ACPI device objects
>> representing valid devices, so it is better to do it for each of these
>> objects individually in acpi_bus_attach().
>>
>> For the EC itself, it is better to do what was done before the
>> $subject commit, so evaluate _REG for the EC operation regions in the
>> EC scope (including the "orphan" _REG).
>>
>> Accordingly, commit 0e6b6dedf168 ("Revert "ACPI: EC: Evaluate orphan
>> _REG under EC device") is reverted, acpi_execute_reg_methods() is
>> modified to take an additional depth argument and it is called for
>> each device object representing a valid device with that argument
>> equal to 1.

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Hans de Goede]

Hi,

On 8/9/24 8:02 PM, Armin Wolf wrote:
> Am 09.08.24 um 15:17 schrieb Rafael J. Wysocki:
> 
>> On Thu, Aug 8, 2024 at 7:22 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>> Hi,
>>>
>>> On Mon, Aug 5, 2024 at 2:47 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Hi,
>>>>
>>>> On 8/5/24 1:28 PM, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 8/1/24 4:28 PM, Hans de Goede wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 7/29/24 1:15 PM, Hans de Goede wrote:
>>>>>>> Hi Rafael,
>>>>>>>
>>>>>>> There are 2 bug reports:
>>>>>>>
>>>>>>> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
>>>>>>> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
>>>>>> I have filed:
>>>>>>
>>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=2302253
>>>>>>
>>>>>> to track this now and an acpidump of the troublesome LG laptop
>>>>>> is attached there. I have also requested dmesg output of
>>>>>> a non working kernel to be attached there.
>>>>>>
>>>>>> As a reminder this is the bug where it has been confirmed that
>>>>>> reverting "EC: Install address space handler at the namespace root"
>>>>>> helps, with the caveat that there is a Thunderbolt related IRQ
>>>>>> storm on the ACPI event IRQ after the revert ...
>>>>> Ok, so the bugzilla now has 2 different dmesg outputs:
>>>>>
>>>>> 1. 6.9.6, this kernel works without problems
>>>>>
>>>>> 2. 6.9.12 with the following patch you suggested on top:
>>>>>
>>>>> --- a/drivers/acpi/ec.c
>>>>> +++ b/drivers/acpi/ec.c
>>>>> @@ -1788,7 +1788,7 @@ void __init acpi_ec_dsdt_probe(void)
>>>>>         * At this point, the GPE is not fully initialized, so do not to
>>>>>         * handle the events.
>>>>>         */
>>>>> -     ret = acpi_ec_setup(ec, NULL, true);
>>>>> +     ret = acpi_ec_setup(ec, NULL, false);
>>>>>        if (ret) {
>>>>>                acpi_ec_free(ec);
>>>>>                return;
>>>>>
>>>>> Unfortunately this does not help. dmesg shows some EC _REG errors, which
>>>>> are now (with the above diff applied) shown just before the
>>>>> "Boot DSDT EC initialization complete" message, which shows that _REG now
>>>>> runs from acpi_ec_add() rather then before:
>>>>>
>>>>> [    1.007566] ACPI BIOS Error (bug): Could not resolve symbol [\_TZ.FN00._OFF], AE_NOT_FOUND (20230628/psargs-330)
>>>>> [    1.007576] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC.EREG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
>>>>> [    1.007580] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC._REG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
>>>>> [    1.007639] ACPI: EC: interrupt unblocked
>>>>> [    1.007640] ACPI: EC: event unblocked
>>>>> [    1.007675] ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
>>>>> [    1.007676] ACPI: EC: GPE=0x6e
>>>>> [    1.007677] ACPI: \_SB_.PC00.LPCB.LGEC: Boot DSDT EC initialization complete
>>>>> [    1.007679] ACPI: \_SB_.PC00.LPCB.LGEC: EC: Used to handle transactions and events
>>>>>
>>>>> Note that the errors are from calling _REG on \_SB.PC00.LPCB.H_EC, where as the actual
>>>>> EC (and the only acpi_device on which _REG would get called for the EC Opregion before) is:
>>>>> \_SB_.PC00.LPCB.LGEC.
>>>>>
>>>>> Looking at the DSDT it seems that the H_EC is not used and is leftover from a copy/paste
>>>>> from some reference design DSDT. Its _REG however does write to the EC before hitting the error
>>>>> and I think that that write may be causing the issue...
>>>>>
>>>>> The H_EC device does have an _STA method and looking closer the troublesome EREG method is
>>>>> also called from _INI. So I guess that _STA is returning 0 causing _INI to not run and
>>>>> that is the reason why we are not seeing the same EREG errors with kernel 6.9.6 where _REG is
>>>>> only called for the EC opregion on \_SB_.PC00.LPCB.LGEC and not for the entire ACPI device
>>>>> hierarchy as is done with >= 6.9.7 .
>>>>>
>>>>> Maybe we should only call _REG for the EC opregion on present devices (and devices without
>>>>> a _STA)?
>>>>>
>>>>> Also note that both LGEC and H_EC use the same cmd + data ports.
>>>>>
>>>>> I'll go and ask the reporter to retrieve the status of both LGEC and H_EC and then see
>>>>> from there.
>>>> The reporter has confirmed that of the 2 EC devices ( H_EC / LGEC ) only LGEC returns
>>>> a _STA of non 0:
>>>>
>>>>> Here it is, with kernel 6.9.6:
>>>>>
>>>>> $ cat /sys/bus/acpi/devices/PNP0C09\:00/path
>>>>> \_SB_.PC00.LPCB.H_EC
>>>>> $ cat /sys/bus/acpi/devices/PNP0C09\:00/status
>>>>> 0
>>>>> $ cat /sys/bus/acpi/devices/PNP0C09\:01/path
>>>>> \_SB_.PC00.LPCB.LGEC
>>>>> $ cat /sys/bus/acpi/devices/PNP0C09\:01/status
>>>>> 15
>>>> And taking a second look at the other bug:
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=2298938
>>>>
>>>> That one also has 2 EC ACPI devices and the errors come from calling _REG on the one
>>>> which is not picked as the boot_ec :
>>>>
>>>> jul 19 17:33:41 kernel: ACPI: EC: EC started
>>>> jul 19 17:33:41 kernel: ACPI: EC: interrupt blocked
>>>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC.ECMD due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>>>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FNCL due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>>>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FN00._OFF due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>>>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC._REG due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>>>> jul 19 17:33:41 kernel: ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
>>>> jul 19 17:33:41 kernel: ACPI: \_SB_.PCI0.LPCB.EC0_: Boot DSDT EC used to handle transactions
>>>>
>>>> Note the H_EC vs EC0_ in the errors vs the "Boot DSDT EC used to
>>>> handle transactions" message.
>>>>
>>>> So the issue in both cases seems to be calling _REG on an unused EC acpi_device.
>>>> Not sure how to best fix this though ...
>>> I have created an experimental acpi-ec-fixes branch:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=acpi-ec-fixes
>>>
>>> for this which illustrates my idea (untested so far).
>> This has just been updated to actually register the EC _REG for all
>> devices (which would have been skipped for the majority of them due to
>> the previous location of the relevant function call).
> 
> Hi,
> 
> i will ask the reporter https://bugzilla.kernel.org/show_bug.cgi?id=219075 to try out your patches.
> His LG Gram seems to be affected by this problem (it has two overlapping EC devices ...).

Ah another reporter with an LG Gram, this Fedora bug:

https://bugzilla.redhat.com/show_bug.cgi?id=2302253

is also about a LG Gram. And shows the same:

[    0.239044] ACPI: EC: EC started
[    0.239045] ACPI: EC: interrupt blocked
[    0.239787] ACPI BIOS Error (bug): Could not resolve symbol [\_TZ.FN00._OFF], AE_NOT_FOUND (20240322/psargs-330)
[    0.239794] fbcon: Taking over console
[    0.239800] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC.EREG due to previous error (AE_NOT_FOUND) (20240322/psparse-
[    0.239805] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC._REG due to previous error (AE_NOT_FOUND) (20240322/psparse-
[    0.239820] ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
[    0.239822] ACPI: \_SB_.PC00.LPCB.LGEC: Boot DSDT EC used to handle transactions

errors in their dmesg with regressed kernels.

So this seems to be the same issue.

Regards,

Hans




> 
> Thanks,
> Armin Wolf
> 
>>> The underlying observation is that _REG only needs to be evaluated for
>>> EC operation regions located in the scopes of ACPI device objects
>>> representing valid devices, so it is better to do it for each of these
>>> objects individually in acpi_bus_attach().
>>>
>>> For the EC itself, it is better to do what was done before the
>>> $subject commit, so evaluate _REG for the EC operation regions in the
>>> EC scope (including the "orphan" _REG).
>>>
>>> Accordingly, commit 0e6b6dedf168 ("Revert "ACPI: EC: Evaluate orphan
>>> _REG under EC device") is reverted, acpi_execute_reg_methods() is
>>> modified to take an additional depth argument and it is called for
>>> each device object representing a valid device with that argument
>>> equal to 1.
> 

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Hans de Goede]

Hi,

On 8/8/24 7:22 PM, Rafael J. Wysocki wrote:
> Hi,
> 
> On Mon, Aug 5, 2024 at 2:47 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
>> On 8/5/24 1:28 PM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 8/1/24 4:28 PM, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 7/29/24 1:15 PM, Hans de Goede wrote:
>>>>> Hi Rafael,
>>>>>
>>>>> There are 2 bug reports:
>>>>>
>>>>> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
>>>>> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
>>>>
>>>> I have filed:
>>>>
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=2302253
>>>>
>>>> to track this now and an acpidump of the troublesome LG laptop
>>>> is attached there. I have also requested dmesg output of
>>>> a non working kernel to be attached there.
>>>>
>>>> As a reminder this is the bug where it has been confirmed that
>>>> reverting "EC: Install address space handler at the namespace root"
>>>> helps, with the caveat that there is a Thunderbolt related IRQ
>>>> storm on the ACPI event IRQ after the revert ...
>>>
>>> Ok, so the bugzilla now has 2 different dmesg outputs:
>>>
>>> 1. 6.9.6, this kernel works without problems
>>>
>>> 2. 6.9.12 with the following patch you suggested on top:
>>>
>>> --- a/drivers/acpi/ec.c
>>> +++ b/drivers/acpi/ec.c
>>> @@ -1788,7 +1788,7 @@ void __init acpi_ec_dsdt_probe(void)
>>>        * At this point, the GPE is not fully initialized, so do not to
>>>        * handle the events.
>>>        */
>>> -     ret = acpi_ec_setup(ec, NULL, true);
>>> +     ret = acpi_ec_setup(ec, NULL, false);
>>>       if (ret) {
>>>               acpi_ec_free(ec);
>>>               return;
>>>
>>> Unfortunately this does not help. dmesg shows some EC _REG errors, which
>>> are now (with the above diff applied) shown just before the
>>> "Boot DSDT EC initialization complete" message, which shows that _REG now
>>> runs from acpi_ec_add() rather then before:
>>>
>>> [    1.007566] ACPI BIOS Error (bug): Could not resolve symbol [\_TZ.FN00._OFF], AE_NOT_FOUND (20230628/psargs-330)
>>> [    1.007576] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC.EREG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
>>> [    1.007580] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC._REG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
>>> [    1.007639] ACPI: EC: interrupt unblocked
>>> [    1.007640] ACPI: EC: event unblocked
>>> [    1.007675] ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
>>> [    1.007676] ACPI: EC: GPE=0x6e
>>> [    1.007677] ACPI: \_SB_.PC00.LPCB.LGEC: Boot DSDT EC initialization complete
>>> [    1.007679] ACPI: \_SB_.PC00.LPCB.LGEC: EC: Used to handle transactions and events
>>>
>>> Note that the errors are from calling _REG on \_SB.PC00.LPCB.H_EC, where as the actual
>>> EC (and the only acpi_device on which _REG would get called for the EC Opregion before) is:
>>> \_SB_.PC00.LPCB.LGEC.
>>>
>>> Looking at the DSDT it seems that the H_EC is not used and is leftover from a copy/paste
>>> from some reference design DSDT. Its _REG however does write to the EC before hitting the error
>>> and I think that that write may be causing the issue...
>>>
>>> The H_EC device does have an _STA method and looking closer the troublesome EREG method is
>>> also called from _INI. So I guess that _STA is returning 0 causing _INI to not run and
>>> that is the reason why we are not seeing the same EREG errors with kernel 6.9.6 where _REG is
>>> only called for the EC opregion on \_SB_.PC00.LPCB.LGEC and not for the entire ACPI device
>>> hierarchy as is done with >= 6.9.7 .
>>>
>>> Maybe we should only call _REG for the EC opregion on present devices (and devices without
>>> a _STA)?
>>>
>>> Also note that both LGEC and H_EC use the same cmd + data ports.
>>>
>>> I'll go and ask the reporter to retrieve the status of both LGEC and H_EC and then see
>>> from there.
>>
>> The reporter has confirmed that of the 2 EC devices ( H_EC / LGEC ) only LGEC returns
>> a _STA of non 0:
>>
>>> Here it is, with kernel 6.9.6:
>>>
>>> $ cat /sys/bus/acpi/devices/PNP0C09\:00/path
>>> \_SB_.PC00.LPCB.H_EC
>>> $ cat /sys/bus/acpi/devices/PNP0C09\:00/status
>>> 0
>>> $ cat /sys/bus/acpi/devices/PNP0C09\:01/path
>>> \_SB_.PC00.LPCB.LGEC
>>> $ cat /sys/bus/acpi/devices/PNP0C09\:01/status
>>> 15
>>
>> And taking a second look at the other bug:
>> https://bugzilla.redhat.com/show_bug.cgi?id=2298938
>>
>> That one also has 2 EC ACPI devices and the errors come from calling _REG on the one
>> which is not picked as the boot_ec :
>>
>> jul 19 17:33:41 kernel: ACPI: EC: EC started
>> jul 19 17:33:41 kernel: ACPI: EC: interrupt blocked
>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC.ECMD due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FNCL due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FN00._OFF due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC._REG due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>> jul 19 17:33:41 kernel: ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
>> jul 19 17:33:41 kernel: ACPI: \_SB_.PCI0.LPCB.EC0_: Boot DSDT EC used to handle transactions
>>
>> Note the H_EC vs EC0_ in the errors vs the "Boot DSDT EC used to
>> handle transactions" message.
>>
>> So the issue in both cases seems to be calling _REG on an unused EC acpi_device.
>> Not sure how to best fix this though ...
> 
> I have created an experimental acpi-ec-fixes branch:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=acpi-ec-fixes
> 
> for this which illustrates my idea (untested so far).

Thank you. I believe that the approach taken here is good and I also
like the code (of the current version) so you may add my:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

to all 3 patches.

I have started a test Fedora 40 6.10.4 kernel build with the patches from
the acpi-ec-fixes branch added:

https://koji.fedoraproject.org/koji/taskinfo?taskID=121834209

and I have asked the reporters of both bugs:

    2298938 - Multiple ACPI errors resulting in incorrect thermal readings and misleading CPU 
    2302253 - ACPI: EC: LG gram laptop brightness keys stop working with kernel >= 6.9.7

to test this. I expect a good turn around time from the reporter
of bug 2302253. So far the reporter of 2298938 is not really
responsive (holidays?).

Regards,

Hans

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Rafael J. Wysocki]

On Mon, Aug 12, 2024 at 1:28 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 8/8/24 7:22 PM, Rafael J. Wysocki wrote:
> > Hi,
> >
> > On Mon, Aug 5, 2024 at 2:47 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi,
> >>
> >> On 8/5/24 1:28 PM, Hans de Goede wrote:
> >>> Hi,
> >>>
> >>> On 8/1/24 4:28 PM, Hans de Goede wrote:
> >>>> Hi,
> >>>>
> >>>> On 7/29/24 1:15 PM, Hans de Goede wrote:
> >>>>> Hi Rafael,
> >>>>>
> >>>>> There are 2 bug reports:
> >>>>>
> >>>>> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
> >>>>> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
> >>>>
> >>>> I have filed:
> >>>>
> >>>> https://bugzilla.redhat.com/show_bug.cgi?id=2302253
> >>>>
> >>>> to track this now and an acpidump of the troublesome LG laptop
> >>>> is attached there. I have also requested dmesg output of
> >>>> a non working kernel to be attached there.
> >>>>
> >>>> As a reminder this is the bug where it has been confirmed that
> >>>> reverting "EC: Install address space handler at the namespace root"
> >>>> helps, with the caveat that there is a Thunderbolt related IRQ
> >>>> storm on the ACPI event IRQ after the revert ...
> >>>
> >>> Ok, so the bugzilla now has 2 different dmesg outputs:
> >>>
> >>> 1. 6.9.6, this kernel works without problems
> >>>
> >>> 2. 6.9.12 with the following patch you suggested on top:
> >>>
> >>> --- a/drivers/acpi/ec.c
> >>> +++ b/drivers/acpi/ec.c
> >>> @@ -1788,7 +1788,7 @@ void __init acpi_ec_dsdt_probe(void)
> >>>        * At this point, the GPE is not fully initialized, so do not to
> >>>        * handle the events.
> >>>        */
> >>> -     ret = acpi_ec_setup(ec, NULL, true);
> >>> +     ret = acpi_ec_setup(ec, NULL, false);
> >>>       if (ret) {
> >>>               acpi_ec_free(ec);
> >>>               return;
> >>>
> >>> Unfortunately this does not help. dmesg shows some EC _REG errors, which
> >>> are now (with the above diff applied) shown just before the
> >>> "Boot DSDT EC initialization complete" message, which shows that _REG now
> >>> runs from acpi_ec_add() rather then before:
> >>>
> >>> [    1.007566] ACPI BIOS Error (bug): Could not resolve symbol [\_TZ.FN00._OFF], AE_NOT_FOUND (20230628/psargs-330)
> >>> [    1.007576] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC.EREG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
> >>> [    1.007580] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC._REG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
> >>> [    1.007639] ACPI: EC: interrupt unblocked
> >>> [    1.007640] ACPI: EC: event unblocked
> >>> [    1.007675] ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
> >>> [    1.007676] ACPI: EC: GPE=0x6e
> >>> [    1.007677] ACPI: \_SB_.PC00.LPCB.LGEC: Boot DSDT EC initialization complete
> >>> [    1.007679] ACPI: \_SB_.PC00.LPCB.LGEC: EC: Used to handle transactions and events
> >>>
> >>> Note that the errors are from calling _REG on \_SB.PC00.LPCB.H_EC, where as the actual
> >>> EC (and the only acpi_device on which _REG would get called for the EC Opregion before) is:
> >>> \_SB_.PC00.LPCB.LGEC.
> >>>
> >>> Looking at the DSDT it seems that the H_EC is not used and is leftover from a copy/paste
> >>> from some reference design DSDT. Its _REG however does write to the EC before hitting the error
> >>> and I think that that write may be causing the issue...
> >>>
> >>> The H_EC device does have an _STA method and looking closer the troublesome EREG method is
> >>> also called from _INI. So I guess that _STA is returning 0 causing _INI to not run and
> >>> that is the reason why we are not seeing the same EREG errors with kernel 6.9.6 where _REG is
> >>> only called for the EC opregion on \_SB_.PC00.LPCB.LGEC and not for the entire ACPI device
> >>> hierarchy as is done with >= 6.9.7 .
> >>>
> >>> Maybe we should only call _REG for the EC opregion on present devices (and devices without
> >>> a _STA)?
> >>>
> >>> Also note that both LGEC and H_EC use the same cmd + data ports.
> >>>
> >>> I'll go and ask the reporter to retrieve the status of both LGEC and H_EC and then see
> >>> from there.
> >>
> >> The reporter has confirmed that of the 2 EC devices ( H_EC / LGEC ) only LGEC returns
> >> a _STA of non 0:
> >>
> >>> Here it is, with kernel 6.9.6:
> >>>
> >>> $ cat /sys/bus/acpi/devices/PNP0C09\:00/path
> >>> \_SB_.PC00.LPCB.H_EC
> >>> $ cat /sys/bus/acpi/devices/PNP0C09\:00/status
> >>> 0
> >>> $ cat /sys/bus/acpi/devices/PNP0C09\:01/path
> >>> \_SB_.PC00.LPCB.LGEC
> >>> $ cat /sys/bus/acpi/devices/PNP0C09\:01/status
> >>> 15
> >>
> >> And taking a second look at the other bug:
> >> https://bugzilla.redhat.com/show_bug.cgi?id=2298938
> >>
> >> That one also has 2 EC ACPI devices and the errors come from calling _REG on the one
> >> which is not picked as the boot_ec :
> >>
> >> jul 19 17:33:41 kernel: ACPI: EC: EC started
> >> jul 19 17:33:41 kernel: ACPI: EC: interrupt blocked
> >> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC.ECMD due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> >> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FNCL due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> >> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FN00._OFF due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> >> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC._REG due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
> >> jul 19 17:33:41 kernel: ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
> >> jul 19 17:33:41 kernel: ACPI: \_SB_.PCI0.LPCB.EC0_: Boot DSDT EC used to handle transactions
> >>
> >> Note the H_EC vs EC0_ in the errors vs the "Boot DSDT EC used to
> >> handle transactions" message.
> >>
> >> So the issue in both cases seems to be calling _REG on an unused EC acpi_device.
> >> Not sure how to best fix this though ...
> >
> > I have created an experimental acpi-ec-fixes branch:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=acpi-ec-fixes
> >
> > for this which illustrates my idea (untested so far).
>
> Thank you. I believe that the approach taken here is good and I also
> like the code (of the current version) so you may add my:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
> to all 3 patches.
>
> I have started a test Fedora 40 6.10.4 kernel build with the patches from
> the acpi-ec-fixes branch added:
>
> https://koji.fedoraproject.org/koji/taskinfo?taskID=121834209
>
> and I have asked the reporters of both bugs:
>
>     2298938 - Multiple ACPI errors resulting in incorrect thermal readings and misleading CPU
>     2302253 - ACPI: EC: LG gram laptop brightness keys stop working with kernel >= 6.9.7
>
> to test this. I expect a good turn around time from the reporter
> of bug 2302253. So far the reporter of 2298938 is not really
> responsive (holidays?).

Thank you!

I'll send the patches to the list later today.

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Hans de Goede]

Hi,

On 8/12/24 1:28 PM, Hans de Goede wrote:
> Hi,
> 
> On 8/8/24 7:22 PM, Rafael J. Wysocki wrote:
>> Hi,
>>
>> On Mon, Aug 5, 2024 at 2:47 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> Hi,
>>>
>>> On 8/5/24 1:28 PM, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 8/1/24 4:28 PM, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 7/29/24 1:15 PM, Hans de Goede wrote:
>>>>>> Hi Rafael,
>>>>>>
>>>>>> There are 2 bug reports:
>>>>>>
>>>>>> 1. Brightness up/down key-presses no longer working on LG laptop (acpi-video related):
>>>>>> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/thread/V2KWAGZIAX4TOWPCH6A6FSIT66PR3KMZ/
>>>>>
>>>>> I have filed:
>>>>>
>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=2302253
>>>>>
>>>>> to track this now and an acpidump of the troublesome LG laptop
>>>>> is attached there. I have also requested dmesg output of
>>>>> a non working kernel to be attached there.
>>>>>
>>>>> As a reminder this is the bug where it has been confirmed that
>>>>> reverting "EC: Install address space handler at the namespace root"
>>>>> helps, with the caveat that there is a Thunderbolt related IRQ
>>>>> storm on the ACPI event IRQ after the revert ...
>>>>
>>>> Ok, so the bugzilla now has 2 different dmesg outputs:
>>>>
>>>> 1. 6.9.6, this kernel works without problems
>>>>
>>>> 2. 6.9.12 with the following patch you suggested on top:
>>>>
>>>> --- a/drivers/acpi/ec.c
>>>> +++ b/drivers/acpi/ec.c
>>>> @@ -1788,7 +1788,7 @@ void __init acpi_ec_dsdt_probe(void)
>>>>        * At this point, the GPE is not fully initialized, so do not to
>>>>        * handle the events.
>>>>        */
>>>> -     ret = acpi_ec_setup(ec, NULL, true);
>>>> +     ret = acpi_ec_setup(ec, NULL, false);
>>>>       if (ret) {
>>>>               acpi_ec_free(ec);
>>>>               return;
>>>>
>>>> Unfortunately this does not help. dmesg shows some EC _REG errors, which
>>>> are now (with the above diff applied) shown just before the
>>>> "Boot DSDT EC initialization complete" message, which shows that _REG now
>>>> runs from acpi_ec_add() rather then before:
>>>>
>>>> [    1.007566] ACPI BIOS Error (bug): Could not resolve symbol [\_TZ.FN00._OFF], AE_NOT_FOUND (20230628/psargs-330)
>>>> [    1.007576] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC.EREG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
>>>> [    1.007580] ACPI Error: Aborting method \_SB.PC00.LPCB.H_EC._REG due to previous error (AE_NOT_FOUND) (20230628/psparse-52
>>>> [    1.007639] ACPI: EC: interrupt unblocked
>>>> [    1.007640] ACPI: EC: event unblocked
>>>> [    1.007675] ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
>>>> [    1.007676] ACPI: EC: GPE=0x6e
>>>> [    1.007677] ACPI: \_SB_.PC00.LPCB.LGEC: Boot DSDT EC initialization complete
>>>> [    1.007679] ACPI: \_SB_.PC00.LPCB.LGEC: EC: Used to handle transactions and events
>>>>
>>>> Note that the errors are from calling _REG on \_SB.PC00.LPCB.H_EC, where as the actual
>>>> EC (and the only acpi_device on which _REG would get called for the EC Opregion before) is:
>>>> \_SB_.PC00.LPCB.LGEC.
>>>>
>>>> Looking at the DSDT it seems that the H_EC is not used and is leftover from a copy/paste
>>>> from some reference design DSDT. Its _REG however does write to the EC before hitting the error
>>>> and I think that that write may be causing the issue...
>>>>
>>>> The H_EC device does have an _STA method and looking closer the troublesome EREG method is
>>>> also called from _INI. So I guess that _STA is returning 0 causing _INI to not run and
>>>> that is the reason why we are not seeing the same EREG errors with kernel 6.9.6 where _REG is
>>>> only called for the EC opregion on \_SB_.PC00.LPCB.LGEC and not for the entire ACPI device
>>>> hierarchy as is done with >= 6.9.7 .
>>>>
>>>> Maybe we should only call _REG for the EC opregion on present devices (and devices without
>>>> a _STA)?
>>>>
>>>> Also note that both LGEC and H_EC use the same cmd + data ports.
>>>>
>>>> I'll go and ask the reporter to retrieve the status of both LGEC and H_EC and then see
>>>> from there.
>>>
>>> The reporter has confirmed that of the 2 EC devices ( H_EC / LGEC ) only LGEC returns
>>> a _STA of non 0:
>>>
>>>> Here it is, with kernel 6.9.6:
>>>>
>>>> $ cat /sys/bus/acpi/devices/PNP0C09\:00/path
>>>> \_SB_.PC00.LPCB.H_EC
>>>> $ cat /sys/bus/acpi/devices/PNP0C09\:00/status
>>>> 0
>>>> $ cat /sys/bus/acpi/devices/PNP0C09\:01/path
>>>> \_SB_.PC00.LPCB.LGEC
>>>> $ cat /sys/bus/acpi/devices/PNP0C09\:01/status
>>>> 15
>>>
>>> And taking a second look at the other bug:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=2298938
>>>
>>> That one also has 2 EC ACPI devices and the errors come from calling _REG on the one
>>> which is not picked as the boot_ec :
>>>
>>> jul 19 17:33:41 kernel: ACPI: EC: EC started
>>> jul 19 17:33:41 kernel: ACPI: EC: interrupt blocked
>>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC.ECMD due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FNCL due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_TZ.FN00._OFF due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>>> jul 19 17:33:41 kernel: ACPI Error: Aborting method \_SB.PCI0.LPCB.H_EC._REG due to previous error (AE_AML_LOOP_TIMEOUT) (20230628/psparse-529)
>>> jul 19 17:33:41 kernel: ACPI: EC: EC_CMD/EC_SC=0x66, EC_DATA=0x62
>>> jul 19 17:33:41 kernel: ACPI: \_SB_.PCI0.LPCB.EC0_: Boot DSDT EC used to handle transactions
>>>
>>> Note the H_EC vs EC0_ in the errors vs the "Boot DSDT EC used to
>>> handle transactions" message.
>>>
>>> So the issue in both cases seems to be calling _REG on an unused EC acpi_device.
>>> Not sure how to best fix this though ...
>>
>> I have created an experimental acpi-ec-fixes branch:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=acpi-ec-fixes
>>
>> for this which illustrates my idea (untested so far).
> 
> Thank you. I believe that the approach taken here is good and I also
> like the code (of the current version) so you may add my:
> 
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> 
> to all 3 patches.
> 
> I have started a test Fedora 40 6.10.4 kernel build with the patches from
> the acpi-ec-fixes branch added:
> 
> https://koji.fedoraproject.org/koji/taskinfo?taskID=121834209
> 
> and I have asked the reporters of both bugs:
> 
>     2298938 - Multiple ACPI errors resulting in incorrect thermal readings and misleading CPU 
>     2302253 - ACPI: EC: LG gram laptop brightness keys stop working with kernel >= 6.9.7
> 
> to test this. I expect a good turn around time from the reporter
> of bug 2302253. So far the reporter of 2298938 is not really
> responsive (holidays?).

The reporter of 2302253 has reported that the patches from:

https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=acpi-ec-fixes

resolve the issue; and as mentioned before the reporter of 2298938
is not responsive atm. So I believe that with it confirmed that this
at least fixes the issues on the LG Gram laptops (1) these patches are
ready to be merged now.

Regards,

Hans



1) And based on dmesg with a regressed kernel likely / hopefully also
the issue from RH bugzilla 2298938.

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Rafael J. Wysocki]

Hi,

On Tue, Aug 13, 2024 at 10:46 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>

[cut]

>
> The reporter of 2302253 has reported that the patches from:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=acpi-ec-fixes
>
> resolve the issue; and as mentioned before the reporter of 2298938
> is not responsive atm. So I believe that with it confirmed that this
> at least fixes the issues on the LG Gram laptops (1) these patches are
> ready to be merged now.

Thanks for the information!

I'll go ahead and put them into linux-next.

> 1) And based on dmesg with a regressed kernel likely / hopefully also
> the issue from RH bugzilla 2298938.
>

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Hans de Goede]

Hi Rafael,

On 8/13/24 12:40 PM, Rafael J. Wysocki wrote:
> Hi,
> 
> On Tue, Aug 13, 2024 at 10:46 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Hi,
>>
> 
> [cut]
> 
>>
>> The reporter of 2302253 has reported that the patches from:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=acpi-ec-fixes
>>
>> resolve the issue; and as mentioned before the reporter of 2298938
>> is not responsive atm. So I believe that with it confirmed that this
>> at least fixes the issues on the LG Gram laptops (1) these patches are
>> ready to be merged now.
> 
> Thanks for the information!
> 
> I'll go ahead and put them into linux-next.

Great. Question is the intention for these to go to Linus
in the 6.12 cycle, or do you just want to let them bake
a bit in linux-next and then plan to send them as a fix
for the 6.11 cycle ?

The reason I'm asking is because the patches these replace
cause a regression on some laptops models starting with 6.9.7
and also all 6.10.y kernels (and if you wait till 6.12,
also all 6.11.y) kernels.

So although this is a somewhat bigger change my personal
preference would be for these to get send out as fixes
and backported to kernels >= 6.9 (1).

Regards,

Hans



1) Just like how the patches they replace end up getting
backported to 6.9.7



> 
>> 1) And based on dmesg with a regressed kernel likely / hopefully also
>> the issue from RH bugzilla 2298938.
>>
> 

Re: "EC: Install address space handler at the namespace root" causing issues for some users

[Rafael J. Wysocki]

Hi Hans,

On Tue, Aug 13, 2024 at 12:45 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Rafael,
>
> On 8/13/24 12:40 PM, Rafael J. Wysocki wrote:
> > Hi,
> >
> > On Tue, Aug 13, 2024 at 10:46 AM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Hi,
> >>
> >
> > [cut]
> >
> >>
> >> The reporter of 2302253 has reported that the patches from:
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/log/?h=acpi-ec-fixes
> >>
> >> resolve the issue; and as mentioned before the reporter of 2298938
> >> is not responsive atm. So I believe that with it confirmed that this
> >> at least fixes the issues on the LG Gram laptops (1) these patches are
> >> ready to be merged now.
> >
> > Thanks for the information!
> >
> > I'll go ahead and put them into linux-next.
>
> Great. Question is the intention for these to go to Linus
> in the 6.12 cycle, or do you just want to let them bake
> a bit in linux-next and then plan to send them as a fix
> for the 6.11 cycle ?
>
> The reason I'm asking is because the patches these replace
> cause a regression on some laptops models starting with 6.9.7
> and also all 6.10.y kernels (and if you wait till 6.12,
> also all 6.11.y) kernels.
>
> So although this is a somewhat bigger change my personal
> preference would be for these to get send out as fixes
> and backported to kernels >= 6.9 (1).

Yes, that's my plan.

> 1) Just like how the patches they replace end up getting
> backported to 6.9.7
>
>
>
> >
> >> 1) And based on dmesg with a regressed kernel likely / hopefully also
> >> the issue from RH bugzilla 2298938.
> >>
> >
>