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.