On Tue, Jun 2, 2026 at 8:50 PM Andy Shevchenko
<[email protected]> wrote:
>
> 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.
Well, whatever.
> > + 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.
I'm not sure what you mean.
Put acpi_dev into struct acpi_notify_handler_devres? That can be done
in a follow-up patch.
> > + dr->handler);
> > +}
>
> ...
>
> > +/**
> > + * devm_acpi_install_notify_handler - Install an ACPI notify handler for a
> > + * managed device
>
> There is a stray space just after asterisk.
Which 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.
Yes, this sentence should just be "When the request fails, an error
message is printed" without the "contextual information" part.
> > + * 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.
But it doesn't hurt either, does it?
> > + 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;
> > +}
>
> --
So thanks for the review, but I don't think I want to send a v2 at this point.
I'd rather send a follow-up patch to clean up these things.
