[PATCH v2 0/3] driver core: remove software node from platform devices on device release

[PATCH v2 0/3] driver core: remove software node from platform devices on device release

[Bartosz Golaszewski]

This fixes an issue in platform device code where, if we specify a
software node for a platform device using struct platform_device_info,
it will not be removed on device .release().

The second patch adds a new kunit helper which is used in patch 3/3 that
adds a test-case that can be used to reproduce the problem and prove that
the fix works as well as another making sure a corner case of using a
software node as the primary firmware node works too.

First patch should go into v7.1 while patches 2/3 and 3/3 can be queued
for v7.2.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Changes in v2:
- Change the order between removing the software node and dropping the
  reference to the device's OF node
- Address a situation where a software node is used as the primary
  firmware node
- Add a patch adding a new kunit helper
- Add another test case
- Link to v1: https://patch.msgid.link/20260410-swnode-remove-on-dev-unreg-v1-0-cd7d305f3db2@oss.qualcomm.com

---
Bartosz Golaszewski (3):
      driver core: platform: remove software node on release()
      kunit: provide kunit_software_node_register()
      driver core: platform: tests: add test cases for correct swnode removal

 drivers/base/platform.c                  |  11 ++++
 drivers/base/test/platform-device-test.c | 106 +++++++++++++++++++++++++++++++
 include/kunit/fwnode.h                   |  19 ++++++
 lib/kunit/Makefile                       |   3 +-
 lib/kunit/fwnode.c                       |  52 +++++++++++++++
 5 files changed, 190 insertions(+), 1 deletion(-)
---
base-commit: 70c8a7ec6715b5fb14e501731b5b9210a16684f7
change-id: 20260410-swnode-remove-on-dev-unreg-42bfc4b23ba8

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

[PATCH v2 1/3] driver core: platform: remove software node on release()

[Bartosz Golaszewski]

If we pass a software node to a newly created device using struct
platform_device_info, it will not be removed when the device is
released. This may happen when a module creating the device is removed
or on failure in platform_device_add().

When we try to reuse that software node in a subsequent call to
platform_device_register_full(), it will fails with -EBUSY. Add the
missing call to device_remove_software_node() in release path.

Make sure that we still function correctly if a software node is used as
the primary firmware node.

Fixes: 0fc434bc2c45 ("driver core: platform: allow attaching software nodes when creating devices")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/base/platform.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 75b4698d0e582e67adafa78c312d75c72fd654cf..43ea7dcd338dd3ddae57e6d0677e5cb2673f6ed5 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -599,6 +599,7 @@ static void platform_device_release(struct device *dev)
 	struct platform_object *pa = container_of(dev, struct platform_object,
 						  pdev.dev);
 
+	device_remove_software_node(dev);
 	of_node_put(pa->pdev.dev.of_node);
 	kfree(pa->pdev.dev.platform_data);
 	kfree(pa->pdev.mfd_cell);
@@ -885,6 +886,16 @@ struct platform_device *platform_device_register_full(const struct platform_devi
 			goto err;
 	}
 
