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

[Cezary Rojewski <cezary.rojewski@intel.com>]

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@intel.com>
>> ---
>>   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;