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

[PATCH v4 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.

Merging strategy: patch 1/3 should go into v7.1. Once it's upstream, the
first tag containing it should be pulled into the driver core tree and
the remaining patches applied on top with an ack from the kunit
maintainers.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Changes in v4:
- Drop redundant !pdevinfo->swnode check in
  platform_device_register_full()
- Handle all three combinations with which two software nodes can be
  assigned to a platform device
- Extend the new kunit test to verify all three combinations
- Link to v3: https://patch.msgid.link/20260428-swnode-remove-on-dev-unreg-v3-0-533bdc71b22e@oss.qualcomm.com

Changes in v3:
- Make sure the reference of the primary software node we possibly take,
  is always released by moving the get() before calls that may fail
- Don't allow passing two software nodes
- Add a test case for that situation
- Link to v2: https://patch.msgid.link/20260423-swnode-remove-on-dev-unreg-v2-0-0e5213cde2ed@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                  |  19 +++-
 drivers/base/test/platform-device-test.c | 168 +++++++++++++++++++++++++++++++
 include/kunit/fwnode.h                   |  19 ++++
 lib/kunit/Makefile                       |   3 +-
 lib/kunit/fwnode.c                       |  52 ++++++++++
 5 files changed, 259 insertions(+), 2 deletions(-)
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260410-swnode-remove-on-dev-unreg-42bfc4b23ba8

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

[PATCH v4 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.

In addition to the above change, make sure that we still function
correctly if a software node is used as the primary firmware node as
well as disallow using two software nodes for platform devices as
device_add_software_node() does not handle this case correctly (in fact
a comment inside it states that only one software node per device is
allowed but it will not bail out if two are used so we need to handle it
here).

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

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 75b4698d0e582e67adafa78c312d75c72fd654cf..22834be0a3aeabf538f57ec6815ab3cd6851c16b 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);
@@ -848,7 +849,13 @@ struct platform_device *platform_device_register_full(const struct platform_devi
 	int ret;
 	struct platform_device *pdev;
 
-	if (pdevinfo->swnode && pdevinfo->properties)
+	/*
+	 * Only one software node per device is allowed. Make sure we don't
+	 * accept or create two.
+	 */
+	if ((pdevinfo->swnode && pdevinfo->properties) ||
+	    (pdevinfo->swnode && is_software_node(pdevinfo->fwnode)) ||
+	    (pdevinfo->properties && is_software_node(pdevinfo->fwnode)))
 		return ERR_PTR(-EINVAL);
 
 	pdev = platform_device_alloc(pdevinfo->name, pdevinfo->id);
@@ -866,6 +873,16 @@ struct platform_device *platform_device_register_full(const struct platform_devi
 		pdev->dev.coherent_dma_mask = pdevinfo->dma_mask;
 	}
 
+	/*
+	 * 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))
+		fwnode_handle_get(pdevinfo->fwnode);
+
 	ret = platform_device_add_resources(pdev, pdevinfo->res, pdevinfo->num_res);
 	if (ret)
 		goto err;

-- 
2.47.3

[PATCH v4 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 v4 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 | 168 +++++++++++++++++++++++++++++++
 1 file changed, 168 insertions(+)

diff --git a/drivers/base/test/platform-device-test.c b/drivers/base/test/platform-device-test.c
index 6355a2231b741791b54eb78af42e13f31f745184..3e42c205fc935ab1dd2066e257d4ecf837c9ad79 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,174 @@ 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 const struct software_node platform_device_test_primary_swnode = { };
+
+/*
+ * Check that passing two software nodes to platform_device_register_full()
+ * fails.
+ */
+static void platform_device_two_swnodes(struct kunit *test)
+{
+	static const struct property_entry properties[] = {
+		PROPERTY_ENTRY_U32("foo", 42),
+		{ }
+	};
+
+	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,
+		.swnode = &platform_device_test_swnode,
+	};
+
+	pdev = platform_device_register_full(&pdevinfo);
+	KUNIT_ASSERT_TRUE(test, IS_ERR(pdev));
+	KUNIT_ASSERT_EQ_MSG(test, PTR_ERR(pdev), -EINVAL,
+			    "Expected errno == -EINVAL, got: %pe", pdev);
+
+	pdevinfo = (struct platform_device_info){
+		.name = DEVICE_NAME,
+		.id = PLATFORM_DEVID_NONE,
+		.swnode = &platform_device_test_swnode,
+		.properties = properties,
+	};
+
+	pdev = platform_device_register_full(&pdevinfo);
+	KUNIT_ASSERT_TRUE(test, IS_ERR(pdev));
+	KUNIT_ASSERT_EQ_MSG(test, PTR_ERR(pdev), -EINVAL,
+			    "Expected errno == -EINVAL, got: %pe", pdev);
+
+	pdevinfo = (struct platform_device_info){
+		.name = DEVICE_NAME,
+		.id = PLATFORM_DEVID_NONE,
+		.fwnode = fwnode,
+		.properties = properties,
+	};
+
+	pdev = platform_device_register_full(&pdevinfo);
+	KUNIT_ASSERT_TRUE(test, IS_ERR(pdev));
+	KUNIT_ASSERT_EQ_MSG(test, PTR_ERR(pdev), -EINVAL,
+			    "Expected errno == -EINVAL, got: %pe", pdev);
+}
+
+static struct kunit_case platform_device_swnode_tests[] = {
+	KUNIT_CASE(platform_device_swnode_add_twice),
+	KUNIT_CASE(platform_device_swnode_as_primary),
+	KUNIT_CASE(platform_device_two_swnodes),
+	{}
+};
+
+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

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

[Andy Shevchenko]

On Thu, Apr 30, 2026 at 09:46:04AM +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.
> 
> In addition to the above change, make sure that we still function
> correctly if a software node is used as the primary firmware node as
> well as disallow using two software nodes for platform devices as
> device_add_software_node() does not handle this case correctly (in fact
> a comment inside it states that only one software node per device is
> allowed but it will not bail out if two are used so we need to handle it
> here).

...

> -	if (pdevinfo->swnode && pdevinfo->properties)
> +	/*
> +	 * Only one software node per device is allowed. Make sure we don't
> +	 * accept or create two.
> +	 */
> +	if ((pdevinfo->swnode && pdevinfo->properties) ||
> +	    (pdevinfo->swnode && is_software_node(pdevinfo->fwnode)) ||
> +	    (pdevinfo->properties && is_software_node(pdevinfo->fwnode)))
>  		return ERR_PTR(-EINVAL);

This makes me think of why we have these many ways of doing things...
Perhaps we should kill pdevinfo::properties completely?

Second thought is what about actually refusing this on the level of
device_add_software_node()? And looking at it, we have that check
there, why do we need it here then? Did we miss to check error code from
device_add_software_node() somewhere?

-- 
With Best Regards,
Andy Shevchenko

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

[Bartosz Golaszewski]

On Thu, Apr 30, 2026 at 2:02 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Thu, Apr 30, 2026 at 09:46:04AM +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.
> >
> > In addition to the above change, make sure that we still function
> > correctly if a software node is used as the primary firmware node as
> > well as disallow using two software nodes for platform devices as
> > device_add_software_node() does not handle this case correctly (in fact
> > a comment inside it states that only one software node per device is
> > allowed but it will not bail out if two are used so we need to handle it
> > here).
>
> ...
>
> > -     if (pdevinfo->swnode && pdevinfo->properties)
> > +     /*
> > +      * Only one software node per device is allowed. Make sure we don't
> > +      * accept or create two.
> > +      */
> > +     if ((pdevinfo->swnode && pdevinfo->properties) ||
> > +         (pdevinfo->swnode && is_software_node(pdevinfo->fwnode)) ||
> > +         (pdevinfo->properties && is_software_node(pdevinfo->fwnode)))
> >               return ERR_PTR(-EINVAL);
>
> This makes me think of why we have these many ways of doing things...
> Perhaps we should kill pdevinfo::properties completely?
>

Good luck with that, I'm pretty sure a lot of users are still out
there. :) It's also relatively convenient and skips one more struct
definition.

> Second thought is what about actually refusing this on the level of
> device_add_software_node()? And looking at it, we have that check
> there, why do we need it here then? Did we miss to check error code from
> device_add_software_node() somewhere?
>

device_create_managed_software_node() doesn't seem to have that check
unlike device_add_software_node() so we'd hit an issue with properties
!= NULL && swnode supplied. I think failing earlier here makes for
more readable code and less error-prone unwinding on failure.

Also: device_add_software_node() return -EBUSY while I think it makes
more sense to return -EINVAL. I prefer to keep this check locally
unless driver core maintainers object.

Bart

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

[Bartosz Golaszewski]

On Thu, Apr 30, 2026 at 2:38 PM Bartosz Golaszewski <brgl@kernel.org> wrote:
>
> On Thu, Apr 30, 2026 at 2:02 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > On Thu, Apr 30, 2026 at 09:46:04AM +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.
> > >
> > > In addition to the above change, make sure that we still function
> > > correctly if a software node is used as the primary firmware node as
> > > well as disallow using two software nodes for platform devices as
> > > device_add_software_node() does not handle this case correctly (in fact
> > > a comment inside it states that only one software node per device is
> > > allowed but it will not bail out if two are used so we need to handle it
> > > here).
> >
> > ...
> >
> > > -     if (pdevinfo->swnode && pdevinfo->properties)
> > > +     /*
> > > +      * Only one software node per device is allowed. Make sure we don't
> > > +      * accept or create two.
> > > +      */
> > > +     if ((pdevinfo->swnode && pdevinfo->properties) ||
> > > +         (pdevinfo->swnode && is_software_node(pdevinfo->fwnode)) ||
> > > +         (pdevinfo->properties && is_software_node(pdevinfo->fwnode)))
> > >               return ERR_PTR(-EINVAL);
> >
> > This makes me think of why we have these many ways of doing things...
> > Perhaps we should kill pdevinfo::properties completely?
> >
>
> Good luck with that, I'm pretty sure a lot of users are still out
> there. :) It's also relatively convenient and skips one more struct
> definition.
>
> > Second thought is what about actually refusing this on the level of
> > device_add_software_node()? And looking at it, we have that check
> > there, why do we need it here then? Did we miss to check error code from
> > device_add_software_node() somewhere?
> >
>
> device_create_managed_software_node() doesn't seem to have that check
> unlike device_add_software_node() so we'd hit an issue with properties
> != NULL && swnode supplied. I think failing earlier here makes for
> more readable code and less error-prone unwinding on failure.
>
> Also: device_add_software_node() return -EBUSY while I think it makes
> more sense to return -EINVAL. I prefer to keep this check locally
> unless driver core maintainers object.
>
> Bart