+	/*
+	 * If the primary firmware node is a software node and there's no
+	 * secondary firmware node, the primary will be affected by the call
+	 * to device_remove_software_node() in platform_device_release() and
+	 * its reference count will be dropped by one. Take another reference
+	 * here to make it have no effect.
+	 */
+	if (is_software_node(pdevinfo->fwnode) && !pdevinfo->swnode)
+		fwnode_handle_get(pdevinfo->fwnode);
+
 	ret = platform_device_add(pdev);
 	if (ret) {
 err:

-- 
2.47.3

Re: [PATCH v2 1/3] driver core: platform: remove software node on release()

[Dmitry Torokhov]

On Thu, Apr 23, 2026 at 02:12:02PM +0200, Bartosz Golaszewski wrote:
> If we pass a software node to a newly created device using struct
> platform_device_info, it will not be removed when the device is
> released. This may happen when a module creating the device is removed
> or on failure in platform_device_add().
> 
> When we try to reuse that software node in a subsequent call to
> platform_device_register_full(), it will fails with -EBUSY. Add the
> missing call to device_remove_software_node() in release path.
> 
> Make sure that we still function correctly if a software node is used as
> the primary firmware node.
> 
> Fixes: 0fc434bc2c45 ("driver core: platform: allow attaching software nodes when creating devices")
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> ---
>  drivers/base/platform.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 75b4698d0e582e67adafa78c312d75c72fd654cf..43ea7dcd338dd3ddae57e6d0677e5cb2673f6ed5 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -599,6 +599,7 @@ static void platform_device_release(struct device *dev)
>  	struct platform_object *pa = container_of(dev, struct platform_object,
>  						  pdev.dev);
>  
> +	device_remove_software_node(dev);
>  	of_node_put(pa->pdev.dev.of_node);
>  	kfree(pa->pdev.dev.platform_data);
>  	kfree(pa->pdev.mfd_cell);
> @@ -885,6 +886,16 @@ struct platform_device *platform_device_register_full(const struct platform_devi
>  			goto err;
>  	}
>  
> +	/*
> +	 * If the primary firmware node is a software node and there's no
> +	 * secondary firmware node, the primary will be affected by the call
> +	 * to device_remove_software_node() in platform_device_release() and
> +	 * its reference count will be dropped by one. Take another reference
> +	 * here to make it have no effect.
> +	 */
> +	if (is_software_node(pdevinfo->fwnode) && !pdevinfo->swnode)
> +		fwnode_handle_get(pdevinfo->fwnode);

It is possible to pass already registered node in pdevinfo->swnode
(because device_add_software_node() can handle this just fine). In this
case we also need to take an extra reference (or figure out whether we
need to drop the reference when removing the device).

Thanks.

-- 
Dmitry

Re: [PATCH v2 1/3] driver core: platform: remove software node on release()

[Bartosz Golaszewski]

On Thu, Apr 23, 2026 at 7:32 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> >
> > +     /*
> > +      * If the primary firmware node is a software node and there's no
> > +      * secondary firmware node, the primary will be affected by the call
> > +      * to device_remove_software_node() in platform_device_release() and
> > +      * its reference count will be dropped by one. Take another reference
> > +      * here to make it have no effect.
> > +      */
> > +     if (is_software_node(pdevinfo->fwnode) && !pdevinfo->swnode)
> > +             fwnode_handle_get(pdevinfo->fwnode);
>
> It is possible to pass already registered node in pdevinfo->swnode
> (because device_add_software_node() can handle this just fine). In this
> case we also need to take an extra reference (or figure out whether we
> need to drop the reference when removing the device).
>

But device_add_software_node() checks if the software node is
registered and - if so - just bumps the reference so the effect is the
same as when it registers the swnode.

Bart

Re: [PATCH v2 1/3] driver core: platform: remove software node on release()

[Dmitry Torokhov]

On Fri, Apr 24, 2026 at 02:26:38PM +0200, Bartosz Golaszewski wrote:
> On Thu, Apr 23, 2026 at 7:32 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > >
> > > +     /*
> > > +      * If the primary firmware node is a software node and there's no
> > > +      * secondary firmware node, the primary will be affected by the call
> > > +      * to device_remove_software_node() in platform_device_release() and
> > > +      * its reference count will be dropped by one. Take another reference
> > > +      * here to make it have no effect.
> > > +      */
> > > +     if (is_software_node(pdevinfo->fwnode) && !pdevinfo->swnode)
> > > +             fwnode_handle_get(pdevinfo->fwnode);
> >
> > It is possible to pass already registered node in pdevinfo->swnode
> > (because device_add_software_node() can handle this just fine). In this
> > case we also need to take an extra reference (or figure out whether we
> > need to drop the reference when removing the device).
> >
> 
> But device_add_software_node() checks if the software node is
> registered and - if so - just bumps the reference so the effect is the
> same as when it registers the swnode.

Ah, yes, you are right.

Still please check sashiko review - I think some of the concerns about
error unwinding are correct.

Thanks.

-- 
Dmitry

Re: [PATCH v2 1/3] driver core: platform: remove software node on release()

[Bartosz Golaszewski]

On Sun, Apr 26, 2026 at 6:24 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Still please check sashiko review - I think some of the concerns about
> error unwinding are correct.
>

Sigh... I hate the walls of text it generates. Can we set it to
"caveman mode"? :)

