Re: [PATCH v1 01/17] ACPI: bus: Introduce devm_acpi_install_notify_handler()

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, May 21, 2026 at 03:59:50PM +0200, Rafael J. Wysocki wrote:

> Introduce devm_acpi_install_notify_handler() for installing an ACPI
> notify handler managed by devres that will be removed automatically on
> driver detach.
> 
> It installs the notify handler on the device object in the ACPI
> namespace that corresponds to the owner device's ACPI companion, if
> present (an error is returned if the owner device doesn't have an ACPI
> companion).
> 
> Currently, there is no way to manually remove the notify handler
> installed by it because none of its users brought on subsequently
> will need to do that.

...

> +static void devm_acpi_notify_handler_release(struct device *dev, void *res)
> +{
> +	struct acpi_notify_handler_devres *dr = res;

'dr' is usually associated with internal devres structures and might be
misleading in here, I would rename to something like handler_devres.

> +	acpi_dev_remove_notify_handler(ACPI_COMPANION(dev), dr->handler_type,

acpi_dev might be also part of the same data structure, so you won't need to
take dev again and derive adev from it.

> +				       dr->handler);
> +}

...

> +/**
> + * devm_acpi_install_notify_handler - Install an ACPI notify handler for a
> + * 				      managed device

There is a stray space just after asterisk.

> + * @dev: Device to install a notify handler for
> + * @handler_type: Type of the notify handler
> + * @handler: Handler function to install
> + * @context: Data passed back to the handler function
> + *
> + * This function performs the same function as acpi_dev_install_notify_handler()
> + * called for the ACPI companion of @dev with the same @handler_type, @handler,
> + * and @context arguments, but the ACPI notify handler installed by it will be
> + * automatically removed on driver detach.
> + *
> + * Callers should ensure that all resources used by @handler have been allocated
> + * prior to invoking this function, in which case those resources should be
> + * devres-managed so that they won't be released before the notify handler
> + * removal.  Otherwise, special synchronization between @handler and the
> + * management of those resources is required.
> + *
> + * When the request fails, an error message is printed with contextual
> + * information (device name, handler function and error code).  Don't add extra

This "handler function" points to __func__? If so, it seems misleading.

> + * error messages at the call sites.
> + *
> + * Return: 0 on success or a negative error number.
> + */
> +int devm_acpi_install_notify_handler(struct device *dev, u32 handler_type,
> +				     acpi_notify_handler handler, void *context)
> +{
> +	struct acpi_notify_handler_devres *dr;
> +	struct acpi_device *adev;
> +	int ret;
> +
> +	adev = ACPI_COMPANION(dev);
> +	if (!adev)
> +		return dev_err_probe(dev, -ENODEV, "No ACPI companion in %s()\n", __func__);

Not sure how __func__ may help here. We will have a device instance to be
printed. It's obvious then how to find the culprit call.

> +	dr = devres_alloc(devm_acpi_notify_handler_release, sizeof(*dr), GFP_KERNEL);
> +	if (!dr)
> +		return -ENOMEM;
> +
> +	ret = acpi_dev_install_notify_handler(adev, handler_type, handler, context);
> +	if (ret) {
> +		devres_free(dr);
> +		return dev_err_probe(dev, ret, "Failed to install an ACPI notify handler\n");
> +	}
> +
> +	dr->handler = handler;
> +	dr->handler_type = handler_type;
> +	devres_add(dev, dr);

> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko