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

Rafael J. Wysocki posted 1 patch 3 weeks, 2 days ago
drivers/acpi/bus.c      | 67 +++++++++++++++++++++++++++++++++++++++++
include/acpi/acpi_bus.h |  2 ++
2 files changed, 69 insertions(+)
[PATCH v1 01/17] ACPI: bus: Introduce devm_acpi_install_notify_handler()
Posted by Rafael J. Wysocki 3 weeks, 2 days ago
From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

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.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/bus.c      | 67 +++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_bus.h |  2 ++
 2 files changed, 69 insertions(+)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 2ec095e2009e..84f0ab47fd40 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -679,6 +679,73 @@ void acpi_dev_remove_notify_handler(struct acpi_device *adev,
 }
 EXPORT_SYMBOL_GPL(acpi_dev_remove_notify_handler);
 
+struct acpi_notify_handler_devres {
+	acpi_notify_handler handler;
+	u32 handler_type;
+};
+
+static void devm_acpi_notify_handler_release(struct device *dev, void *res)
+{
+	struct acpi_notify_handler_devres *dr = res;
+
+	acpi_dev_remove_notify_handler(ACPI_COMPANION(dev), dr->handler_type,
+				       dr->handler);
+}
+
+/**
+ * devm_acpi_install_notify_handler - Install an ACPI notify handler for a
+ * 				      managed device
+ * @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
+ * 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__);
+
+	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;
+}
+EXPORT_SYMBOL_GPL(devm_acpi_install_notify_handler);
+
 /* Handle events targeting \_SB device (at present only graceful shutdown) */
 
 #define ACPI_SB_NOTIFY_SHUTDOWN_REQUEST 0x81
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index c41d9a7565cf..7e57f9698f7c 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -629,6 +629,8 @@ int acpi_dev_install_notify_handler(struct acpi_device *adev,
 void acpi_dev_remove_notify_handler(struct acpi_device *adev,
 				    u32 handler_type,
 				    acpi_notify_handler handler);
+int devm_acpi_install_notify_handler(struct device *dev, u32 handler_type,
+				     acpi_notify_handler handler, void *context);
 extern int acpi_notifier_call_chain(const char *device_class,
 				    const char *bus_id, u32 type, u32 data);
 extern int register_acpi_notifier(struct notifier_block *);
-- 
2.51.0
Re: [PATCH v1 01/17] ACPI: bus: Introduce devm_acpi_install_notify_handler()
Posted by Andy Shevchenko 1 week, 4 days ago
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
Re: [PATCH v1 01/17] ACPI: bus: Introduce devm_acpi_install_notify_handler()
Posted by Rafael J. Wysocki 1 week, 4 days ago
On Tue, Jun 2, 2026 at 8:50 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> 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.
Re: [PATCH v1 01/17] ACPI: bus: Introduce devm_acpi_install_notify_handler()
Posted by Andy Shevchenko 1 week, 4 days ago
On Tue, Jun 02, 2026 at 10:57:23PM +0200, Rafael J. Wysocki wrote:
> On Tue, Jun 2, 2026 at 8:50 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, May 21, 2026 at 03:59:50PM +0200, Rafael J. Wysocki wrote:

...

> > > +     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?

Yes.

> > > +                                    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?

The line above has "<space>*<space>(sic!)<tab><tab> ... managed device".
The <space> after the asterisk is a stray one.

...

> > > +             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?

From my p.o.v. it's just extra information that's not needed. But I'm not going
fight to death against it.

...

> 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.

Okay!

-- 
With Best Regards,
Andy Shevchenko