On Thu, Apr 30, 2026 at 2:38 PM Bartosz Golaszewski <[email protected]> wrote: > > On Thu, Apr 30, 2026 at 2:02 PM Andy Shevchenko > <[email protected]> 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
