Re: [PATCH v4 10/35] hwmon/acpi_power_meter: Move handler installing logic to driver

[Guenter Roeck <linux@xxxxxxxxxxxx>]

On 6/1/23 06:17, Michal Wilczynski wrote:
> Currently logic for installing notifications from ACPI devices is
> implemented using notify callback in struct acpi_driver. Preparations
> are being made to replace acpi_driver with more generic struct
> platform_driver, which doesn't contain notify callback. Furthermore
> as of now handlers are being called indirectly through
> acpi_notify_device(), which decreases performance.
>
> Call acpi_device_install_event_handler() at the end of .add() callback.
> Call acpi_device_remove_event_handler() at the beginning of .remove()
> callback. Change arguments passed to the notify callback to match with
> what's required by acpi_device_install_event_handler().
>
> Signed-off-by: Michal Wilczynski <michal.wilczynski@xxxxxxxxx>

Acked-by: Guenter Roeck <linux@xxxxxxxxxxxx>

> ---
>   drivers/hwmon/acpi_power_meter.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c
> index fa28d447f0df..7410ee8693ba 100644
> --- a/drivers/hwmon/acpi_power_meter.c
> +++ b/drivers/hwmon/acpi_power_meter.c
> @@ -817,9 +817,10 @@ static int read_capabilities(struct acpi_power_meter_resource *resource)
>   }
>   
>   /* Handle ACPI event notifications */
> -static void acpi_power_meter_notify(struct acpi_device *device, u32 event)
> +static void acpi_power_meter_notify(acpi_handle handle, u32 event, void *data)
>   {
>   	struct acpi_power_meter_resource *resource;
> +	struct acpi_device *device = data;
>   	int res;
>   
>   	if (!device || !acpi_driver_data(device))
> @@ -897,8 +898,12 @@ static int acpi_power_meter_add(struct acpi_device *device)
>   		goto exit_remove;
>   	}
>   
> -	res = 0;
> -	goto exit;
> +	res = acpi_device_install_event_handler(device, ACPI_DEVICE_NOTIFY,
> +						acpi_power_meter_notify);
> +	if (res)
> +		goto exit_remove;
> +
> +	return 0;
>   
>   exit_remove:
>   	remove_attrs(resource);
> @@ -906,7 +911,7 @@ static int acpi_power_meter_add(struct acpi_device *device)
>   	free_capabilities(resource);
>   exit_free:
>   	kfree(resource);
> -exit:
> +
>   	return res;
>   }
>   
> @@ -917,6 +922,7 @@ static void acpi_power_meter_remove(struct acpi_device *device)
>   	if (!device || !acpi_driver_data(device))
>   		return;
>   
> +	acpi_device_remove_event_handler(device, ACPI_DEVICE_NOTIFY, acpi_power_meter_notify);
>   	resource = acpi_driver_data(device);
>   	hwmon_device_unregister(resource->hwmon_dev);
>   
> @@ -953,7 +959,6 @@ static struct acpi_driver acpi_power_meter_driver = {
>   	.ops = {
>   		.add = acpi_power_meter_add,
>   		.remove = acpi_power_meter_remove,
> -		.notify = acpi_power_meter_notify,
>   		},
>   	.drv.pm = pm_sleep_ptr(&acpi_power_meter_pm),
>   };