On 11/29/23 10:35, Krzysztof Kozlowski wrote:
On 28/11/2023 18:39, Pierre-Louis Bossart wrote:
+int qcom_snd_sdw_startup(struct snd_pcm_substream *substream) +{
- struct snd_soc_pcm_runtime *rtd = substream->private_data;
- struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
- struct sdw_stream_runtime *sruntime;
- struct snd_soc_dai *codec_dai;
- int ret, i;
- sruntime = sdw_alloc_stream(cpu_dai->name);
- if (!sruntime)
return -ENOMEM;- for_each_rtd_codec_dais(rtd, i, codec_dai) {
ret = snd_soc_dai_set_stream(codec_dai, sruntime,substream->stream);if (ret < 0 && ret != -ENOTSUPP) {I know this is existing code moved into a helper, but out of curiosity why is -ENOTSUPP ignored? Isn't this problematic?
This is for all DAI links, so if some don't have set_stream callback, we assume it is not needed. For example few codecs do not need it because they are not on Soundwire bus at all and they don't care about the stream.
Humm, it was my understanding that the substream is mapped 1:1 with a dailink, so not sure how SoundWire and non-SoundWire DAIs could be part of the same dailink?
I am not saying this test is silly, just wondering if there is any case where this error code is returned. Worst-case it's always false but harmless.
dev_err(rtd->dev, "Failed to set sdw stream on %s\n",codec_dai->name);goto err_set_stream;}- }
Also should the CPU DAIs also be used to set the stream information? it's not clear to me why only the CODEC DAIs are used.
I don't know, we never did. As you pointed out, I am just moving things around, so I don't really know the original intention.
Fair enough, I've been in your shoes :-) Not always easy to grade 3+ yr code as 'miss', 'bug', 'optimization' or 'feature'...