Danilo: if there are no further comments, can you pick it up for v7.1?

Thanks,
Bartosz

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

[Danilo Krummrich]

On Mon May 11, 2026 at 9:33 AM CEST, Bartosz Golaszewski wrote:
> Danilo: if there are no further comments, can you pick it up for v7.1?

It seems that sashiko has a valid concern in [1]; can you confirm?

[1] https://sashiko.dev/#/patchset/20260430-swnode-remove-on-dev-unreg-v4-0-01574da0aed3%40oss.qualcomm.com

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

[Bartosz Golaszewski]

On Mon, 11 May 2026 16:00:23 +0200, Danilo Krummrich <dakr@kernel.org> said:
> On Mon May 11, 2026 at 9:33 AM CEST, Bartosz Golaszewski wrote:
>> Danilo: if there are no further comments, can you pick it up for v7.1?
>
> It seems that sashiko has a valid concern in [1]; can you confirm?
>
> [1] https://sashiko.dev/#/patchset/20260430-swnode-remove-on-dev-unreg-v4-0-01574da0aed3%40oss.qualcomm.com
>

Yes, I explained it here[1]. Basically it's similar to how we need to call
platform_device_add_data() for devices created with platform_device_alloc().

We can consider adding platform_device_add_software_node() once there's
a potential user but for now I'd just leave it like this.

Bartosz

[1] https://lore.kernel.org/all/CAMRc=MdR+xKCP5e+RiF-MvHvD9-z-Vxno=0B7nmOm+1oS6TDMQ@mail.gmail.com/

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

[Danilo Krummrich]

On Mon May 11, 2026 at 4:17 PM CEST, Bartosz Golaszewski wrote:
> On Mon, 11 May 2026 16:00:23 +0200, Danilo Krummrich <dakr@kernel.org> said:
>> On Mon May 11, 2026 at 9:33 AM CEST, Bartosz Golaszewski wrote:
>>> Danilo: if there are no further comments, can you pick it up for v7.1?
>>
>> It seems that sashiko has a valid concern in [1]; can you confirm?
>>
>> [1] https://sashiko.dev/#/patchset/20260430-swnode-remove-on-dev-unreg-v4-0-01574da0aed3%40oss.qualcomm.com
>>
>
> Yes, I explained it here[1]. Basically it's similar to how we need to call
> platform_device_add_data() for devices created with platform_device_alloc().
>
> We can consider adding platform_device_add_software_node() once there's
> a potential user but for now I'd just leave it like this.

But there are users that already need this, no? There is Xe [1] and Surface GPE
[2], or am I missing something?

[1] https://elixir.bootlin.com/linux/v7.1-rc2/source/drivers/gpu/drm/xe/xe_i2c.c#L99
[2] https://elixir.bootlin.com/linux/v7.1-rc2/source/drivers/platform/surface/surface_gpe.c#L308

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

[Bartosz Golaszewski]

On Mon, May 11, 2026 at 4:31 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Mon May 11, 2026 at 4:17 PM CEST, Bartosz Golaszewski wrote:
> > On Mon, 11 May 2026 16:00:23 +0200, Danilo Krummrich <dakr@kernel.org> said:
> >> On Mon May 11, 2026 at 9:33 AM CEST, Bartosz Golaszewski wrote:
> >>> Danilo: if there are no further comments, can you pick it up for v7.1?
> >>
> >> It seems that sashiko has a valid concern in [1]; can you confirm?
> >>
> >> [1] https://sashiko.dev/#/patchset/20260430-swnode-remove-on-dev-unreg-v4-0-01574da0aed3%40oss.qualcomm.com
> >>
> >
> > Yes, I explained it here[1]. Basically it's similar to how we need to call
> > platform_device_add_data() for devices created with platform_device_alloc().
> >
> > We can consider adding platform_device_add_software_node() once there's
> > a potential user but for now I'd just leave it like this.
>
> But there are users that already need this, no? There is Xe [1] and Surface GPE
> [2], or am I missing something?
>
> [1] https://elixir.bootlin.com/linux/v7.1-rc2/source/drivers/gpu/drm/xe/xe_i2c.c#L99
> [2] https://elixir.bootlin.com/linux/v7.1-rc2/source/drivers/platform/surface/surface_gpe.c#L308

Right, I was not aware of these. That could indeed cause a regression.
I'd like to fix the problem in v7.1 but also keep it minimal so adding
platform_device_add_software_node() and updating drivers to using it
may be the next step but for now: how about adding
platform_device_release_full() which would call
device_remove_software_node() and then the existing
platform_device_release()? We'd replace the .release callback of
struct device in platform_device_register_full() but if the user just
uses platform_device_alloc(), they would keep the regular .release()
that doesn't remove the software node?

That would go into v7.1 and then I'd provide
platform_device_add_software_node(), use it in all drivers that need
it, and then we'd remove platform_device_release_full() and go back to
a single, unified release callback?

Does it make sense?

Bart

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

[Danilo Krummrich]

On Mon May 11, 2026 at 4:50 PM CEST, Bartosz Golaszewski wrote:
> On Mon, May 11, 2026 at 4:31 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Mon May 11, 2026 at 4:17 PM CEST, Bartosz Golaszewski wrote:
>> > On Mon, 11 May 2026 16:00:23 +0200, Danilo Krummrich <dakr@kernel.org> said:
>> >> On Mon May 11, 2026 at 9:33 AM CEST, Bartosz Golaszewski wrote:
>> >>> Danilo: if there are no further comments, can you pick it up for v7.1?
>> >>
>> >> It seems that sashiko has a valid concern in [1]; can you confirm?
>> >>
>> >> [1] https://sashiko.dev/#/patchset/20260430-swnode-remove-on-dev-unreg-v4-0-01574da0aed3%40oss.qualcomm.com
>> >>
>> >
>> > Yes, I explained it here[1]. Basically it's similar to how we need to call
>> > platform_device_add_data() for devices created with platform_device_alloc().
>> >
>> > We can consider adding platform_device_add_software_node() once there's
>> > a potential user but for now I'd just leave it like this.
>>
>> But there are users that already need this, no? There is Xe [1] and Surface GPE
>> [2], or am I missing something?
>>
>> [1] https://elixir.bootlin.com/linux/v7.1-rc2/source/drivers/gpu/drm/xe/xe_i2c.c#L99
>> [2] https://elixir.bootlin.com/linux/v7.1-rc2/source/drivers/platform/surface/surface_gpe.c#L308
>
> Right, I was not aware of these. That could indeed cause a regression.
> I'd like to fix the problem in v7.1 but also keep it minimal so adding
> platform_device_add_software_node() and updating drivers to using it
> may be the next step but for now: how about adding
> platform_device_release_full() which would call
> device_remove_software_node() and then the existing
> platform_device_release()? We'd replace the .release callback of
> struct device in platform_device_register_full() but if the user just
> uses platform_device_alloc(), they would keep the regular .release()
> that doesn't remove the software node?
>
> That would go into v7.1 and then I'd provide
> platform_device_add_software_node(), use it in all drivers that need
> it, and then we'd remove platform_device_release_full() and go back to
> a single, unified release callback?
>
> Does it make sense?

If it is just those two (and at least I did not find any other drivers) it might
be easier to just add platform_device_add_software_node(), use it within those
two drivers and still land it for v7.1.

I feel like this change turns out simpler and less error prone than the above
workaround to keep changes local to the driver core.

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

[Bartosz Golaszewski]

On Mon, May 11, 2026 at 5:04 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Mon May 11, 2026 at 4:50 PM CEST, Bartosz Golaszewski wrote:
> > On Mon, May 11, 2026 at 4:31 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >>
> >> On Mon May 11, 2026 at 4:17 PM CEST, Bartosz Golaszewski wrote:
> >> > On Mon, 11 May 2026 16:00:23 +0200, Danilo Krummrich <dakr@kernel.org> said:
> >> >> On Mon May 11, 2026 at 9:33 AM CEST, Bartosz Golaszewski wrote:
> >> >>> Danilo: if there are no further comments, can you pick it up for v7.1?
> >> >>
> >> >> It seems that sashiko has a valid concern in [1]; can you confirm?
> >> >>
> >> >> [1] https://sashiko.dev/#/patchset/20260430-swnode-remove-on-dev-unreg-v4-0-01574da0aed3%40oss.qualcomm.com
> >> >>
> >> >
> >> > Yes, I explained it here[1]. Basically it's similar to how we need to call
> >> > platform_device_add_data() for devices created with platform_device_alloc().
> >> >
> >> > We can consider adding platform_device_add_software_node() once there's
> >> > a potential user but for now I'd just leave it like this.
> >>
> >> But there are users that already need this, no? There is Xe [1] and Surface GPE
> >> [2], or am I missing something?
> >>
> >> [1] https://elixir.bootlin.com/linux/v7.1-rc2/source/drivers/gpu/drm/xe/xe_i2c.c#L99
> >> [2] https://elixir.bootlin.com/linux/v7.1-rc2/source/drivers/platform/surface/surface_gpe.c#L308
> >
> > Right, I was not aware of these. That could indeed cause a regression.
> > I'd like to fix the problem in v7.1 but also keep it minimal so adding
> > platform_device_add_software_node() and updating drivers to using it
> > may be the next step but for now: how about adding
> > platform_device_release_full() which would call
> > device_remove_software_node() and then the existing
> > platform_device_release()? We'd replace the .release callback of
> > struct device in platform_device_register_full() but if the user just
> > uses platform_device_alloc(), they would keep the regular .release()
> > that doesn't remove the software node?
> >
> > That would go into v7.1 and then I'd provide
> > platform_device_add_software_node(), use it in all drivers that need
> > it, and then we'd remove platform_device_release_full() and go back to
> > a single, unified release callback?
> >
> > Does it make sense?
>
> If it is just those two (and at least I did not find any other drivers) it might
> be easier to just add platform_device_add_software_node(), use it within those
> two drivers and still land it for v7.1.
>
> I feel like this change turns out simpler and less error prone than the above
> workaround to keep changes local to the driver core.

Sounds good, I'll do this then.

Bart