ACPI / platform: Unregister stale platform devices

PATCH In review Andy Shevchenko 4 participants 9 messages Aug 30, 2019 – Oct 18, 2019 ACPI

lore

Share & tools

lore

Andy Shevchenko

author

2019-08-30 14:34 UTC #0 lore raw

PATCH 1/1 ACPI / platform: Unregister stale platform devices

When the commit 68bdb6773289

("ACPI: add support for ACPI reconfiguration notifiers")

introduced reconfiguration notifiers it misses the point that the ACPI table, which may be loaded and then unloaded via ConfigFS, can contain devices that are not enumerated by their parents.

In such case the stale platform device is dangling in the system while the rest of the devices from the same table are already gone.

Introduce acpi_platform_device_remove_notify() notifier that, in similar way to I²C or SPI buses, unregisters the platform devices on table removal event.

Depends-on: 00500147cbd3 ("drivers: Introduce device lookup variants by ACPI_COMPANION device")

Cc Mika Westerberg <[email protected]>

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/acpi/acpi_platform.c | 43 ++++++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c          |  1 +
 2 files changed, 44 insertions(+)

drivers/acpi/acpi_platform.c +43 -0

index 00ec4f2bf015..dfcd6210828e 100644
@@ -31,6 +31,44 @@ static const struct acpi_device_id forbidden_id_list[] = {
 	{"", 0},
 };
 