The first part of the review - while valid - already has a precedence
with this function. If someone allocates the platform device with
platform_device_alloc() and assigns the address of a static structure
as its platform data, on release() we'll unconditionally call kfree()
on it. The user must not assign platform data directly but use the
dedicated platform_device_add_data() helper. We could consider
providing platform_device_add_software_node() but since there are no
users, let's cross that bridge when we get there?

For the second point: sashiko's right, we need to bump the refcount earlier.

For the third: I need to look into it deeper. I thought my second test
case from patch 3/3 checks that use-case but the review makes sense.

Bart

Re: [PATCH v2 1/3] driver core: platform: remove software node on release()

[Andy Shevchenko]

On Thu, Apr 23, 2026 at 02:12:02PM +0200, Bartosz Golaszewski wrote:
> If we pass a software node to a newly created device using struct
> platform_device_info, it will not be removed when the device is
> released. This may happen when a module creating the device is removed
> or on failure in platform_device_add().
> 
> When we try to reuse that software node in a subsequent call to
> platform_device_register_full(), it will fails with -EBUSY. Add the
> missing call to device_remove_software_node() in release path.
> 
> Make sure that we still function correctly if a software node is used as
> the primary firmware node.

...

> +	device_remove_software_node(dev);

>  	of_node_put(pa->pdev.dev.of_node);

Can we rather replace of_* get/put with the fwnode_* get/put and make a
conditional here?

	if (is_software_node(...))
		device_remove...
	else
		fwnode_handle_put().

(or something like this)

And IIRC the above pattern has been already seen somewhere else. But I can't
point to it, just some weak memories of seeing that already.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] driver core: platform: remove software node on release()

[Bartosz Golaszewski]

On Fri, Apr 24, 2026 at 11:01 AM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Thu, Apr 23, 2026 at 02:12:02PM +0200, Bartosz Golaszewski wrote:
> > If we pass a software node to a newly created device using struct
> > platform_device_info, it will not be removed when the device is
> > released. This may happen when a module creating the device is removed
> > or on failure in platform_device_add().
> >
> > When we try to reuse that software node in a subsequent call to
> > platform_device_register_full(), it will fails with -EBUSY. Add the
> > missing call to device_remove_software_node() in release path.
> >
> > Make sure that we still function correctly if a software node is used as
> > the primary firmware node.
>
> ...
>
> > +     device_remove_software_node(dev);
>
> >       of_node_put(pa->pdev.dev.of_node);
>
> Can we rather replace of_* get/put with the fwnode_* get/put and make a
> conditional here?
>

I thought about it but I'm not sure why we bump the refcount of OF
nodes but not of the firmware nodes supplied in struct
platform_device_info. Maybe there was a reason for it. That would
simplify things.

>         if (is_software_node(...))
>                 device_remove...
>         else
>                 fwnode_handle_put().
>
> (or something like this)
>
> And IIRC the above pattern has been already seen somewhere else. But I can't
> point to it, just some weak memories of seeing that already.
>

That needs to account for two software nodes, I think it's fine to
call the two in sequence unconditionally (like I do this this patch)
and just dump the refcount on registration as necessary.

Bart

Re: [PATCH v2 1/3] driver core: platform: remove software node on release()

[Andy Shevchenko]

On Fri, Apr 24, 2026 at 02:34:29PM +0200, Bartosz Golaszewski wrote:
> On Fri, Apr 24, 2026 at 11:01 AM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > On Thu, Apr 23, 2026 at 02:12:02PM +0200, Bartosz Golaszewski wrote:

...

> > > +     device_remove_software_node(dev);
> >
> > >       of_node_put(pa->pdev.dev.of_node);
> >
> > Can we rather replace of_* get/put with the fwnode_* get/put and make a
> > conditional here?
> 
> I thought about it but I'm not sure why we bump the refcount of OF
> nodes but not of the firmware nodes supplied in struct
> platform_device_info. Maybe there was a reason for it. That would
> simplify things.

