[alsa-devel] [PATCH v2 09/25] ASoC: soc-core: tidyup for snd_soc_dapm_add_routes()

[Pierre-Louis Bossart]

On 8/20/19 6:18 AM, Cezary Rojewski wrote:
> On 2019-08-07 03:31, Kuninori Morimoto wrote:
>>
>> From: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
>>
>> snd_soc_dapm_add_routes() registers routes by using
>> for(... i < num; ...). If routes was NULL, num should be zero.
>> Thus, we don't need to check about route pointer.
>> This patch also cares missing return value.
>>
>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
>> ---
>> v1 -> v2
>>
>>     - check return value
>>     - change Subject
>>
>>   sound/soc/soc-core.c | 23 +++++++++++++----------
>>   1 file changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
>> index 21cdd3c..ca1b04c 100644
>> --- a/sound/soc/soc-core.c
>> +++ b/sound/soc/soc-core.c
>> @@ -1310,10 +1310,11 @@ static int soc_probe_component(struct 
>> snd_soc_card *card,
>>       if (ret < 0)
>>           goto err_probe;
>> -    if (component->driver->dapm_routes)
>> -        snd_soc_dapm_add_routes(dapm,
>> -                    component->driver->dapm_routes,
>> -                    component->driver->num_dapm_routes);
>> +    ret = snd_soc_dapm_add_routes(dapm,
>> +                      component->driver->dapm_routes,
>> +                      component->driver->num_dapm_routes);
>> +    if (ret < 0)
>> +        goto err_probe;
>>       list_add(&dapm->list, &card->dapm_list);
>>       /* see for_each_card_components */
>> @@ -2060,13 +2061,15 @@ static int snd_soc_instantiate_card(struct 
>> snd_soc_card *card)
>>           snd_soc_add_card_controls(card, card->controls,
>>                         card->num_controls);
>> -    if (card->dapm_routes)
>> -        snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes,
>> -                    card->num_dapm_routes);
>> +    ret = snd_soc_dapm_add_routes(&card->dapm, card->dapm_routes,
>> +                      card->num_dapm_routes);
>> +    if (ret < 0)
>> +        goto probe_end;
>> -    if (card->of_dapm_routes)
>> -        snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes,
>> -                    card->num_of_dapm_routes);
>> +    ret = snd_soc_dapm_add_routes(&card->dapm, card->of_dapm_routes,
>> +                      card->num_of_dapm_routes);
>> +    if (ret < 0)
>> +        goto probe_end;
>>       /* try to set some sane longname if DMI is available */
>>       snd_soc_set_dmi_name(card, NULL);
>>
> 
> Hello there,
> 
> I've run a validation cycle on recent broonie/for-next and this commit 
> caused regression. However, it may be simply an error on board side 
> instead.
> 
> Previously, ret from snd_soc_dapm_add_routes has been ignored thus it 
> was permissive for addition of several routes to fail. As long as some 
> routes succeeded, card was working just fine. Now it's no longer the 
> case - behavior of the card initialization has changed: it is required 
> for ALL routes to succeed before card can be fully instantiated.
> 
> Must say collapsing snd_soc_instantiate_card is a wonderful way to test 
> your card's removal flow (soc__cleanup_card_resources and friends)..
> 
> Question is simple: are we staying with all-for-one/ one-for-all 
> approach or we reverting to permissive behavior?

Can you elaborate in which test case this patch creates a problem? Just 
curious why the route addition fails in the first place.