2024-06-12 14:16:08

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH v1] ACPI: EC: Evaluate orphan _REG under EC device

From: Rafael J. Wysocki <[email protected]>

After starting to install the EC address space handler at the ACPI
namespace root, if there is an "orphan" _REG method in the EC device's
scope, it will not be evaluated any more. This breaks EC operation
regions on some systems, like Asus gu605.

To address this, use a wrapper around an existing ACPICA function to
look for an "orphan" _REG method in the EC device scope and evaluate
it if present.

Fixes: 60fa6ae6e6d0 ("ACPI: EC: Install address space handler at the namespace root")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218945
Reported-by: VitaliiT <[email protected]>
Tested-by: VitaliiT <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
---

Yes, this includes ACPICA changes that are obviously not upstream
and I am going to take care of pusing them to upstream, but for
now there is a regression to fix and it is relatively late in the
cycle.

---
drivers/acpi/acpica/acevents.h | 4 +++
drivers/acpi/acpica/evregion.c | 6 ----
drivers/acpi/acpica/evxfregn.c | 54 +++++++++++++++++++++++++++++++++++++++++
drivers/acpi/ec.c | 3 ++
include/acpi/acpixf.h | 4 +++
5 files changed, 66 insertions(+), 5 deletions(-)

Index: linux-pm/drivers/acpi/acpica/evxfregn.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/evxfregn.c
+++ linux-pm/drivers/acpi/acpica/evxfregn.c
@@ -306,3 +306,57 @@ acpi_execute_reg_methods(acpi_handle dev
}

