Re: [PATCH 2/5] ASoC: topology: Do not ignore route checks when parsing graphs

[Cezary Rojewski <cezary.rojewski@xxxxxxxxx>]

On 2024-03-04 8:32 PM, Pierre-Louis Bossart wrote:
> On 3/4/24 13:05, Cezary Rojewski wrote:
> > One of the framework responsibilities is to ensure that the enumerated
> > DPCMs are valid i.e.: a valid BE is connected to a valid FE DAI. While
> > the are checks in soc-core.c and soc-pcm.c that verify this, a component
> > driver may attempt to workaround this by loading an invalid graph
> > through the topology file.
> >
> > Be strict and fail topology loading when invalid graph is encountered.
>
> This is very invasive, it's perfectly possible that we have a number of
> 'broken' topologies where one path is 'invalid' but it doesn't impact
> functionality.
>
> This should be an opt-in behavior IMHO, not a blanket change.

To my best knowledge, soc-topology.c' first "customer" was the 
skylake-driver and the final details were cloudy at best back then.

Right now sound-drivers utilizing the topology feature do so in more 
refined fashion. Next, in ASoC we have three locations where 
snd_soc_dapm_add_routes() is called but error-checks are done only in 
2/3 of them. This is bogus.

If the intended way of using snd_soc_dapm_add_routes() is to ignore the 
return, it should be converted to void and flag ->disable_route_checks 
removed.


Kind regards,
Czarek


> > Signed-off-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
> > ---
> >   sound/soc/soc-topology.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
> > index d6d368837235..778f539d9ff5 100644
> > --- a/sound/soc/soc-topology.c
> > +++ b/sound/soc/soc-topology.c
> > @@ -1083,8 +1083,9 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg,
> >   			break;
> >   		}
> >   
> > -		/* add route, but keep going if some fail */
> > -		snd_soc_dapm_add_routes(dapm, route, 1);
> > +		ret = snd_soc_dapm_add_routes(dapm, route, 1);
> > +		if (ret && !dapm->card->disable_route_checks)
> > +			break;
> >   	}
> >   
> >   	return ret;