* [alsa-devel] Is this bug ? dpcm_prune_paths()
@ 2019-10-08 6:53 Kuninori Morimoto
2019-10-08 14:58 ` Pierre-Louis Bossart
0 siblings, 1 reply; 3+ messages in thread
From: Kuninori Morimoto @ 2019-10-08 6:53 UTC (permalink / raw)
To: Mark Brown; +Cc: Linux-ALSA
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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [alsa-devel] Is this bug ? dpcm_prune_paths()
2019-10-08 6:53 [alsa-devel] Is this bug ? dpcm_prune_paths() Kuninori Morimoto
@ 2019-10-08 14:58 ` Pierre-Louis Bossart
2019-10-09 0:58 ` Kuninori Morimoto
0 siblings, 1 reply; 3+ messages in thread
From: Pierre-Louis Bossart @ 2019-10-08 14:58 UTC (permalink / raw)
To: Kuninori Morimoto, Mark Brown; +Cc: Linux-ALSA, Benoit Cousson
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.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [alsa-devel] Is this bug ? dpcm_prune_paths()
2019-10-08 14:58 ` Pierre-Louis Bossart
@ 2019-10-09 0:58 ` Kuninori Morimoto
0 siblings, 0 replies; 3+ messages in thread
From: Kuninori Morimoto @ 2019-10-09 0:58 UTC (permalink / raw)
To: Pierre-Louis Bossart; +Cc: Linux-ALSA, Mark Brown, Benoit Cousson
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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-09 0:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-08 6:53 [alsa-devel] Is this bug ? dpcm_prune_paths() Kuninori Morimoto
2019-10-08 14:58 ` Pierre-Louis Bossart
2019-10-09 0:58 ` Kuninori Morimoto
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).