[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ac04c433-b0ac-4b82-b8eb-98ac16f872d8@gmx.de>
Date: Fri, 10 May 2024 18:10:42 +0200
From: Armin Wolf <W_Armin@....de>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>,
Linux ACPI <linux-acpi@...r.kernel.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Hans de Goede <hdegoede@...hat.com>,
Mario Limonciello <mario.limonciello@....com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Subject: Re: [PATCH v1 1/2] ACPI: EC: Install address space handler at the
namespace root
Am 10.05.24 um 16:03 schrieb Rafael J. Wysocki:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> It is reported that _DSM evaluation fails in ucsi_acpi_dsm() on Lenovo
> IdeaPad Pro 5 due to a missing address space handler for the EC address
> space:
>
> ACPI Error: No handler for Region [ECSI] (000000007b8176ee) [EmbeddedControl] (20230628/evregion-130)
>
> This happens because the EC driver only registers the EC address space
> handler for operation regions defined in the EC device scope of the
> ACPI namespace while the operation region being accessed by the _DSM
> in question is located beyond that scope.
>
> To address this, modify the ACPI EC driver to install the EC address
> space handler at the root of the ACPI namespace.
>
> Note that this change is consistent with some examples in the ACPI
> specification in which EC operation regions located outside the EC
> device scope are used (for example, see Section 9.17.15 in ACPI 6.5),
> so the current behavior of the EC driver is arguably questionable.
Hi,
the patch itself looks good to me, but i wonder what happens if multiple
ACPI EC devices are present. How would we handle such a situation?
Thanks,
Armin Wolf
> Reported-by: webcaptcha <webcapcha@...il.com>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218789
> Link: https://uefi.org/specs/ACPI/6.5/09_ACPI_Defined_Devices_and_Device_Specific_Objects.html#example-asl-code
> Link: https://lore.kernel.org/linux-acpi/Zi+0whTvDbAdveHq@kuha.fi.intel.com
> Suggested-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> drivers/acpi/ec.c | 10 +++++-----
> drivers/acpi/internal.h | 1 -
> 2 files changed, 5 insertions(+), 6 deletions(-)
>
> Index: linux-pm/drivers/acpi/ec.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/ec.c
> +++ linux-pm/drivers/acpi/ec.c
> @@ -1488,7 +1488,7 @@ static int ec_install_handlers(struct ac
>
> if (!test_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)) {
> acpi_ec_enter_noirq(ec);
> - status = acpi_install_address_space_handler_no_reg(ec->handle,
> + status = acpi_install_address_space_handler_no_reg(ACPI_ROOT_OBJECT,
> ACPI_ADR_SPACE_EC,
> &acpi_ec_space_handler,
> NULL, ec);
> @@ -1497,11 +1497,10 @@ static int ec_install_handlers(struct ac
> return -ENODEV;
> }
> set_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags);
> - ec->address_space_handler_holder = ec->handle;
> }
>
> if (call_reg && !test_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags)) {
> - acpi_execute_reg_methods(ec->handle, ACPI_ADR_SPACE_EC);
> + acpi_execute_reg_methods(ACPI_ROOT_OBJECT, ACPI_ADR_SPACE_EC);
> set_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags);
> }
>
> @@ -1555,8 +1554,9 @@ static void ec_remove_handlers(struct ac
> {
> if (test_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags)) {
> if (ACPI_FAILURE(acpi_remove_address_space_handler(
> - ec->address_space_handler_holder,
> - ACPI_ADR_SPACE_EC, &acpi_ec_space_handler)))
> + ACPI_ROOT_OBJECT,
> + ACPI_ADR_SPACE_EC,
> + &acpi_ec_space_handler)))
> pr_err("failed to remove space handler\n");
> clear_bit(EC_FLAGS_EC_HANDLER_INSTALLED, &ec->flags);
> }
> Index: linux-pm/drivers/acpi/internal.h
> ===================================================================
> --- linux-pm.orig/drivers/acpi/internal.h
> +++ linux-pm/drivers/acpi/internal.h
> @@ -186,7 +186,6 @@ enum acpi_ec_event_state {
>
> struct acpi_ec {
> acpi_handle handle;
> - acpi_handle address_space_handler_holder;
> int gpe;
> int irq;
> unsigned long command_addr;
>
>
>
Powered by blists - more mailing lists