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
