On Mon, 13 Jul 2026 20:45:13 +0200, Danilo Krummrich <[email protected]> said: > 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. >
Ideally we should convert all users of platform_device_set_of_node() to platform_device_set_fwnode() and remove the former instead of the latter. I agree it's not optimal for it to be unused now but I can start converting the users of platform_device_set_of_node() once this one's in the driver core tree. > 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. > Sounds good. > 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. This sounds good too and it should be easy to implement another test case while at it. > > 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. > As I said: I prefer to keep platform_device_set_fwnode() and work on using it exclusively and drop platform_device_set_of_node(). If this is not urgent then it can wait until v7.3-rc1 where we'd have all this upstream, then I could send individual patches per subsystem tree and we'd drop platform_device_set_of_node() once there are no more users. Does it sound fine? Bart