ACPI_EXPORT_SYMBOL(acpi_execute_reg_methods)
+
+/*******************************************************************************
+ *
+ * FUNCTION: acpi_execute_orphan_reg_method
+ *
+ * PARAMETERS: device - Handle for the device
+ * space_id - The address space ID
+ *
+ * RETURN: Status
+ *
+ * DESCRIPTION: Execute an "orphan" _REG method that appears under an ACPI
+ * device. This is a _REG method that has no corresponding region
+ * within the device's scope.
+ *
+ ******************************************************************************/
+acpi_status
+acpi_execute_orphan_reg_method(acpi_handle device, acpi_adr_space_type space_id)
+{
+ struct acpi_namespace_node *node;
+ acpi_status status;
+
+ ACPI_FUNCTION_TRACE(acpi_execute_orphan_reg_method);
+
+ /* Parameter validation */
+
+ if (!device) {
+ return_ACPI_STATUS(AE_BAD_PARAMETER);
+ }
+
+ status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
+ if (ACPI_FAILURE(status)) {
+ return_ACPI_STATUS(status);
+ }
+
+ /* Convert and validate the device handle */
+
+ node = acpi_ns_validate_handle(device);
+ if (node) {
+
+ /*
+ * If an "orphan" _REG method is present in the device's scope
+ * for the given address space ID, run it.
+ */
+
+ acpi_ev_execute_orphan_reg_method(node, space_id);
+ } else {
+ status = AE_BAD_PARAMETER;
+ }
+
+ (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
+ return_ACPI_STATUS(status);
+}
+
+ACPI_EXPORT_SYMBOL(acpi_execute_orphan_reg_method)
Index: linux-pm/include/acpi/acpixf.h
===================================================================
--- linux-pm.orig/include/acpi/acpixf.h
+++ linux-pm/include/acpi/acpixf.h
@@ -663,6 +663,10 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status
acpi_adr_space_type
space_id))
ACPI_EXTERNAL_RETURN_STATUS(acpi_status
+ acpi_execute_orphan_reg_method(acpi_handle device,
+ acpi_adr_space_type
+ space_id))
+ACPI_EXTERNAL_RETURN_STATUS(acpi_status
acpi_remove_address_space_handler(acpi_handle
device,
acpi_adr_space_type
Index: linux-pm/drivers/acpi/acpica/acevents.h
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/acevents.h
+++ linux-pm/drivers/acpi/acpica/acevents.h
@@ -191,6 +191,10 @@ void
acpi_ev_execute_reg_methods(struct acpi_namespace_node *node,
acpi_adr_space_type space_id, u32 function);

+void
+acpi_ev_execute_orphan_reg_method(struct acpi_namespace_node *node,
+ acpi_adr_space_type space_id);
+
acpi_status
acpi_ev_execute_reg_method(union acpi_operand_object *region_obj, u32 function);

Index: linux-pm/drivers/acpi/acpica/evregion.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/evregion.c
+++ linux-pm/drivers/acpi/acpica/evregion.c
@@ -20,10 +20,6 @@ extern u8 acpi_gbl_default_address_space

/* Local prototypes */

-static void
-acpi_ev_execute_orphan_reg_method(struct acpi_namespace_node *device_node,
- acpi_adr_space_type space_id);
-
static acpi_status
acpi_ev_reg_run(acpi_handle obj_handle,
u32 level, void *context, void **return_value);
@@ -818,7 +814,7 @@ acpi_ev_reg_run(acpi_handle obj_handle,
*
******************************************************************************/

-static void
+void
acpi_ev_execute_orphan_reg_method(struct acpi_namespace_node *device_node,
acpi_adr_space_type space_id)
{
Index: linux-pm/drivers/acpi/ec.c
===================================================================
--- linux-pm.orig/drivers/acpi/ec.c
+++ linux-pm/drivers/acpi/ec.c
@@ -1507,6 +1507,9 @@ static int ec_install_handlers(struct ac

if (call_reg && !test_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags)) {
acpi_execute_reg_methods(scope_handle, ACPI_ADR_SPACE_EC);
+ if (scope_handle != ec->handle)
+ acpi_execute_orphan_reg_method(ec->handle, ACPI_ADR_SPACE_EC);
+
set_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags);
}






2024-06-16 09:47:34

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v1] ACPI: EC: Evaluate orphan _REG under EC device

Hi,

On 6/12/24 4:15 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> After starting to install the EC address space handler at the ACPI
> namespace root, if there is an "orphan" _REG method in the EC device's
> scope, it will not be evaluated any more. This breaks EC operation
> regions on some systems, like Asus gu605.
>
> To address this, use a wrapper around an existing ACPICA function to
> look for an "orphan" _REG method in the EC device scope and evaluate
> it if present.
>
> Fixes: 60fa6ae6e6d0 ("ACPI: EC: Install address space handler at the namespace root")
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218945
> Reported-by: VitaliiT <[email protected]>
> Tested-by: VitaliiT <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
>
> Yes, this includes ACPICA changes that are obviously not upstream
> and I am going to take care of pusing them to upstream, but for
> now there is a regression to fix and it is relatively late in the
> cycle.
>
> ---
> drivers/acpi/acpica/acevents.h | 4 +++
> drivers/acpi/acpica/evregion.c | 6 ----
> drivers/acpi/acpica/evxfregn.c | 54 +++++++++++++++++++++++++++++++++++++++++
> drivers/acpi/ec.c | 3 ++
> include/acpi/acpixf.h | 4 +++
> 5 files changed, 66 insertions(+), 5 deletions(-)
>
> Index: linux-pm/drivers/acpi/acpica/evxfregn.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/acpica/evxfregn.c
> +++ linux-pm/drivers/acpi/acpica/evxfregn.c
> @@ -306,3 +306,57 @@ acpi_execute_reg_methods(acpi_handle dev
> }
>
> ACPI_EXPORT_SYMBOL(acpi_execute_reg_methods)
> +
> +/*******************************************************************************
> + *
> + * FUNCTION: acpi_execute_orphan_reg_method
> + *
> + * PARAMETERS: device - Handle for the device
> + * space_id - The address space ID
> + *
> + * RETURN: Status
> + *
> + * DESCRIPTION: Execute an "orphan" _REG method that appears under an ACPI
> + * device. This is a _REG method that has no corresponding region
> + * within the device's scope.
> + *
> + ******************************************************************************/
> +acpi_status
> +acpi_execute_orphan_reg_method(acpi_handle device, acpi_adr_space_type space_id)
> +{
> + struct acpi_namespace_node *node;
> + acpi_status status;
> +
> + ACPI_FUNCTION_TRACE(acpi_execute_orphan_reg_method);
> +
> + /* Parameter validation */
> +
> + if (!device) {
> + return_ACPI_STATUS(AE_BAD_PARAMETER);
> + }
> +
> + status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
> + if (ACPI_FAILURE(status)) {
> + return_ACPI_STATUS(status);
> + }
> +
> + /* Convert and validate the device handle */
> +
> + node = acpi_ns_validate_handle(device);
> + if (node) {
> +
> + /*
> + * If an "orphan" _REG method is present in the device's scope
> + * for the given address space ID, run it.
> + */
> +
> + acpi_ev_execute_orphan_reg_method(node, space_id);
> + } else {
> + status = AE_BAD_PARAMETER;
> + }
> +
> + (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
> + return_ACPI_STATUS(status);
> +}
> +
> +ACPI_EXPORT_SYMBOL(acpi_execute_orphan_reg_method)
> Index: linux-pm/include/acpi/acpixf.h
> ===================================================================
> --- linux-pm.orig/include/acpi/acpixf.h
> +++ linux-pm/include/acpi/acpixf.h
> @@ -663,6 +663,10 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status
> acpi_adr_space_type
> space_id))
> ACPI_EXTERNAL_RETURN_STATUS(acpi_status
> + acpi_execute_orphan_reg_method(acpi_handle device,
> + acpi_adr_space_type
> + space_id))
> +ACPI_EXTERNAL_RETURN_STATUS(acpi_status
> acpi_remove_address_space_handler(acpi_handle
> device,
> acpi_adr_space_type
> Index: linux-pm/drivers/acpi/acpica/acevents.h
> ===================================================================
> --- linux-pm.orig/drivers/acpi/acpica/acevents.h
> +++ linux-pm/drivers/acpi/acpica/acevents.h
> @@ -191,6 +191,10 @@ void
> acpi_ev_execute_reg_methods(struct acpi_namespace_node *node,
> acpi_adr_space_type space_id, u32 function);
>
> +void
> +acpi_ev_execute_orphan_reg_method(struct acpi_namespace_node *node,
> + acpi_adr_space_type space_id);
> +
> acpi_status
> acpi_ev_execute_reg_method(union acpi_operand_object *region_obj, u32 function);
>
> Index: linux-pm/drivers/acpi/acpica/evregion.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/acpica/evregion.c
> +++ linux-pm/drivers/acpi/acpica/evregion.c
> @@ -20,10 +20,6 @@ extern u8 acpi_gbl_default_address_space
>
> /* Local prototypes */
>
> -static void
> -acpi_ev_execute_orphan_reg_method(struct acpi_namespace_node *device_node,
> - acpi_adr_space_type space_id);
> -
> static acpi_status
> acpi_ev_reg_run(acpi_handle obj_handle,
> u32 level, void *context, void **return_value);
> @@ -818,7 +814,7 @@ acpi_ev_reg_run(acpi_handle obj_handle,
> *
> ******************************************************************************/
>
> -static void
> +void
> acpi_ev_execute_orphan_reg_method(struct acpi_namespace_node *device_node,
> acpi_adr_space_type space_id)
> {
> Index: linux-pm/drivers/acpi/ec.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/ec.c
> +++ linux-pm/drivers/acpi/ec.c
> @@ -1507,6 +1507,9 @@ static int ec_install_handlers(struct ac
>
> if (call_reg && !test_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags)) {
> acpi_execute_reg_methods(scope_handle, ACPI_ADR_SPACE_EC);
> + if (scope_handle != ec->handle)
> + acpi_execute_orphan_reg_method(ec->handle, ACPI_ADR_SPACE_EC);
> +
> set_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags);
> }
>

TL;DR: this change made me wonder about a possible issue, but all is
fine, except that the code flow leading to "fine" is a bit convoluted.

I noticed this landing in Linus' tree and this got me thinking about
if the "if (scope_handle != ec->handle)" would not cause the orphan
_REG method to not get called for the case where the EC is created
by parsing the ECDT early on, since then we set
ec->handle = ACPI_ROOT_OBJECT for the ec object.

So I checked and acpi_ec_ecdt_probe() calls acpi_ec_setup(ec, NULL, false)
with the false making call_reg above false, so the _REG methods do not
get executed at this point.

Instead they should get executed when parsing the DSDT finds the EC ACPI
device by acpi_ec_add() calling acpi_ec_setup(ec, device, true).

acpi_ec_add() updated ec->handle away from ACPI_ROOT_OBJECT to the actual
acpi_device's handle before calling acpi_ec_setup(ec, device, true) so
all should be well.

But while checkimg this I noticed a pre-existing issue where if the boot_ec
object is created from the ECDT, so with ec->handle == NULL, acpi_ec_add()
does not update ec->handle, which means an orphan _REG method will not get
executed.

Specifically if this if condition evaluated to true, because the !strcmp()
evaluates to true:

if (boot_ec && (boot_ec->handle == device->handle ||
!strcmp(acpi_device_hid(device), ACPI_ECDT_HID))) {
/* Fast path: this device corresponds to the boot EC. */
ec = boot_ec;
} else {

then [boot_]ec->handle does not get set to device->handle .

So at a first glance it looks like we need something like this to make sure
that any orphan _REG methods are also run in the described scenario:

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index e7793ee9e649..af61b9bb3749 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -1635,6 +1635,7 @@ static int acpi_ec_add(struct acpi_device *device)
!strcmp(acpi_device_hid(device), ACPI_ECDT_HID))) {
/* Fast path: this device corresponds to the boot EC. */
ec = boot_ec;
+ ec->handle = device->handle;
} else {
acpi_status status;



But upon further checking the only place creating an acpi_device
with an ACPI_ECDT_HID is acpi_ec_ecdt_start() which does:

status = acpi_get_handle(NULL, ecdt_ptr->id, &handle);
if (ACPI_SUCCESS(status)) {
boot_ec->handle = handle;

/* Add a special ACPI device object to represent the boot EC. */
acpi_bus_register_early_device(ACPI_BUS_TYPE_ECDT_EC);
}

So boot_ec->handle gets updated in this case before acpi_ec_add() runs
and everything is fine.

...

Still I'm wondering if it would not be better to replace this
"boot_ec->handle = handle;" in acpi_ec_ecdt_start() with the change
from my proposed diff above, so that we consistently about the
[boot_]ec->handle with device->handle for the being added acpi_device
in both branches of the if ... else ... in acpi_ec_add() ?

Regards,

Hans


2024-06-16 10:00:40

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v1] ACPI: EC: Evaluate orphan _REG under EC device

Hi,

On 6/16/24 11:47 AM, Hans de Goede wrote:
> Hi,
>
> On 6/12/24 4:15 PM, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <[email protected]>
>>
>> After starting to install the EC address space handler at the ACPI
>> namespace root, if there is an "orphan" _REG method in the EC device's
>> scope, it will not be evaluated any more. This breaks EC operation
>> regions on some systems, like Asus gu605.
>>
>> To address this, use a wrapper around an existing ACPICA function to
>> look for an "orphan" _REG method in the EC device scope and evaluate
>> it if present.
>>
>> Fixes: 60fa6ae6e6d0 ("ACPI: EC: Install address space handler at the namespace root")
>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218945
>> Reported-by: VitaliiT <[email protected]>
>> Tested-by: VitaliiT <[email protected]>
>> Signed-off-by: Rafael J. Wysocki <[email protected]>
>> ---
>>
>> Yes, this includes ACPICA changes that are obviously not upstream
>> and I am going to take care of pusing them to upstream, but for
>> now there is a regression to fix and it is relatively late in the
>> cycle.
>>
>> ---
>> drivers/acpi/acpica/acevents.h | 4 +++
>> drivers/acpi/acpica/evregion.c | 6 ----
>> drivers/acpi/acpica/evxfregn.c | 54 +++++++++++++++++++++++++++++++++++++++++
>> drivers/acpi/ec.c | 3 ++
>> include/acpi/acpixf.h | 4 +++
>> 5 files changed, 66 insertions(+), 5 deletions(-)
>>
>> Index: linux-pm/drivers/acpi/acpica/evxfregn.c
>> ===================================================================
>> --- linux-pm.orig/drivers/acpi/acpica/evxfregn.c
>> +++ linux-pm/drivers/acpi/acpica/evxfregn.c
>> @@ -306,3 +306,57 @@ acpi_execute_reg_methods(acpi_handle dev
>> }
>>
>> ACPI_EXPORT_SYMBOL(acpi_execute_reg_methods)
>> +
>> +/*******************************************************************************
>> + *
>> + * FUNCTION: acpi_execute_orphan_reg_method
>> + *
>> + * PARAMETERS: device - Handle for the device
>> + * space_id - The address space ID
>> + *
>> + * RETURN: Status
>> + *
>> + * DESCRIPTION: Execute an "orphan" _REG method that appears under an ACPI
>> + * device. This is a _REG method that has no corresponding region
>> + * within the device's scope.
>> + *
>> + ******************************************************************************/
>> +acpi_status
>> +acpi_execute_orphan_reg_method(acpi_handle device, acpi_adr_space_type space_id)
>> +{
>> + struct acpi_namespace_node *node;
>> + acpi_status status;
>> +
>> + ACPI_FUNCTION_TRACE(acpi_execute_orphan_reg_method);
>> +
>> + /* Parameter validation */
>> +
>> + if (!device) {
>> + return_ACPI_STATUS(AE_BAD_PARAMETER);
>> + }
>> +
>> + status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
>> + if (ACPI_FAILURE(status)) {
>> + return_ACPI_STATUS(status);
>> + }
>> +
>> + /* Convert and validate the device handle */
>> +
>> + node = acpi_ns_validate_handle(device);
>> + if (node) {
>> +
>> + /*
>> + * If an "orphan" _REG method is present in the device's scope
>> + * for the given address space ID, run it.
>> + */
>> +
>> + acpi_ev_execute_orphan_reg_method(node, space_id);
>> + } else {
>> + status = AE_BAD_PARAMETER;
>> + }
>> +
>> + (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
>> + return_ACPI_STATUS(status);
>> +}
>> +
>> +ACPI_EXPORT_SYMBOL(acpi_execute_orphan_reg_method)
>> Index: linux-pm/include/acpi/acpixf.h
>> ===================================================================
>> --- linux-pm.orig/include/acpi/acpixf.h
>> +++ linux-pm/include/acpi/acpixf.h
>> @@ -663,6 +663,10 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>> acpi_adr_space_type
>> space_id))
>> ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>> + acpi_execute_orphan_reg_method(acpi_handle device,
>> + acpi_adr_space_type
>> + space_id))
>> +ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>> acpi_remove_address_space_handler(acpi_handle
>> device,
>> acpi_adr_space_type
>> Index: linux-pm/drivers/acpi/acpica/acevents.h
>> ===================================================================
>> --- linux-pm.orig/drivers/acpi/acpica/acevents.h
>> +++ linux-pm/drivers/acpi/acpica/acevents.h
>> @@ -191,6 +191,10 @@ void
>> acpi_ev_execute_reg_methods(struct acpi_namespace_node *node,
>> acpi_adr_space_type space_id, u32 function);
>>
>> +void
>> +acpi_ev_execute_orphan_reg_method(struct acpi_namespace_node *node,
>> + acpi_adr_space_type space_id);
>> +
>> acpi_status
>> acpi_ev_execute_reg_method(union acpi_operand_object *region_obj, u32 function);
>>
>> Index: linux-pm/drivers/acpi/acpica/evregion.c
>> ===================================================================
>> --- linux-pm.orig/drivers/acpi/acpica/evregion.c
>> +++ linux-pm/drivers/acpi/acpica/evregion.c
>> @@ -20,10 +20,6 @@ extern u8 acpi_gbl_default_address_space
>>
>> /* Local prototypes */
>>
>> -static void
>> -acpi_ev_execute_orphan_reg_method(struct acpi_namespace_node *device_node,
>> - acpi_adr_space_type space_id);
>> -
>> static acpi_status
>> acpi_ev_reg_run(acpi_handle obj_handle,
>> u32 level, void *context, void **return_value);
>> @@ -818,7 +814,7 @@ acpi_ev_reg_run(acpi_handle obj_handle,
>> *
>> ******************************************************************************/
>>
>> -static void
>> +void
>> acpi_ev_execute_orphan_reg_method(struct acpi_namespace_node *device_node,
>> acpi_adr_space_type space_id)
>> {
>> Index: linux-pm/drivers/acpi/ec.c
>> ===================================================================
>> --- linux-pm.orig/drivers/acpi/ec.c
>> +++ linux-pm/drivers/acpi/ec.c
>> @@ -1507,6 +1507,9 @@ static int ec_install_handlers(struct ac
>>
>> if (call_reg && !test_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags)) {
>> acpi_execute_reg_methods(scope_handle, ACPI_ADR_SPACE_EC);
>> + if (scope_handle != ec->handle)
>> + acpi_execute_orphan_reg_method(ec->handle, ACPI_ADR_SPACE_EC);
>> +
>> set_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags);
>> }
>>
>
> TL;DR: this change made me wonder about a possible issue, but all is
> fine, except that the code flow leading to "fine" is a bit convoluted.
>
> I noticed this landing in Linus' tree and this got me thinking about
> if the "if (scope_handle != ec->handle)" would not cause the orphan
> _REG method to not get called for the case where the EC is created
> by parsing the ECDT early on, since then we set
> ec->handle = ACPI_ROOT_OBJECT for the ec object.
>
> So I checked and acpi_ec_ecdt_probe() calls acpi_ec_setup(ec, NULL, false)
> with the false making call_reg above false, so the _REG methods do not
> get executed at this point.
>
> Instead they should get executed when parsing the DSDT finds the EC ACPI
> device by acpi_ec_add() calling acpi_ec_setup(ec, device, true).
>
> acpi_ec_add() updated ec->handle away from ACPI_ROOT_OBJECT to the actual
> acpi_device's handle before calling acpi_ec_setup(ec, device, true) so
> all should be well.
>
> But while checkimg this I noticed a pre-existing issue where if the boot_ec
> object is created from the ECDT, so with ec->handle == NULL, acpi_ec_add()
> does not update ec->handle, which means an orphan _REG method will not get
> executed.
>
> Specifically if this if condition evaluated to true, because the !strcmp()
> evaluates to true:
>
> if (boot_ec && (boot_ec->handle == device->handle ||
> !strcmp(acpi_device_hid(device), ACPI_ECDT_HID))) {
> /* Fast path: this device corresponds to the boot EC. */
> ec = boot_ec;
> } else {
>
> then [boot_]ec->handle does not get set to device->handle .
>
> So at a first glance it looks like we need something like this to make sure
> that any orphan _REG methods are also run in the described scenario:
>
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index e7793ee9e649..af61b9bb3749 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -1635,6 +1635,7 @@ static int acpi_ec_add(struct acpi_device *device)
> !strcmp(acpi_device_hid(device), ACPI_ECDT_HID))) {
> /* Fast path: this device corresponds to the boot EC. */
> ec = boot_ec;
> + ec->handle = device->handle;
> } else {
> acpi_status status;
>
>
>
> But upon further checking the only place creating an acpi_device
> with an ACPI_ECDT_HID is acpi_ec_ecdt_start() which does:
>
> status = acpi_get_handle(NULL, ecdt_ptr->id, &handle);
> if (ACPI_SUCCESS(status)) {
> boot_ec->handle = handle;
>
> /* Add a special ACPI device object to represent the boot EC. */
> acpi_bus_register_early_device(ACPI_BUS_TYPE_ECDT_EC);
> }
>
> So boot_ec->handle gets updated in this case before acpi_ec_add() runs
> and everything is fine.
>
> ...
>
> Still I'm wondering if it would not be better to replace this
> "boot_ec->handle = handle;" in acpi_ec_ecdt_start() with the change
> from my proposed diff above, so that we consistently about the
> [boot_]ec->handle with device->handle for the being added acpi_device
> in both branches of the if ... else ... in acpi_ec_add() ?

Never mind that will not work because then we would set boot_ec->handle
to the handle of the special / fake ACPI device object added to
represent the boot EC, rather then to the handle corresponding to
ecdt_ptr->id, so this is a bad idea.

Regards,

Hans