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

Commit Message

Kuninori Morimoto Aug. 7, 2019, 1:31 a.m. UTC

From: Kuninori Morimoto <[e-mail]>

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 <[e-mail]>
---
v1 -> v2

	- check return value
	- change Subject

 sound/soc/soc-core.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

Comments

Cezary Rojewski Aug. 20, 2019, 11:18 a.m. UTC | #1

On 2019-08-07 03:31, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <[e-mail]>
> 
> 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 <[e-mail]>
> ---
> 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?

Czarek

Pierre-Louis Bossart Aug. 20, 2019, 12:36 p.m. UTC | #2

On 8/20/19 6:18 AM, Cezary Rojewski wrote:
> On 2019-08-07 03:31, Kuninori Morimoto wrote:
>>
>> From: Kuninori Morimoto <[e-mail]>
>>
>> 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 <[e-mail]>
>> ---
>> 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.

Cezary Rojewski Aug. 20, 2019, 1:38 p.m. UTC | #3

On 2019-08-20 14:36, Pierre-Louis Bossart wrote:
> 
> 
> On 8/20/19 6:18 AM, Cezary Rojewski wrote:
>> On 2019-08-07 03:31, Kuninori Morimoto wrote:
>>>
>>> From: Kuninori Morimoto <[e-mail]>
>>>
>>> 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 <[e-mail]>
>>> ---
>>> 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.

If snd_soc_instantiate_card fails so does any test, really. Red wall was 
easy to spot even for a hungry developer : )

Our cnl_rt274 board declares several routes, yet our topology does not 
provide necessary info for all of them. And thus, addition of some 
routes fails. This was fine till now. That's also why I'd mentioned in 
the very first sentence: it might be simply a board issue. Maybe we 
should have never abused permissive behavior in the first place.

Pierre-Louis Bossart Aug. 20, 2019, 2:05 p.m. UTC | #4

On 8/20/19 8:38 AM, Cezary Rojewski wrote:
> On 2019-08-20 14:36, Pierre-Louis Bossart wrote:
>>
>>
>> On 8/20/19 6:18 AM, Cezary Rojewski wrote:
>>> On 2019-08-07 03:31, Kuninori Morimoto wrote:
>>>>
>>>> From: Kuninori Morimoto <[e-mail]>
>>>>
>>>> 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 <[e-mail]>
>>>> ---
>>>> 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.
> 
> If snd_soc_instantiate_card fails so does any test, really. Red wall was 
> easy to spot even for a hungry developer : )
> 
> Our cnl_rt274 board declares several routes, yet our topology does not 
> provide necessary info for all of them. And thus, addition of some 
> routes fails. This was fine till now. That's also why I'd mentioned in 
> the very first sentence: it might be simply a board issue. Maybe we 
> should have never abused permissive behavior in the first place.

Yep, and that driver is not upstream as well so Intel can't complain here...

Cezary Rojewski Aug. 20, 2019, 3:04 p.m. UTC | #5

On 2019-08-20 16:05, Pierre-Louis Bossart wrote:
> 
> 
> On 8/20/19 8:38 AM, Cezary Rojewski wrote:
>> On 2019-08-20 14:36, Pierre-Louis Bossart wrote:
>>>
>>>
>>> On 8/20/19 6:18 AM, Cezary Rojewski wrote:
>>>> On 2019-08-07 03:31, Kuninori Morimoto wrote:
>>>>>
>>>>> From: Kuninori Morimoto <[e-mail]>
>>>>>
>>>>> 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 <[e-mail]>
>>>>> ---
>>>>> 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.
>>
>> If snd_soc_instantiate_card fails so does any test, really. Red wall 
>> was easy to spot even for a hungry developer : )
>>
>> Our cnl_rt274 board declares several routes, yet our topology does not 
>> provide necessary info for all of them. And thus, addition of some 
>> routes fails. This was fine till now. That's also why I'd mentioned in 
>> the very first sentence: it might be simply a board issue. Maybe we 
>> should have never abused permissive behavior in the first place.
> 
> Yep, and that driver is not upstream as well so Intel can't complain 
> here...

??

It's not about complaining, rather starting a discussion. If I were 
using boards with topology not fully matching its board equivalent 
(because if has never been required of me) then there may be others who 
did the exact same trick. Your card won't be enumerated now == change of 
behavior.

Board bxt_rt298 is upstreamed and the exact same failures could be 
reproduced since topology has something to say here too..

Pierre-Louis Bossart Aug. 20, 2019, 5:39 p.m. UTC | #6

>>>>> 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.
>>>
>>> If snd_soc_instantiate_card fails so does any test, really. Red wall 
>>> was easy to spot even for a hungry developer : )
>>>
>>> Our cnl_rt274 board declares several routes, yet our topology does 
>>> not provide necessary info for all of them. And thus, addition of 
>>> some routes fails. This was fine till now. That's also why I'd 
>>> mentioned in the very first sentence: it might be simply a board 
>>> issue. Maybe we should have never abused permissive behavior in the 
>>> first place.
>>
>> Yep, and that driver is not upstream as well so Intel can't complain 
>> here...
> 
> ??
> 
> It's not about complaining, rather starting a discussion. If I were 
> using boards with topology not fully matching its board equivalent 
> (because if has never been required of me) then there may be others who 
> did the exact same trick. Your card won't be enumerated now == change of 
> behavior.
> 
> Board bxt_rt298 is upstreamed and the exact same failures could be 
> reproduced since topology has something to say here too..

That's different. For bxt_rt298 there is a topology that was released, 
and if it's incorrect it should be fixed.
I *hope* we are not going to see such regressions with SKL/KBL 
Chromebooks, that would be more of a problem since there are more users.

Mark Brown Aug. 20, 2019, 5:40 p.m. UTC | #7

On Tue, Aug 20, 2019 at 01:18:01PM +0200, Cezary Rojewski wrote:

> 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.

I really wouldn't expect routes failing to add to be a normal part of
instantiation (we should have been complaining about that in the logs
shouldn't we?).  Whatever enumeration is causing the missing widgets
to not instantiate also ought to control the addition of the routes
connecting them.

Patch

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);