Re: [PATCH v1 2/2] platform/x86: wmi: Remove custom EC address space handler

[Armin Wolf]

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

> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> The custom EC address space handler in the WMI driver was only needed
> because the EC driver did not install its address space handler for
> EC operation regions beyond the EC device scope in the ACPI namespace.
>
> That has just changed, so the custom EC address handler is not needed
> any more and it can be removed.

The patch seems ok to me, but it might conflict with pdx86/for-next.

Other than that:
Reviewed-by: Armin Wolf <W_Armin@xxxxxx>

> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> ---
>   drivers/platform/x86/wmi.c |   62 ---------------------------------------------
>   1 file changed, 62 deletions(-)
>
> Index: linux-pm/drivers/platform/x86/wmi.c
> ===================================================================
> --- linux-pm.orig/drivers/platform/x86/wmi.c
> +++ linux-pm/drivers/platform/x86/wmi.c
> @@ -1153,47 +1153,6 @@ static int parse_wdg(struct device *wmi_
>   	return 0;
>   }
>
> -/*
> - * WMI can have EmbeddedControl access regions. In which case, we just want to
> - * hand these off to the EC driver.
> - */
> -static acpi_status
> -acpi_wmi_ec_space_handler(u32 function, acpi_physical_address address,
> -			  u32 bits, u64 *value,
> -			  void *handler_context, void *region_context)
> -{
> -	int result = 0;
> -	u8 temp = 0;
> -
> -	if ((address > 0xFF) || !value)
> -		return AE_BAD_PARAMETER;
> -
> -	if (function != ACPI_READ && function != ACPI_WRITE)
> -		return AE_BAD_PARAMETER;
> -
> -	if (bits != 8)
> -		return AE_BAD_PARAMETER;
> -
> -	if (function == ACPI_READ) {
> -		result = ec_read(address, &temp);
> -		*value = temp;
> -	} else {
> -		temp = 0xff & *value;
> -		result = ec_write(address, temp);
> -	}
> -
> -	switch (result) {
> -	case -EINVAL:
> -		return AE_BAD_PARAMETER;
> -	case -ENODEV:
> -		return AE_NOT_FOUND;
> -	case -ETIME:
> -		return AE_TIME;
> -	default:
> -		return AE_OK;
> -	}
> -}
> -
>   static int wmi_get_notify_data(struct wmi_block *wblock, union acpi_object **obj)
>   {
>   	struct acpi_buffer data = { ACPI_ALLOCATE_BUFFER, NULL };
> @@ -1308,14 +1267,6 @@ static void acpi_wmi_remove_notify_handl
>   	acpi_remove_notify_handler(acpi_device->handle, ACPI_ALL_NOTIFY, acpi_wmi_notify_handler);
>   }
>
> -static void acpi_wmi_remove_address_space_handler(void *data)
> -{
> -	struct acpi_device *acpi_device = data;
> -
> -	acpi_remove_address_space_handler(acpi_device->handle, ACPI_ADR_SPACE_EC,
> -					  &acpi_wmi_ec_space_handler);
> -}
> -
>   static void acpi_wmi_remove_bus_device(void *data)
>   {
>   	struct device *wmi_bus_dev = data;
> @@ -1347,19 +1298,6 @@ static int acpi_wmi_probe(struct platfor
>
>   	dev_set_drvdata(&device->dev, wmi_bus_dev);
>
> -	status = acpi_install_address_space_handler(acpi_device->handle,
> -						    ACPI_ADR_SPACE_EC,
> -						    &acpi_wmi_ec_space_handler,
> -						    NULL, NULL);
> -	if (ACPI_FAILURE(status)) {
> -		dev_err(&device->dev, "Error installing EC region handler\n");
> -		return -ENODEV;
> -	}
> -	error = devm_add_action_or_reset(&device->dev, acpi_wmi_remove_address_space_handler,
> -					 acpi_device);
> -	if (error < 0)
> -		return error;
> -
>   	status = acpi_install_notify_handler(acpi_device->handle, ACPI_ALL_NOTIFY,
>   					     acpi_wmi_notify_handler, wmi_bus_dev);
>   	if (ACPI_FAILURE(status)) {