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@xxxxxxxxxxxxxxx>
> >
> > 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
