* [PATCH v7 1/4] platform/surface: gpe: use platform_device_register_full()
2026-07-13 15:11 [PATCH v7 0/4] driver core: unify the release path for dynamically allocated platform devices Bartosz Golaszewski
@ 2026-07-13 15:11 ` Bartosz Golaszewski
2026-07-13 15:11 ` [PATCH v7 2/4] drm/xe/i2c: use device_create_managed_software_node() Bartosz Golaszewski
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2026-07-13 15:11 UTC (permalink / raw)
To: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
Cc: brgl, platform-driver-x86, linux-kernel, intel-xe, dri-devel,
driver-core, Bartosz Golaszewski
Creating a software node for a given set of properties and adding it to
a platform device can be achieved with a single call to
platform_device_register_full(). There's nothing in this driver that
suggests using the more fine-grained interfaces was intentional so
switch to using the high-level helper.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/platform/surface/surface_gpe.c | 39 ++++++++++------------------------
1 file changed, 11 insertions(+), 28 deletions(-)
diff --git a/drivers/platform/surface/surface_gpe.c b/drivers/platform/surface/surface_gpe.c
index 40896a8544b0a4da4261ea881b1eaed62d93b32b..4ca7ad76f3b9d2b6c455a5d5b37f23a85eb69c69 100644
--- a/drivers/platform/surface/surface_gpe.c
+++ b/drivers/platform/surface/surface_gpe.c
@@ -290,9 +290,9 @@ static struct platform_device *surface_gpe_device;
static int __init surface_gpe_init(void)
{
+ struct platform_device_info pdevinfo;
const struct dmi_system_id *match;
struct platform_device *pdev;
- struct fwnode_handle *fwnode;
int status;
match = dmi_first_match(dmi_lid_device_table);
@@ -305,44 +305,27 @@ static int __init surface_gpe_init(void)
if (status)
return status;
- fwnode = fwnode_create_software_node(match->driver_data, NULL);
- if (IS_ERR(fwnode)) {
- status = PTR_ERR(fwnode);
- goto err_node;
- }
-
- pdev = platform_device_alloc("surface_gpe", PLATFORM_DEVID_NONE);
- if (!pdev) {
- status = -ENOMEM;
- goto err_alloc;
+ pdevinfo = (struct platform_device_info){
+ .name = "surface_gpe",
+ .id = PLATFORM_DEVID_NONE,
+ .properties = match->driver_data,
+ };
+
+ pdev = platform_device_register_full(&pdevinfo);
+ if (IS_ERR(pdev)) {
+ platform_driver_unregister(&surface_gpe_driver);
+ return PTR_ERR(pdev);
}
- platform_device_set_fwnode(pdev, fwnode);
-
- status = platform_device_add(pdev);
- if (status)
- goto err_add;
-
surface_gpe_device = pdev;
return 0;
-
-err_add:
- platform_device_put(pdev);
-err_alloc:
- fwnode_remove_software_node(fwnode);
-err_node:
- platform_driver_unregister(&surface_gpe_driver);
- return status;
}
module_init(surface_gpe_init);
static void __exit surface_gpe_exit(void)
{
- struct fwnode_handle *fwnode = surface_gpe_device->dev.fwnode;
-
platform_device_unregister(surface_gpe_device);
platform_driver_unregister(&surface_gpe_driver);
- fwnode_remove_software_node(fwnode);
}
module_exit(surface_gpe_exit);
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v7 2/4] drm/xe/i2c: use device_create_managed_software_node()
2026-07-13 15:11 [PATCH v7 0/4] driver core: unify the release path for dynamically allocated platform devices Bartosz Golaszewski
2026-07-13 15:11 ` [PATCH v7 1/4] platform/surface: gpe: use platform_device_register_full() Bartosz Golaszewski
@ 2026-07-13 15:11 ` Bartosz Golaszewski
2026-07-13 15:37 ` Rodrigo Vivi
2026-07-13 15:11 ` [PATCH v7 3/4] driver core: platform: unify release path Bartosz Golaszewski
2026-07-13 15:12 ` [PATCH v7 4/4] driver core: platform: tests: add test cases for correct swnode removal Bartosz Golaszewski
3 siblings, 1 reply; 8+ messages in thread
From: Bartosz Golaszewski @ 2026-07-13 15:11 UTC (permalink / raw)
To: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
Cc: brgl, platform-driver-x86, linux-kernel, intel-xe, dri-devel,
driver-core, Bartosz Golaszewski
This driver intentionally uses the fine-grained approach to creating
platform devices. It assigns a software node as the primary firmware
node of the device it creates. Ahead of improving the reference counting
of platform device software nodes, switch to using
device_create_managed_software_node(). This way, we create a dynamic
software node whose life-time is tied to the device to which it's
assigned.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/gpu/drm/xe/xe_i2c.c | 22 ++++++++--------------
drivers/gpu/drm/xe/xe_i2c.h | 1 -
2 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
index 3d80eafb9a1318996d10db11e14af4cfde07ba55..af6e973e5e68125a9c006bb646447ca6e377a6e6 100644
--- a/drivers/gpu/drm/xe/xe_i2c.c
+++ b/drivers/gpu/drm/xe/xe_i2c.c
@@ -93,13 +93,8 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
{
struct pci_dev *pci = to_pci_dev(i2c->drm_dev);
struct platform_device *pdev;
- struct fwnode_handle *fwnode;
int ret;
- fwnode = fwnode_create_software_node(xe_i2c_adapter_properties, NULL);
- if (IS_ERR(fwnode))
- return PTR_ERR(fwnode);
-
/*
* Not using platform_device_register_full() here because we don't have
* a handle to the platform_device before it returns. xe_i2c_notifier()
@@ -107,10 +102,14 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
* platform_device_register_full() is done.
*/
pdev = platform_device_alloc(adapter_name, pci_dev_id(pci));
- if (!pdev) {
- ret = -ENOMEM;
- goto err_fwnode_remove;
- }
+ if (!pdev)
+ return -ENOMEM;
+
+ ret = device_create_managed_software_node(&pdev->dev,
+ xe_i2c_adapter_properties,
+ NULL);
+ if (ret)
+ goto err_pdev_put;
if (i2c->adapter_irq) {
struct resource res;
@@ -123,8 +122,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
}
pdev->dev.parent = i2c->drm_dev;
- platform_device_set_fwnode(pdev, fwnode);
- i2c->adapter_node = fwnode;
i2c->pdev = pdev;
ret = platform_device_add(pdev);
@@ -135,8 +132,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
err_pdev_put:
platform_device_put(pdev);
-err_fwnode_remove:
- fwnode_remove_software_node(fwnode);
return ret;
}
@@ -144,7 +139,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
static void xe_i2c_unregister_adapter(struct xe_i2c *i2c)
{
platform_device_unregister(i2c->pdev);
- fwnode_remove_software_node(i2c->adapter_node);
}
/**
diff --git a/drivers/gpu/drm/xe/xe_i2c.h b/drivers/gpu/drm/xe/xe_i2c.h
index 425d8160835f4648891ff75f2a9c06284241710e..b28229f056c5a664e1d41dc0cc1978782fe1c2f1 100644
--- a/drivers/gpu/drm/xe/xe_i2c.h
+++ b/drivers/gpu/drm/xe/xe_i2c.h
@@ -30,7 +30,6 @@ struct xe_i2c_endpoint {
};
struct xe_i2c {
- struct fwnode_handle *adapter_node;
struct platform_device *pdev;
struct i2c_adapter *adapter;
struct i2c_client *client[XE_I2C_MAX_CLIENTS];
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v7 2/4] drm/xe/i2c: use device_create_managed_software_node()
2026-07-13 15:11 ` [PATCH v7 2/4] drm/xe/i2c: use device_create_managed_software_node() Bartosz Golaszewski
@ 2026-07-13 15:37 ` Rodrigo Vivi
2026-07-14 9:40 ` Heikki Krogerus
0 siblings, 1 reply; 8+ messages in thread
From: Rodrigo Vivi @ 2026-07-13 15:37 UTC (permalink / raw)
To: Bartosz Golaszewski, Heikki Krogerus
Cc: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, brgl,
platform-driver-x86, linux-kernel, intel-xe, dri-devel,
driver-core
On Mon, Jul 13, 2026 at 05:11:58PM +0200, Bartosz Golaszewski wrote:
> This driver intentionally uses the fine-grained approach to creating
> platform devices. It assigns a software node as the primary firmware
> node of the device it creates. Ahead of improving the reference counting
> of platform device software nodes, switch to using
> device_create_managed_software_node(). This way, we create a dynamic
> software node whose life-time is tied to the device to which it's
> assigned.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/gpu/drm/xe/xe_i2c.c | 22 ++++++++--------------
> drivers/gpu/drm/xe/xe_i2c.h | 1 -
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 2 files changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
> index 3d80eafb9a1318996d10db11e14af4cfde07ba55..af6e973e5e68125a9c006bb646447ca6e377a6e6 100644
> --- a/drivers/gpu/drm/xe/xe_i2c.c
> +++ b/drivers/gpu/drm/xe/xe_i2c.c
> @@ -93,13 +93,8 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
> {
> struct pci_dev *pci = to_pci_dev(i2c->drm_dev);
> struct platform_device *pdev;
> - struct fwnode_handle *fwnode;
> int ret;
>
> - fwnode = fwnode_create_software_node(xe_i2c_adapter_properties, NULL);
> - if (IS_ERR(fwnode))
> - return PTR_ERR(fwnode);
> -
> /*
> * Not using platform_device_register_full() here because we don't have
> * a handle to the platform_device before it returns. xe_i2c_notifier()
> @@ -107,10 +102,14 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
> * platform_device_register_full() is done.
> */
> pdev = platform_device_alloc(adapter_name, pci_dev_id(pci));
> - if (!pdev) {
> - ret = -ENOMEM;
> - goto err_fwnode_remove;
> - }
> + if (!pdev)
> + return -ENOMEM;
> +
> + ret = device_create_managed_software_node(&pdev->dev,
> + xe_i2c_adapter_properties,
> + NULL);
> + if (ret)
> + goto err_pdev_put;
>
> if (i2c->adapter_irq) {
> struct resource res;
> @@ -123,8 +122,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
> }
>
> pdev->dev.parent = i2c->drm_dev;
> - platform_device_set_fwnode(pdev, fwnode);
> - i2c->adapter_node = fwnode;
> i2c->pdev = pdev;
>
> ret = platform_device_add(pdev);
> @@ -135,8 +132,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
>
> err_pdev_put:
> platform_device_put(pdev);
> -err_fwnode_remove:
> - fwnode_remove_software_node(fwnode);
>
> return ret;
> }
> @@ -144,7 +139,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
> static void xe_i2c_unregister_adapter(struct xe_i2c *i2c)
> {
> platform_device_unregister(i2c->pdev);
> - fwnode_remove_software_node(i2c->adapter_node);
> }
>
> /**
> diff --git a/drivers/gpu/drm/xe/xe_i2c.h b/drivers/gpu/drm/xe/xe_i2c.h
> index 425d8160835f4648891ff75f2a9c06284241710e..b28229f056c5a664e1d41dc0cc1978782fe1c2f1 100644
> --- a/drivers/gpu/drm/xe/xe_i2c.h
> +++ b/drivers/gpu/drm/xe/xe_i2c.h
> @@ -30,7 +30,6 @@ struct xe_i2c_endpoint {
> };
>
> struct xe_i2c {
> - struct fwnode_handle *adapter_node;
> struct platform_device *pdev;
> struct i2c_adapter *adapter;
> struct i2c_client *client[XE_I2C_MAX_CLIENTS];
>
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v7 2/4] drm/xe/i2c: use device_create_managed_software_node()
2026-07-13 15:37 ` Rodrigo Vivi
@ 2026-07-14 9:40 ` Heikki Krogerus
0 siblings, 0 replies; 8+ messages in thread
From: Heikki Krogerus @ 2026-07-14 9:40 UTC (permalink / raw)
To: Bartosz Golaszewski, Rodrigo Vivi
Cc: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich, brgl,
platform-driver-x86, linux-kernel, intel-xe, dri-devel,
driver-core
On Mon, Jul 13, 2026 at 11:37:03AM -0400, Rodrigo Vivi wrote:
> On Mon, Jul 13, 2026 at 05:11:58PM +0200, Bartosz Golaszewski wrote:
> > This driver intentionally uses the fine-grained approach to creating
> > platform devices. It assigns a software node as the primary firmware
> > node of the device it creates. Ahead of improving the reference counting
> > of platform device software nodes, switch to using
> > device_create_managed_software_node(). This way, we create a dynamic
> > software node whose life-time is tied to the device to which it's
> > assigned.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
>
> Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
LGTM:
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > ---
> > drivers/gpu/drm/xe/xe_i2c.c | 22 ++++++++--------------
> > drivers/gpu/drm/xe/xe_i2c.h | 1 -
>
> Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> > 2 files changed, 8 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_i2c.c b/drivers/gpu/drm/xe/xe_i2c.c
> > index 3d80eafb9a1318996d10db11e14af4cfde07ba55..af6e973e5e68125a9c006bb646447ca6e377a6e6 100644
> > --- a/drivers/gpu/drm/xe/xe_i2c.c
> > +++ b/drivers/gpu/drm/xe/xe_i2c.c
> > @@ -93,13 +93,8 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
> > {
> > struct pci_dev *pci = to_pci_dev(i2c->drm_dev);
> > struct platform_device *pdev;
> > - struct fwnode_handle *fwnode;
> > int ret;
> >
> > - fwnode = fwnode_create_software_node(xe_i2c_adapter_properties, NULL);
> > - if (IS_ERR(fwnode))
> > - return PTR_ERR(fwnode);
> > -
> > /*
> > * Not using platform_device_register_full() here because we don't have
> > * a handle to the platform_device before it returns. xe_i2c_notifier()
> > @@ -107,10 +102,14 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
> > * platform_device_register_full() is done.
> > */
> > pdev = platform_device_alloc(adapter_name, pci_dev_id(pci));
> > - if (!pdev) {
> > - ret = -ENOMEM;
> > - goto err_fwnode_remove;
> > - }
> > + if (!pdev)
> > + return -ENOMEM;
> > +
> > + ret = device_create_managed_software_node(&pdev->dev,
> > + xe_i2c_adapter_properties,
> > + NULL);
> > + if (ret)
> > + goto err_pdev_put;
> >
> > if (i2c->adapter_irq) {
> > struct resource res;
> > @@ -123,8 +122,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
> > }
> >
> > pdev->dev.parent = i2c->drm_dev;
> > - platform_device_set_fwnode(pdev, fwnode);
> > - i2c->adapter_node = fwnode;
> > i2c->pdev = pdev;
> >
> > ret = platform_device_add(pdev);
> > @@ -135,8 +132,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
> >
> > err_pdev_put:
> > platform_device_put(pdev);
> > -err_fwnode_remove:
> > - fwnode_remove_software_node(fwnode);
> >
> > return ret;
> > }
> > @@ -144,7 +139,6 @@ static int xe_i2c_register_adapter(struct xe_i2c *i2c)
> > static void xe_i2c_unregister_adapter(struct xe_i2c *i2c)
> > {
> > platform_device_unregister(i2c->pdev);
> > - fwnode_remove_software_node(i2c->adapter_node);
> > }
> >
> > /**
> > diff --git a/drivers/gpu/drm/xe/xe_i2c.h b/drivers/gpu/drm/xe/xe_i2c.h
> > index 425d8160835f4648891ff75f2a9c06284241710e..b28229f056c5a664e1d41dc0cc1978782fe1c2f1 100644
> > --- a/drivers/gpu/drm/xe/xe_i2c.h
> > +++ b/drivers/gpu/drm/xe/xe_i2c.h
> > @@ -30,7 +30,6 @@ struct xe_i2c_endpoint {
> > };
> >
> > struct xe_i2c {
> > - struct fwnode_handle *adapter_node;
> > struct platform_device *pdev;
> > struct i2c_adapter *adapter;
> > struct i2c_client *client[XE_I2C_MAX_CLIENTS];
> >
> > --
> > 2.47.3
> >
--
heikki
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v7 3/4] driver core: platform: unify release path
2026-07-13 15:11 [PATCH v7 0/4] driver core: unify the release path for dynamically allocated platform devices Bartosz Golaszewski
2026-07-13 15:11 ` [PATCH v7 1/4] platform/surface: gpe: use platform_device_register_full() Bartosz Golaszewski
2026-07-13 15:11 ` [PATCH v7 2/4] drm/xe/i2c: use device_create_managed_software_node() Bartosz Golaszewski
@ 2026-07-13 15:11 ` Bartosz Golaszewski
2026-07-13 18:45 ` Danilo Krummrich
2026-07-13 15:12 ` [PATCH v7 4/4] driver core: platform: tests: add test cases for correct swnode removal Bartosz Golaszewski
3 siblings, 1 reply; 8+ messages in thread
From: Bartosz Golaszewski @ 2026-07-13 15:11 UTC (permalink / raw)
To: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
Cc: brgl, platform-driver-x86, linux-kernel, intel-xe, dri-devel,
driver-core, Bartosz Golaszewski
With no drivers that manually assign software nodes to platform devices
created with platform_device_alloc(), we can now unify the release path
and remove platform_device_release_full().
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/base/platform.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index a71015f1d915340aa01ebfae67196d4d7ccecba8..1df1a9ebd59ca6e0efdcd4038b100bebe224ee82 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);
fwnode_handle_put(pa->pdev.dev.fwnode);
kfree(pa->pdev.dev.platform_data);
kfree(pa->pdev.mfd_cell);
@@ -606,12 +607,6 @@ static void platform_device_release(struct device *dev)
kfree(pa);
}
-static void platform_device_release_full(struct device *dev)
-{
- device_remove_software_node(dev);
- platform_device_release(dev);
-}
-
/**
* platform_device_alloc - create a platform device
* @name: base name of the device we're adding
@@ -933,6 +928,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;
@@ -945,8 +950,6 @@ struct platform_device *platform_device_register_full(const struct platform_devi
ret = device_add_software_node(&pdev->dev, pdevinfo->swnode);
if (ret)
goto err;
-
- pdev->dev.release = platform_device_release_full;
} else if (pdevinfo->properties) {
ret = device_create_managed_software_node(&pdev->dev,
pdevinfo->properties, NULL);
--
2.47.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v7 3/4] driver core: platform: unify release path
2026-07-13 15:11 ` [PATCH v7 3/4] driver core: platform: unify release path Bartosz Golaszewski
@ 2026-07-13 18:45 ` Danilo Krummrich
0 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-07-13 18:45 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, brgl, platform-driver-x86,
linux-kernel, intel-xe, dri-devel, driver-core
On Mon Jul 13, 2026 at 5:11 PM CEST, Bartosz Golaszewski wrote:
> @@ -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);
> fwnode_handle_put(pa->pdev.dev.fwnode);
This technically changes the semantics of platform_device_set_fwnode():
Previously it took an additional reference count for the fwnode pointer passed
to it. But now, *iff* the fwnode is a software node it consumes the callers
reference count, which for obvious reasons isn't great.
Now, platform_device_set_fwnode() is unused now, so we could just get rid of it,
which also gets us rid of its asymmetric semantics.
However, couldn't we also just move the if (is_software_node(pdevinfo->fwnode))
conditional up here, such that we have:
device_remove_software_node(dev);
if (!is_software_node(dev->fwnode))
fwnode_handle_put(pa->pdev.dev.fwnode);
This way, the swnode special case would go away. Well, at least the "two
reference counts" special case.
Another special case I just noticed still remains, but is independent of this
change:
If platform_device_set_fwnode() or platform_device_set_of_node() is called for a
device that already has a swnode set, we only call fwnode_handle_put(), but
software_node_notify_remove() etc. isn't called.
Of course that never happens, but it is an inconsistency in the API. Now, it
seems that neither platform_device_set_fwnode() (which we should remove anyway),
nor platform_device_set_of_node() is ever called with a fwnode already set.
So, either we have to special case platform_device_set_of_node() too (for the
case that a swnode is set already), or just require that the function must only
be called when no node has been set, as all users already do; I'd go for the
latter.
If my proposal about moving the is_software_node() check into
platform_device_release() holds, it would probably be good to just add those
patches in a subsequent version, otherwise I'm happy to pull this in as is and
address the other stuff subsequently.
> kfree(pa->pdev.dev.platform_data);
> kfree(pa->pdev.mfd_cell);
> @@ -606,12 +607,6 @@ static void platform_device_release(struct device *dev)
> kfree(pa);
> }
>
> -static void platform_device_release_full(struct device *dev)
> -{
> - device_remove_software_node(dev);
> - platform_device_release(dev);
> -}
> -
> /**
> * platform_device_alloc - create a platform device
> * @name: base name of the device we're adding
> @@ -933,6 +928,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;
> @@ -945,8 +950,6 @@ struct platform_device *platform_device_register_full(const struct platform_devi
> ret = device_add_software_node(&pdev->dev, pdevinfo->swnode);
> if (ret)
> goto err;
> -
> - pdev->dev.release = platform_device_release_full;
> } else if (pdevinfo->properties) {
> ret = device_create_managed_software_node(&pdev->dev,
> pdevinfo->properties, NULL);
>
> --
> 2.47.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v7 4/4] driver core: platform: tests: add test cases for correct swnode removal
2026-07-13 15:11 [PATCH v7 0/4] driver core: unify the release path for dynamically allocated platform devices Bartosz Golaszewski
` (2 preceding siblings ...)
2026-07-13 15:11 ` [PATCH v7 3/4] driver core: platform: unify release path Bartosz Golaszewski
@ 2026-07-13 15:12 ` Bartosz Golaszewski
3 siblings, 0 replies; 8+ messages in thread
From: Bartosz Golaszewski @ 2026-07-13 15:12 UTC (permalink / raw)
To: Maximilian Luz, Hans de Goede, Ilpo Järvinen, Matthew Brost,
Thomas Hellström, Rodrigo Vivi, David Airlie, Simona Vetter,
Greg Kroah-Hartman, Rafael J. Wysocki, Danilo Krummrich
Cc: brgl, platform-driver-x86, linux-kernel, intel-xe, dri-devel,
driver-core, 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 | 180 +++++++++++++++++++++++++++++++
1 file changed, 180 insertions(+)
diff --git a/drivers/base/test/platform-device-test.c b/drivers/base/test/platform-device-test.c
index 6355a2231b741791b54eb78af42e13f31f745184..9ce563f76aad6943472dc78f3639ea1a2d72dbde 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,186 @@ 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 already 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;
+ bool bound = false;
+ int ret;
+
+ fwnode = kunit_kzalloc(test, sizeof(*fwnode), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, fwnode);
+
+ 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();
+ scoped_guard(device, &pdev->dev)
+ bound = device_is_bound(&pdev->dev);
+
+ KUNIT_ASSERT_TRUE(test, bound);
+
+ platform_device_unregister(pdev);
+
+ pdev = platform_device_register_full(&pdevinfo);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pdev);
+
+ wait_for_device_probe();
+ scoped_guard(device, &pdev->dev)
+ bound = device_is_bound(&pdev->dev);
+
+ KUNIT_ASSERT_TRUE(test, bound);
+
+ 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;
+ bool bound = false;
+ 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();
+ scoped_guard(device, &pdev->dev)
+ bound = device_is_bound(&pdev->dev);
+
+ KUNIT_ASSERT_TRUE(test, bound);
+
+ platform_device_unregister(pdev);
+
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, software_node_fwnode(&platform_device_test_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
^ permalink raw reply related [flat|nested] 8+ messages in thread