Re: [PATCH v2 1/3] software node: Introduce device_add_software_node()

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Wed, Jan 13, 2021 at 12:40:03AM +0000, Daniel Scally wrote:
> On 11/01/2021 14:10, Heikki Krogerus wrote:

...

> > +/**
> > + * device_remove_software_node - Remove device's software node
> > + * @dev: The device with the software node.
> > + *
> > + * This function will unregister the software node of @dev.
> > + */
> > +void device_remove_software_node(struct device *dev)
> > +{
> > +	struct swnode *swnode;
> > +
> > +	swnode = dev_to_swnode(dev);
> > +	if (!swnode)
> > +		return;
> > +
> > +	kobject_put(&swnode->kobj);
> > +}
> > +EXPORT_SYMBOL_GPL(device_remove_software_node);
> 
> I wonder if this also ought to set dev_fwnode(dev)->secondary back to
> ERR_PTR(-ENODEV)?

Looking more into this code I think we need to call

	set_secondary_fwnode(dev, NULL);

among these lines.

The real problem is that set_primary_fwnode() and set_secondary_fwnode() have
no reference counting. If we have a chain ->primary->secondary->-ENODEV is
being used somewhere we can't tell from here.

So, in practice it means that we lack of some kind of primary node to increment
reference count of the secondary node when the latter is chained to the given
primary. But it makes things too complicated. Any other options for shared
primary-secondary chain? Standalone primary along with standalone (exclusive)
secondary doesn't need this AFAICS. Perhaps a flag to primary like shared /
exclusive that will prevent breaking the chain in set_secondary_fwnode()?

-- 
With Best Regards,
Andy Shevchenko