I could with 99% assurance state that the reason behind this as simple as
ACPI doesn't need that. It's no-op there and no-one at that time thought
of software nodes. More, I have some (a few years old) patch locally that
does this conversion, but never had time to look into it carefully for
any missed corner cases (as part of that work, the preparation was done
for AMBA devices in the ca5a75df36dd ("amba: bus: balance firmware node
reference counting") which is more than a couple of years in upstream).

> >         if (is_software_node(...))
> >                 device_remove...
> >         else
> >                 fwnode_handle_put().
> >
> > (or something like this)
> >
> > And IIRC the above pattern has been already seen somewhere else. But I can't
> > point to it, just some weak memories of seeing that already.
> 
> That needs to account for two software nodes, I think it's fine to
> call the two in sequence unconditionally (like I do this this patch)
> and just dump the refcount on registration as necessary.

Sure.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 2/3] kunit: provide kunit_software_node_register()

[Bartosz Golaszewski]

Implement a helper for registering kunit test-managed software nodes.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 include/kunit/fwnode.h | 19 ++++++++++++++++++
 lib/kunit/Makefile     |  3 ++-
 lib/kunit/fwnode.c     | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/include/kunit/fwnode.h b/include/kunit/fwnode.h
new file mode 100644
index 0000000000000000000000000000000000000000..e1554ace64b8a5899aff9f4b4247e5157826a49b
--- /dev/null
+++ b/include/kunit/fwnode.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * KUnit resource management helpers firmware nodes.
+ *
+ * Copyright (C) Qualcomm Technologies, Inc. and/or its subsidiaries
+ */
+
+#ifndef _KUNIT_FWNODE_H
+#define _KUNIT_FWNODE_H
+
+struct kunit;
+struct fwnode_handle;
+struct software_node;
+
+struct fwnode_handle *
+kunit_software_node_register(struct kunit *test,
+			     const struct software_node *node);
+
+#endif /* _KUNIT_FWNODE_H */
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile
index 656f1fa35abcc635e67d5b4cb1bc586b48415ac5..7549a701791b5b7eaa8e0637b6818cdfd0b655a8 100644
--- a/lib/kunit/Makefile
+++ b/lib/kunit/Makefile
@@ -10,7 +10,8 @@ kunit-objs +=				test.o \
 					executor.o \
 					attributes.o \
 					device.o \
-					platform.o
+					platform.o \
+					fwnode.o
 
 ifeq ($(CONFIG_KUNIT_DEBUGFS),y)
 kunit-objs +=				debugfs.o
diff --git a/lib/kunit/fwnode.c b/lib/kunit/fwnode.c
new file mode 100644
index 0000000000000000000000000000000000000000..bc8bf06762dd71a741a3419c1ca04028d6ad3ec8
--- /dev/null
+++ b/lib/kunit/fwnode.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Qualcomm Technologies, Inc. and/or its subsidiaries
+ */
+
+#include <kunit/fwnode.h>
+#include <kunit/test.h>
+
+#include <linux/fwnode.h>
+#include <linux/property.h>
+
+static void kunit_software_node_unregister(void *data)
+{
+	const struct software_node *swnode = data;
+
+	software_node_unregister(swnode);
+}
+
+/**
+ * kunit_software_node_register() - Register a KUnit-managed software node
+ * @test: test context
+ * @swnode: Software node to register
+ *
+ * Register a test-managed software node and return its firmware node handle.
+ * The software node is unregistered after the test case completes.
+ *
+ * Return: Firmware node handle of the registered software node or IS_ERR()
+ * on failure.
+ */
+struct fwnode_handle *
+kunit_software_node_register(struct kunit *test,
+			     const struct software_node *swnode)
+{
+	struct fwnode_handle *fwnode;
+	int ret;
+
+	ret = software_node_register(swnode);
+	if (ret)
+		return ERR_PTR(ret);
+
+	fwnode = software_node_fwnode(swnode);
+	if (WARN_ON(!fwnode))
+		return ERR_PTR(-ENOENT);
+
+	ret = kunit_add_action_or_reset(test, kunit_software_node_unregister,
+					(void *)swnode);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return fwnode;
+}
+EXPORT_SYMBOL_GPL(kunit_software_node_register);

-- 
2.47.3

[PATCH v2 3/3] driver core: platform: tests: add test cases for correct swnode removal

[Bartosz Golaszewski]

Extend the kunit module for platform devices with test cases verifying
that the same software node can be added to platform devices repeatedly.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
 drivers/base/test/platform-device-test.c | 106 +++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)

