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

[Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>]

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@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>
> > ---
> > 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.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel