[PATCH v1 06/17] ACPI: HED: Switch over to devres-based resource management

Posted by Rafael J. Wysocki 3 weeks, 2 days ago

From: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>

Use the newly introduced devm_acpi_install_notify_handler() for
installing an ACPI notify handler and since that function checks the
ACPI companion of the owner device against NULL internally, remove the
the explicit ACPI companion check from acpi_hed_probe().

No intentional functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/hed.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/hed.c b/drivers/acpi/hed.c
index 4b5dc95922ea..48562f53d3ab 100644
--- a/drivers/acpi/hed.c
+++ b/drivers/acpi/hed.c
@@ -50,19 +50,14 @@ static void acpi_hed_notify(acpi_handle handle, u32 event, void *data)
 
 static int acpi_hed_probe(struct platform_device *pdev)
 {
-	struct acpi_device *device;
 	int err;
 
-	device = ACPI_COMPANION(&pdev->dev);
-	if (!device)
-		return -ENODEV;
-
 	/* Only one hardware error device */
 	if (hed_present)
 		return -EINVAL;
 
-	err = acpi_dev_install_notify_handler(device, ACPI_DEVICE_NOTIFY,
-					      acpi_hed_notify, device);
+	err = devm_acpi_install_notify_handler(&pdev->dev, ACPI_DEVICE_NOTIFY,
+					       acpi_hed_notify, NULL);
 	if (err)
 		return err;
 
@@ -72,11 +67,7 @@ static int acpi_hed_probe(struct platform_device *pdev)
 
 static void acpi_hed_remove(struct platform_device *pdev)
 {
-	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
-
 	hed_present = false;
-	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
-				       acpi_hed_notify);
 }
 
 static struct platform_driver acpi_hed_driver = {
-- 
2.51.0

Re: [PATCH v1 06/17] ACPI: HED: Switch over to devres-based resource management

Posted by Andy Shevchenko 1 week, 4 days ago

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

> Use the newly introduced devm_acpi_install_notify_handler() for
> installing an ACPI notify handler and since that function checks the
> ACPI companion of the owner device against NULL internally, remove the
> the explicit ACPI companion check from acpi_hed_probe().
> 
> No intentional functional impact.

...

>  static void acpi_hed_remove(struct platform_device *pdev)
>  {
> -	struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> -
>  	hed_present = false;
> -	acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> -				       acpi_hed_notify);
>  }

devm will be cleaned after this, right? So, here is a window that if one tries
to unbind device from the driver in one thread and do the opposite in another
they will see the flag false while resources are still allocated for the old
one. It seems that hed_present should be also devm:ed?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 06/17] ACPI: HED: Switch over to devres-based resource management

Posted by Rafael J. Wysocki 1 week, 3 days ago

On Wed, Jun 3, 2026 at 12:10 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, May 21, 2026 at 04:04:03PM +0200, Rafael J. Wysocki wrote:
>
> > Use the newly introduced devm_acpi_install_notify_handler() for
> > installing an ACPI notify handler and since that function checks the
> > ACPI companion of the owner device against NULL internally, remove the
> > the explicit ACPI companion check from acpi_hed_probe().
> >
> > No intentional functional impact.
>
> ...
>
> >  static void acpi_hed_remove(struct platform_device *pdev)
> >  {
> > -     struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> > -
> >       hed_present = false;
> > -     acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > -                                    acpi_hed_notify);
> >  }
>
> devm will be cleaned after this, right?

Yes.

> So, here is a window that if one tries
> to unbind device from the driver in one thread and do the opposite in another
> they will see the flag false while resources are still allocated for the old
> one. It seems that hed_present should be also devm:ed?

If this is the same device, both binding and unbinding (including
devm) take place under its device lock which will serialize all of
that.

If they are different devices with different ACPI companions, there's
no resource conflict.  If they are both valid HEDs (which would be a
stretch already) and they both trigger a notification exactly at the
"wrong" time (more stretch), worst-case stuff will be called twice in
a row via blocking_notifier_call_chain().

Re: [PATCH v1 06/17] ACPI: HED: Switch over to devres-based resource management

Posted by Andy Shevchenko 1 week, 3 days ago

On Wed, Jun 03, 2026 at 12:51:20PM +0200, Rafael J. Wysocki wrote:
> On Wed, Jun 3, 2026 at 12:10 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, May 21, 2026 at 04:04:03PM +0200, Rafael J. Wysocki wrote:

...

> > >  static void acpi_hed_remove(struct platform_device *pdev)
> > >  {
> > > -     struct acpi_device *device = ACPI_COMPANION(&pdev->dev);
> > > -
> > >       hed_present = false;
> > > -     acpi_dev_remove_notify_handler(device, ACPI_DEVICE_NOTIFY,
> > > -                                    acpi_hed_notify);
> > >  }
> >
> > devm will be cleaned after this, right?
> 
> Yes.
> 
> > So, here is a window that if one tries
> > to unbind device from the driver in one thread and do the opposite in another
> > they will see the flag false while resources are still allocated for the old
> > one. It seems that hed_present should be also devm:ed?
> 
> If this is the same device, both binding and unbinding (including
> devm) take place under its device lock which will serialize all of
> that.
> 
> If they are different devices with different ACPI companions, there's
> no resource conflict.  If they are both valid HEDs (which would be a
> stretch already) and they both trigger a notification exactly at the
> "wrong" time (more stretch), worst-case stuff will be called twice in
> a row via blocking_notifier_call_chain().

Thanks for elaborating. We are good then.

-- 
With Best Regards,
Andy Shevchenko