diff --git a/drivers/base/test/platform-device-test.c b/drivers/base/test/platform-device-test.c
index 6355a2231b741791b54eb78af42e13f31f745184..4b23f0d7f88548c62c49801fcee0919915e6e153 100644
--- a/drivers/base/test/platform-device-test.c
+++ b/drivers/base/test/platform-device-test.c
@@ -1,12 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0
 
+#include <kunit/fwnode.h>
 #include <kunit/platform_device.h>
 #include <kunit/resource.h>
 
 #include <linux/device.h>
 #include <linux/device/bus.h>
+#include <linux/fwnode.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 
 #define DEVICE_NAME "test"
 
@@ -253,9 +256,112 @@ static struct kunit_suite platform_device_match_test_suite = {
 	.test_cases = platform_device_match_tests,
 };
 
+static int platform_device_swnode_test_probe(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct platform_driver platform_swnode_test_driver = {
+	.probe = platform_device_swnode_test_probe,
+	.driver = {
+		.name = DEVICE_NAME,
+	},
+};
+
+static const struct software_node platform_device_test_swnode = { };
+
+/*
+ * Check that reusing a software node works correctly. If the call to
+ * platform_device_register_full() fails after adding the secondary firmware
+ * node, the software node must be unregistered in the device's release()
+ * callback or the subsequent call to platform_device_register_full() will fail
+ * with -EBUSY due to the software node aleady having been registered.
+ */
+static void platform_device_swnode_add_twice(struct kunit *test)
+{
+	struct platform_device_info pdevinfo;
+	struct platform_device *pdev;
+	struct fwnode_handle fwnode;
+	int ret;
+
+	ret = kunit_platform_driver_register(test, &platform_swnode_test_driver);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	fwnode_init(&fwnode, NULL);
+	pdevinfo = (struct platform_device_info){
+		.name = DEVICE_NAME,
+		.id = PLATFORM_DEVID_NONE,
+		.fwnode = &fwnode,
+		.swnode = &platform_device_test_swnode,
+	};
+
+	pdev = platform_device_register_full(&pdevinfo);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+
+	wait_for_device_probe();
+	KUNIT_ASSERT_TRUE(test, device_is_bound(&pdev->dev));
+
+	platform_device_unregister(pdev);
+
+	pdev = platform_device_register_full(&pdevinfo);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+
+	wait_for_device_probe();
+	KUNIT_ASSERT_TRUE(test, device_is_bound(&pdev->dev));
+
+	platform_device_unregister(pdev);
+}
+
+/*
+ * Check that passing a software node as the primary firmware node of the
+ * platform device does not result in it being unregistered by the call to
+ * device_remove_software_node() in its release path.
+ */
+static void platform_device_swnode_as_primary(struct kunit *test)
+{
+	struct platform_device_info pdevinfo;
+	struct platform_device *pdev;
+	struct fwnode_handle *fwnode;
+	int ret;
+
+	ret = kunit_platform_driver_register(test, &platform_swnode_test_driver);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	fwnode = kunit_software_node_register(test, &platform_device_test_swnode);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fwnode);
+
+	pdevinfo = (struct platform_device_info){
+		.name = DEVICE_NAME,
+		.id = PLATFORM_DEVID_NONE,
+		.fwnode = fwnode,
+	};
+
+	pdev = platform_device_register_full(&pdevinfo);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+
+	wait_for_device_probe();
+	KUNIT_ASSERT_TRUE(test, device_is_bound(&pdev->dev));
+
+	platform_device_unregister(pdev);
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, software_node_fwnode(&platform_device_test_swnode));
+}
+
+static struct kunit_case platform_device_swnode_tests[] = {
+	KUNIT_CASE(platform_device_swnode_add_twice),
+	KUNIT_CASE(platform_device_swnode_as_primary),
+	{}
+};
+
+static struct kunit_suite platform_device_swnode_test_suite = {
+	.name = "platform-device-swnode",
+	.test_cases = platform_device_swnode_tests,
+};
+
 kunit_test_suites(
 	&platform_device_devm_test_suite,
 	&platform_device_match_test_suite,
+	&platform_device_swnode_test_suite,
 );
 
 MODULE_DESCRIPTION("Test module for platform devices");

-- 
2.47.3