Re: [PATCH 2/2] ASoC: Intel: boards: use software node API in Atom boards

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

On Tue, Jun 08, 2021 at 10:18:08AM +0200, Hans de Goede wrote:
> On 6/8/21 12:35 AM, Pierre-Louis Bossart wrote:
> > From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > 
> > The function device_add_properties() is going to be removed.
> > Replacing it with software node API equivalents.

...

> > +	device_remove_software_node(priv->codec_dev);
> 
> This is a problem, nothing guarantees codec_dev not going away before
> snd_byt_cht_es8316_mc_remove() runs. Although the only thing which I can come up
> with where this happens is unbinding the i2c-controller driver I still would like
> us to take this scenario into account.
> 
> I think it would be better to use device_create_managed_software_node() to tie
> the lifetime of the swnode to the lifetime of the device, this also removes
> the need for all the goto err cases (and introducing a remove function in
> the bytcr_rt5640.c case).

Which device? If you are telling about codec, the unload-load of the machine
driver won't be successful or will produce a leak / warning / etc.

If you are telling about machine related device, it simply doesn't belong to it.

Am I got all this right?

> This does mean that we could end up calling device_create_managed_software_node()
> on the same device twice, when the machine driver gets unbound + rebound, but
> that is an already existing issue with our current device_add_properties() usage.

Yep.

> We could fix this (in a separate commit since it is an already existing issue)
> by adding a PROPERTY_ENTRY_BOOL("cht_es8316,swnode-created") property to the
> properties and checking for that being set with
> device_property_read_bool(codec, "cht_es8316,swnode-created")

Not sure it's a good idea, this sounds like a hack.

> Or we could move the device_put(codec_dev) to snd_byt_cht_es8316_mc_remove().

This sounds better.

> I've a slight preference for using device_create_managed_software_node() +
> some mechanism to avoid a double adding of the properties, since I would like
> to try and avoid the "goto err" additions.
> 
> Ideally device_create_managed_software_node() would detect the double-add
> itself and it would return -EEXISTS. Heikki, would that be feasible ?

If I got the big picture correct, the SOF needs to switch to use fwnode graphs.

-- 
With Best Regards,
Andy Shevchenko