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

[Hans de Goede]

Hi,

On 6/8/21 1:22 PM, Pierre-Louis Bossart wrote:
> 
>>>   static int snd_byt_cht_es8316_mc_remove(struct platform_device *pdev)
>>> @@ -617,6 +634,8 @@ static int snd_byt_cht_es8316_mc_remove(struct platform_device *pdev)
>>>       struct byt_cht_es8316_private *priv = snd_soc_card_get_drvdata(card);
>>>         gpiod_put(priv->speaker_en_gpio);
>>> +    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.
> 
> Is this possible really? the codec driver will register a component that's used by the ASoC card, I was assuming that there was some sort of reference count already to prevent this unbinding from happening.

There might very well be some reference count elsewhere, but IMHO if the
machine-driver is going to keep a pointer to the device around it should
keep its own reference.

>> 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).
>>
>> 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.
>>
>> 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")
>>
>> Or we could move the device_put(codec_dev) to snd_byt_cht_es8316_mc_remove().
> 
> that sounds like a better plan if you want to manage explicit dependencies - regardless of how which API is used to manage properties.

Ok, so lets go with that then.

Regards,

Hans