+static struct platform_device *acpi_platform_device_find_by_adev(struct acpi_device *adev)
+{
+	struct device *dev;
+
+	dev = bus_find_device_by_acpi_dev(&platform_bus_type, adev);
+	return dev ? to_platform_device(dev) : NULL;
+}
+
+static int acpi_platform_device_remove_notify(struct notifier_block *nb,
+					      unsigned long value, void *arg)
+{
+	struct acpi_device *adev = arg;
+	struct platform_device *pdev;
+
+	switch (value) {
+	case ACPI_RECONFIG_DEVICE_ADD:
+		/* Nothing to do here */
+		break;
+	case ACPI_RECONFIG_DEVICE_REMOVE:
+		if (!acpi_device_enumerated(adev))
+			break;
+
+		pdev = acpi_platform_device_find_by_adev(adev);
+		if (!pdev)
+			break;
+
+		platform_device_unregister(pdev);
+		put_device(&pdev->dev);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block acpi_platform_notifier = {
+	.notifier_call = acpi_platform_device_remove_notify,
+};
+
 static void acpi_platform_fill_resource(struct acpi_device *adev,
 	const struct resource *src, struct resource *dest)
 {

@@ -130,3 +168,8 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
 	return pdev;
 }
 EXPORT_SYMBOL_GPL(acpi_create_platform_device);
+
+void __init acpi_platform_init(void)
+{
+	acpi_reconfig_notifier_register(&acpi_platform_notifier);
+}

drivers/acpi/scan.c +1 -0

index aad6be5c0af0..915650bf519f 100644
@@ -2174,6 +2174,7 @@ int __init acpi_scan_init(void)
 	acpi_pci_root_init();
 	acpi_pci_link_init();
 	acpi_processor_init();
+	acpi_platform_init();
 	acpi_lpss_init();
 	acpi_apd_init();
 	acpi_cmos_rtc_init();

Signature · 1 line

2.23.0.rc1

kbuild test robot

build bot

2019-09-02 06:38 UTC #1 lore raw

I love your patch! Yet something to improve:

Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc6 next-20190830]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Andy-Shevchenko/ACPI-platform-Unregister-stale-platform-devices/20190902-001307
config: x86_64-lkp (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0 reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64

If you fix the issue, kindly add following tag

Reported-by kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers//acpi/acpi_platform.c: In function 'acpi_platform_device_find_by_adev':

>> drivers//acpi/acpi_platform.c:38:8: error: implicit declaration of function 'bus_find_device_by_acpi_dev'; did you mean 'bus_find_device_by_name'? [-Werror=implicit-function-declaration]
     dev = bus_find_device_by_acpi_dev(&platform_bus_type, adev);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
           bus_find_device_by_name
   drivers//acpi/acpi_platform.c:38:6: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     dev = bus_find_device_by_acpi_dev(&platform_bus_type, adev);
         ^

cc1: some warnings being treated as errors

vim +38 drivers//acpi/acpi_platform.c

    33	
    34	static struct platform_device *acpi_platform_device_find_by_adev(struct acpi_device *adev)
    35	{
    36		struct device *dev;
    37	
  > 38		dev = bus_find_device_by_acpi_dev(&platform_bus_type, adev);
    39		return dev ? to_platform_device(dev) : NULL;
    40	}
    41

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Andy Shevchenko

author

2019-09-02 09:52 UTC #2 lore raw

Quoted 15 lines · kbuild test robot

Hi Andy,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc6 next-20190830]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Andy-Shevchenko/ACPI-platform-Unregister-stale-platform-devices/20190902-001307
config: x86_64-lkp (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0 reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Quoted 7 lines

If you fix the issue, kindly add following tag
Reported-by kbuild test robot <[email protected]>
All errors (new ones prefixed by >>):
drivers//acpi/acpi_platform.c: In function 'acpi_platform_device_find_by_adev':
Quoted 1 line
Quoted 1 line
drivers//acpi/acpi_platform.c:38:8: error: implicit declaration of function 'bus_find_device_by_acpi_dev'; did you mean 'bus_find_device_by_name'? [-Werror=implicit-function-declaration]

False positive, it has Depends-on tag for the dependency which is not yet in upstream.

Btw, have you noticed double slash in the paths in your scripts for LKP? (Look above)

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Rong Chen

2019-09-04 08:46 UTC #3 lore raw

Quoted 21 lines · Andy Shevchenko

Quoted 20 lines · kbuild test robot
Hi Andy,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc6 next-20190830]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Andy-Shevchenko/ACPI-platform-Unregister-stale-platform-devices/20190902-001307
config: x86_64-lkp (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0 reproduce:
         # save the attached .config to linux build tree
         make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by kbuild test robot <[email protected]>
All errors (new ones prefixed by >>):

Quoted 7 lines

Quoted 2 lines
drivers//acpi/acpi_platform.c: In function 'acpi_platform_device_find_by_adev':
Quoted 1 line
Quoted 1 line
drivers//acpi/acpi_platform.c:38:8: error: implicit declaration of function 'bus_find_device_by_acpi_dev'; did you mean 'bus_find_device_by_name'? [-Werror=implicit-function-declaration]
False positive, it has Depends-on tag for the dependency which is not yet in upstream.
Btw, have you noticed double slash in the paths in your scripts for LKP? (Look above)

Hi Andy,

Thanks for the new finding, The double slash not always appears . I think the double slash is not from our scripts.

Best Regards,
Rong Chen

kbuild test robot

build bot

2019-09-02 07:19 UTC #4 lore raw

In reply to Andy Shevchenko

I love your patch! Yet something to improve:

Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc6 next-20190830]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Andy-Shevchenko/ACPI-platform-Unregister-stale-platform-devices/20190902-001307
config: x86_64-rhel (attached as .config)
compiler: gcc-7 (Debian 7.4.0-11) 7.4.0 reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64

If you fix the issue, kindly add following tag

Reported-by kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/acpi/acpi_platform.c: In function 'acpi_platform_device_find_by_adev':

>> drivers/acpi/acpi_platform.c:38:8: error: implicit declaration of function 'bus_find_device_by_acpi_dev'; did you mean 'bus_find_device_by_name'? [-Werror=implicit-function-declaration]
     dev = bus_find_device_by_acpi_dev(&platform_bus_type, adev);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
           bus_find_device_by_name
   drivers/acpi/acpi_platform.c:38:6: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
     dev = bus_find_device_by_acpi_dev(&platform_bus_type, adev);
         ^

cc1: some warnings being treated as errors

vim +38 drivers/acpi/acpi_platform.c

    33	
    34	static struct platform_device *acpi_platform_device_find_by_adev(struct acpi_device *adev)
    35	{
    36		struct device *dev;
    37	
  > 38		dev = bus_find_device_by_acpi_dev(&platform_bus_type, adev);
    39		return dev ? to_platform_device(dev) : NULL;
    40	}
    41

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Andy Shevchenko

author

2019-09-02 09:55 UTC #5 lore raw

Quoted 2 lines · kbuild test robot

drivers/acpi/acpi_platform.c: In function 'acpi_platform_device_find_by_adev':
Quoted 1 line
Quoted 1 line
drivers/acpi/acpi_platform.c:38:8: error: implicit declaration of function 'bus_find_device_by_acpi_dev'; did you mean 'bus_find_device_by_name'? [-Werror=implicit-function-declaration]

Same false positive.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2019-10-17 11:38 UTC #6 lore raw

In reply to Andy Shevchenko

Quoted 8 lines · Andy Shevchenko

When the commit 68bdb6773289
("ACPI: add support for ACPI reconfiguration notifiers")
introduced reconfiguration notifiers it misses the point that the ACPI table, which may be loaded and then unloaded via ConfigFS, can contain devices that are not enumerated by their parents.

Quoted 6 lines

In such case the stale platform device is dangling in the system while the rest of the devices from the same table are already gone.
Introduce acpi_platform_device_remove_notify() notifier that, in similar way to I²C or SPI buses, unregisters the platform devices on table removal event.

Rafael, all dependencies now in v5.4-rc1.
Can this be applied, or I need to do more work?

Quoted 81 lines

Depends-on: 00500147cbd3 ("drivers: Introduce device lookup variants by ACPI_COMPANION device")

Cc Mika Westerberg <[email protected]>

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/acpi/acpi_platform.c | 43 ++++++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c          |  1 +
 2 files changed, 44 insertions(+)

drivers/acpi/acpi_platform.c +43 -0

index 00ec4f2bf015..dfcd6210828e 100644
@@ -31,6 +31,44 @@ static const struct acpi_device_id forbidden_id_list[] = {
 	{"", 0},
 };
 
+static struct platform_device *acpi_platform_device_find_by_adev(struct acpi_device *adev)
+{
+	struct device *dev;
+
+	dev = bus_find_device_by_acpi_dev(&platform_bus_type, adev);
+	return dev ? to_platform_device(dev) : NULL;
+}
+
+static int acpi_platform_device_remove_notify(struct notifier_block *nb,
+					      unsigned long value, void *arg)
+{
+	struct acpi_device *adev = arg;
+	struct platform_device *pdev;
+
+	switch (value) {
+	case ACPI_RECONFIG_DEVICE_ADD:
+		/* Nothing to do here */
+		break;
+	case ACPI_RECONFIG_DEVICE_REMOVE:
+		if (!acpi_device_enumerated(adev))
+			break;
+
+		pdev = acpi_platform_device_find_by_adev(adev);
+		if (!pdev)
+			break;
+
+		platform_device_unregister(pdev);
+		put_device(&pdev->dev);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block acpi_platform_notifier = {
+	.notifier_call = acpi_platform_device_remove_notify,
+};
+
 static void acpi_platform_fill_resource(struct acpi_device *adev,
 	const struct resource *src, struct resource *dest)
 {

@@ -130,3 +168,8 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
 	return pdev;
 }
 EXPORT_SYMBOL_GPL(acpi_create_platform_device);
+
+void __init acpi_platform_init(void)
+{
+	acpi_reconfig_notifier_register(&acpi_platform_notifier);
+}

drivers/acpi/scan.c +1 -0

index aad6be5c0af0..915650bf519f 100644
@@ -2174,6 +2174,7 @@ int __init acpi_scan_init(void)
 	acpi_pci_root_init();
 	acpi_pci_link_init();
 	acpi_processor_init();
+	acpi_platform_init();
 	acpi_lpss_init();
 	acpi_apd_init();
 	acpi_cmos_rtc_init();

Signature · 1 line

2.23.0.rc1

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Rafael J. Wysocki

maintainer

2019-10-17 21:57 UTC #7 lore raw

In reply to Andy Shevchenko

Quoted 86 lines · Andy Shevchenko

When the commit 68bdb6773289

("ACPI: add support for ACPI reconfiguration notifiers")

introduced reconfiguration notifiers it misses the point that the ACPI table, which may be loaded and then unloaded via ConfigFS, can contain devices that are not enumerated by their parents.

In such case the stale platform device is dangling in the system while the rest of the devices from the same table are already gone.

Introduce acpi_platform_device_remove_notify() notifier that, in similar way to I²C or SPI buses, unregisters the platform devices on table removal event.

Depends-on: 00500147cbd3 ("drivers: Introduce device lookup variants by ACPI_COMPANION device")

Cc Mika Westerberg <[email protected]>

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/acpi/acpi_platform.c | 43 ++++++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c          |  1 +
 2 files changed, 44 insertions(+)

drivers/acpi/acpi_platform.c +43 -0

index 00ec4f2bf015..dfcd6210828e 100644
@@ -31,6 +31,44 @@ static const struct acpi_device_id forbidden_id_list[] = {
        {"", 0},
 };
 
+static struct platform_device *acpi_platform_device_find_by_adev(struct acpi_device *adev)
+{
+       struct device *dev;
+
+       dev = bus_find_device_by_acpi_dev(&platform_bus_type, adev);
+       return dev ? to_platform_device(dev) : NULL;
+}
+
+static int acpi_platform_device_remove_notify(struct notifier_block *nb,
+                                             unsigned long value, void *arg)
+{
+       struct acpi_device *adev = arg;
+       struct platform_device *pdev;
+
+       switch (value) {
+       case ACPI_RECONFIG_DEVICE_ADD:
+               /* Nothing to do here */
+               break;
+       case ACPI_RECONFIG_DEVICE_REMOVE:
+               if (!acpi_device_enumerated(adev))
+                       break;
+
+               pdev = acpi_platform_device_find_by_adev(adev);
+               if (!pdev)
+                       break;
+
+               platform_device_unregister(pdev);
+               put_device(&pdev->dev);
+               break;
+       }
+
+       return NOTIFY_OK;
+}
+
+static struct notifier_block acpi_platform_notifier = {
+       .notifier_call = acpi_platform_device_remove_notify,
+};
+
 static void acpi_platform_fill_resource(struct acpi_device *adev,
        const struct resource *src, struct resource *dest)
 {

@@ -130,3 +168,8 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
        return pdev;
 }
 EXPORT_SYMBOL_GPL(acpi_create_platform_device);
+
+void __init acpi_platform_init(void)
+{
+       acpi_reconfig_notifier_register(&acpi_platform_notifier);
+}

drivers/acpi/scan.c +0 -0

index aad6be5c0af0..915650bf519f 100644

drivers/acpi/scan.c int __init acpi_scan_init(void)

Quoted 8 lines

drivers/acpi/scan.c +1 -1

@@ -2174,6 +2174,7 @@ int __init acpi_scan_init(void)
        acpi_pci_root_init();
        acpi_pci_link_init();
        acpi_processor_init();
+       acpi_platform_init();
        acpi_lpss_init();
        acpi_apd_init();
        acpi_cmos_rtc_init();
--

Applying (with minor modifications) as 5.5 material, thanks!

Andy Shevchenko

author

2019-10-18 08:35 UTC #8 lore raw

Quoted 1 line · Rafael J. Wysocki

Applying (with minor modifications) as 5.5 material, thanks!

Thank you!

Signature · 2 lines

With Best Regards,
Andy Shevchenko