Re: Patch "ACPI: EC: Install address space handler at the namespace root" has been added to the 6.6-stable tree

["Wysocki, Rafael J" <rafael.j.wysocki@xxxxxxxxx>]

On 6/21/2024 5:47 PM, Sasha Levin wrote:
> This is a note to let you know that I've just added the patch titled
>
>      ACPI: EC: Install address space handler at the namespace root

For this, you need

https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=acpi&id=0e6b6dedf16800df0ff73ffe2bb5066514db29c2

too (here and for 6.9).

Thanks!

> to the 6.6-stable tree which can be found at:
>      http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
>       acpi-ec-install-address-space-handler-at-the-namespa.patch
> and it can be found in the queue-6.6 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@xxxxxxxxxxxxxxx> know about it.
>
>
>
> commit ee44236dfbf5541d5fbcb52db961616292c84c0d
> Author: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> Date:   Wed May 15 21:40:54 2024 +0200
>
>      ACPI: EC: Install address space handler at the namespace root
>      
>      [ Upstream commit 60fa6ae6e6d09e377fce6f8d9b6f6a4d88769f63 ]
>      
>      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 if there is no ECDT, 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 for the first EC that
>      can be found regardless of whether or not an ECDT is present.
>      
>      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.
>      
>      Reported-by: webcaptcha <webcapcha@xxxxxxxxx>
>      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@xxxxxxxxxxxxxxxxx
>      Suggested-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
>      Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>      Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>      Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx>
>      Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>      Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
>
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index a59c11df73754..0795f92d8927d 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -1482,13 +1482,14 @@ static bool install_gpio_irq_event_handler(struct acpi_ec *ec)
>   static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device,
>   			       bool call_reg)
>   {
> +	acpi_handle scope_handle = ec == first_ec ? ACPI_ROOT_OBJECT : ec->handle;
>   	acpi_status status;
>   
>   	acpi_ec_start(ec, false);
>   
>   	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(scope_handle,
>   								   ACPI_ADR_SPACE_EC,
>   								   &acpi_ec_space_handler,
>   								   NULL, ec);
> @@ -1497,11 +1498,10 @@ static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device,
>   			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(scope_handle, ACPI_ADR_SPACE_EC);
>   		set_bit(EC_FLAGS_EC_REG_CALLED, &ec->flags);
>   	}
>   
> @@ -1553,10 +1553,13 @@ static int ec_install_handlers(struct acpi_ec *ec, struct acpi_device *device,
>   
>   static void ec_remove_handlers(struct acpi_ec *ec)
>   {
> +	acpi_handle scope_handle = ec == first_ec ? ACPI_ROOT_OBJECT : ec->handle;
> +
>   	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)))
> +						scope_handle,
> +						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);
>   	}
> @@ -1595,14 +1598,18 @@ static int acpi_ec_setup(struct acpi_ec *ec, struct acpi_device *device, bool ca
>   {
>   	int ret;
>   
> -	ret = ec_install_handlers(ec, device, call_reg);
> -	if (ret)
> -		return ret;
> -
>   	/* First EC capable of handling transactions */
>   	if (!first_ec)
>   		first_ec = ec;
>   
> +	ret = ec_install_handlers(ec, device, call_reg);
> +	if (ret) {
> +		if (ec == first_ec)
> +			first_ec = NULL;
> +
> +		return ret;
> +	}
> +
>   	pr_info("EC_CMD/EC_SC=0x%lx, EC_DATA=0x%lx\n", ec->command_addr,
>   		ec->data_addr);
>   
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 866c7c4ed2331..6db1a03dd5399 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -167,7 +167,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;