[alsa-devel] Is this bug ? dpcm_prune_paths()
Hi ALSA ML.
I'm checking soc-pcm.c, and I noticed strange code at dpcm_prune_paths().
static int dpcm_prune_paths(...) { ... /* Destroy any old FE <--> BE connections */ (1) for_each_dpcm_be(fe, stream, dpcm) { unsigned int i;
- /* is there a valid CPU DAI widget for this BE */ | widget = dai_get_widget(dpcm->be->cpu_dai, stream); | (A) /* prune the BE if it's no longer in our active list */ | if (widget && widget_in_list(list, widget)) |(a) continue; - | /* is there a valid CODEC DAI widget for this BE */ |(2) for_each_rtd_codec_dai(dpcm->be, i, dai) { | widget = dai_get_widget(dai, stream); (B) | /* prune the BE if it's no longer in our active list */ | if (widget && widget_in_list(list, widget)) |(b) continue; - }
- ... | dpcm->state = ... (C) dpcm->be->dpcm[stream].runtime_update = ... | prune++; - } ... }
In my understanding, (A) part is for CPU, and (B) part is for Codec. Guessing from (a), I think it want to skip setup (C) if CPU widget exist and matches to list. If so, I guess (b) is maybe bug ? This continue is for (2) loop, thus, it is totally do nothing now. But maybe it want to be continue for (1) loop ?
Thank you for your help !! Best regards --- Kuninori Morimoto
On 10/8/19 1:53 AM, Kuninori Morimoto wrote:
Hi ALSA ML.
I'm checking soc-pcm.c, and I noticed strange code at dpcm_prune_paths().
static int dpcm_prune_paths(...) { ... /* Destroy any old FE <--> BE connections */ (1) for_each_dpcm_be(fe, stream, dpcm) { unsigned int i;
/* is there a valid CPU DAI widget for this BE */
| widget = dai_get_widget(dpcm->be->cpu_dai, stream); | (A) /* prune the BE if it's no longer in our active list */ | if (widget && widget_in_list(list, widget)) |(a) continue;
| /* is there a valid CODEC DAI widget for this BE */ |(2) for_each_rtd_codec_dai(dpcm->be, i, dai) { | widget = dai_get_widget(dai, stream); (B) | /* prune the BE if it's no longer in our active list */ | if (widget && widget_in_list(list, widget)) |(b) continue;
}
...
| dpcm->state = ... (C) dpcm->be->dpcm[stream].runtime_update = ... | prune++;
...}
}
In my understanding, (A) part is for CPU, and (B) part is for Codec. Guessing from (a), I think it want to skip setup (C) if CPU widget exist and matches to list. If so, I guess (b) is maybe bug ? This continue is for (2) loop, thus, it is totally do nothing now. But maybe it want to be continue for (1) loop ?
Nice catch. This looks like a problem added during the transition to multi-codec.
The code before 2e5894d73789 ('ASoC: pcm: Add support for DAI multicodec') was this:
/* is there a valid CODEC DAI widget for this BE */ widget = dai_get_widget(dpcm->be->codec_dai, stream);
/* prune the BE if it's no longer in our active list */ if (widget && widget_in_list(list, widget)) continue;
and the addition of the loop for the codec seems incorrect
/* is there a valid CODEC DAI widget for this BE */ for (i = 0; i < dpcm->be->num_codecs; i++) { struct snd_soc_dai *dai = dpcm->be->codec_dais[i]; widget = dai_get_widget(dai, stream);
/* prune the BE if it's no longer in our active list */ if (widget && widget_in_list(list, widget)) continue; }
the continue was not meant to continue the for loop on num_codecs, but the outer loop for dpcm.
Hi Pierre-Louis
static int dpcm_prune_paths(...) { ... /* Destroy any old FE <--> BE connections */ (1) for_each_dpcm_be(fe, stream, dpcm) { unsigned int i;
/* is there a valid CPU DAI widget for this BE */
| widget = dai_get_widget(dpcm->be->cpu_dai, stream); | (A) /* prune the BE if it's no longer in our active list */ | if (widget && widget_in_list(list, widget)) |(a) continue;
| /* is there a valid CODEC DAI widget for this BE */ |(2) for_each_rtd_codec_dai(dpcm->be, i, dai) { | widget = dai_get_widget(dai, stream); (B) | /* prune the BE if it's no longer in our active list */ | if (widget && widget_in_list(list, widget)) |(b) continue;
}
...
| dpcm->state = ... (C) dpcm->be->dpcm[stream].runtime_update = ... | prune++;
...}
}
(snip)
Nice catch. This looks like a problem added during the transition to multi-codec.
(snip)
the continue was not meant to continue the for loop on num_codecs, but the outer loop for dpcm.
Thank for checking !! OK, it is bug. I will post patch for it.
Thank you for your help !! Best regards --- Kuninori Morimoto
participants (2)
-
Kuninori Morimoto
-
Pierre-Louis Bossart