Received-SPF: pass (google.com: domain of linux-kernel+bounces-175997-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <ac04c433-b0ac-4b82-b8eb-98ac16f872d8@gmx.de>
Date: Fri, 10 May 2024 18:10:42 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 1/2] ACPI: EC: Install address space handler at the
 namespace root
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
 Linux ACPI <linux-acpi@vger.kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
 Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
 Hans de Goede <hdegoede@redhat.com>,
 Mario Limonciello <mario.limonciello@amd.com>,
 Heikki Krogerus <heikki.krogerus@linux.intel.com>
References: <5787281.DvuYhMxLoT@kreacher> <4926735.31r3eYUQgx@kreacher>
Content-Language: en-US
From: Armin Wolf <W_Armin@gmx.de>
In-Reply-To: <4926735.31r3eYUQgx@kreacher>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:B1Q9O29DO6k=;5JAa00wiMzS+9jZ5sBBfJXlIKvS
 ippXqs6y/q0bvs2T9xaBYAlJAayYnQUTHOxf4D/mOoSbtZAttUsXoyb0orxwYosAgL7QQ4cPR
 q3Ag/WlGGeNH/vsYQ9gTB/XRIvQ7m2WEr59Wa06UrLR2JN8vvn9Wm50oQgW96ViV1L/rL7rtq
 xFqZyi9evjXCkVRmn7inaS3W8/J7TseuOp606rnjJxf3ECWH3Q9McW4na3q6/a6Gtv9x3ovWG
 omSNmbe+gjyPxuLOo4iRpaMJN6JbawpCXGeE1xQ+ISJJibbZxvVM0ztfPLzXgOJyR/dUkCRYQ
 06d7/QqSS1H37NIeUH9LMkWo4WCMFCLT0vMdGdFwTqa/MUTj7uHIkZIua/1NNSf2o00UoAml8
 e5oUSBVNmSuWjWWL+vUMOgF6TDd4erprYxMqd0fIZEn+dYTUyYPx+j0WruUwvZYlueWJrco8h
 QsBVSex3BchnTMurJbpBNokazNs71bRhpCXKArW7TppKsvB1Q8yrcHjsBtC+M727w04Esuwml
 yL8ZsANJZuA1vLMRybJ/pDYdUFduehFwDogFP2joaDUii+B+i3VSXR280xUOhVNvgvfcv8tCH
 Tm0V7TIo5ZG+tT+TSmn7hAJqjHXSiJNOJJW7aWi2ba4z4xNbmfS89ZZHR3nDbuT+glBmqkx7z
 uv5UEK7sswUo7ugVDP1oYit7jWZm+xRCbjn3c9gcyi07MIfU6frDbr8lP9EOABF8Vn3+IuPDl
 olBBlEGD63e1H6JApPH+xs7tVE+YCJ+87vluubpDae11bNaqhlpt9j71wABOF7mkbdGAZqLvz
 YAkoUv+fdn8evLRs6bHAQ2Gl6r94JcOjjJnRpWKhiag3o=

Am 10.05.24 um 16:03 schrieb Rafael J. Wysocki:

> From: Rafael J. Wysocki <rafael.j.wysocki@intel.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) [EmbeddedC=
ontrol] (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@gmail.com>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=3D218789
> 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@linux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.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
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- 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 =3D acpi_install_address_space_handler_no_reg(ec->handle,
> +		status =3D 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 =3D 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
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- 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;
>
>
>