* [PATCH] ASoC: SOF: fix uninitialised "work" with VirtIO
@ 2020-03-20 12:36 Guennadi Liakhovetski
2020-03-20 16:31 ` Sridharan, Ranjani
0 siblings, 1 reply; 4+ messages in thread
From: Guennadi Liakhovetski @ 2020-03-20 12:36 UTC (permalink / raw)
To: alsa-devel; +Cc: Mark Brown, Sridharan, Ranjani, sound-open-firmware
In the VirtIO case the sof_pcm_open() function isn't called on the
host during guest streaming, which then leaves "work" structures
uninitialised. However it is then used to handle position update
messages from the DSP. Move their initialisation to immediately after
allocation of the containing structure.
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
This is a re-send of "[PATCH 08/14] ASoC: SOF: fix uninitialised "work"
with VirtIO" as suggested by Mark, also taking into account a comment
from Ranjani - thanks. Note: I haven't sent patches before from mutt,
hope this will work, if not - will have to re-send.
sound/soc/sof/pcm.c | 4 +---
sound/soc/sof/sof-audio.h | 3 +++
sound/soc/sof/topology.c | 17 ++++++++++++-----
3 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
index f4769e1..47cd741 100644
--- a/sound/soc/sof/pcm.c
+++ b/sound/soc/sof/pcm.c
@@ -57,7 +57,7 @@ static int sof_pcm_dsp_params(struct snd_sof_pcm *spcm, struct snd_pcm_substream
/*
* sof pcm period elapse work
*/
-static void sof_pcm_period_elapsed_work(struct work_struct *work)
+void snd_sof_pcm_period_elapsed_work(struct work_struct *work)
{
struct snd_sof_pcm_stream *sps =
container_of(work, struct snd_sof_pcm_stream,
@@ -475,8 +475,6 @@ static int sof_pcm_open(struct snd_soc_component *component,
dev_dbg(component->dev, "pcm: open stream %d dir %d\n",
spcm->pcm.pcm_id, substream->stream);
- INIT_WORK(&spcm->stream[substream->stream].period_elapsed_work,
- sof_pcm_period_elapsed_work);
caps = &spcm->pcm.caps[substream->stream];
diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
index eacd10e..bf65f31a 100644
--- a/sound/soc/sof/sof-audio.h
+++ b/sound/soc/sof/sof-audio.h
@@ -11,6 +11,8 @@
#ifndef __SOUND_SOC_SOF_AUDIO_H
#define __SOUND_SOC_SOF_AUDIO_H
+#include <linux/workqueue.h>
+
#include <sound/soc.h>
#include <sound/control.h>
#include <sound/sof/stream.h> /* needs to be included before control.h */
@@ -189,6 +191,7 @@ struct snd_sof_pcm *snd_sof_find_spcm_comp(struct snd_soc_component *scomp,
struct snd_sof_pcm *snd_sof_find_spcm_pcm_id(struct snd_soc_component *scomp,
unsigned int pcm_id);
void snd_sof_pcm_period_elapsed(struct snd_pcm_substream *substream);
+void snd_sof_pcm_period_elapsed_work(struct work_struct *work);
/*
* Mixer IPC
diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
index 058de94..fe8ba3e 100644
--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -9,6 +9,7 @@
//
#include <linux/firmware.h>
+#include <linux/workqueue.h>
#include <sound/tlv.h>
#include <sound/pcm_params.h>
#include <uapi/sound/sof/tokens.h>
@@ -2448,7 +2449,7 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index,
struct snd_soc_tplg_stream_caps *caps;
struct snd_soc_tplg_private *private = &pcm->priv;
struct snd_sof_pcm *spcm;
- int stream = SNDRV_PCM_STREAM_PLAYBACK;
+ int stream;
int ret = 0;
/* nothing to do for BEs atm */
@@ -2460,8 +2461,12 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index,
return -ENOMEM;
spcm->scomp = scomp;
- spcm->stream[SNDRV_PCM_STREAM_PLAYBACK].comp_id = COMP_ID_UNASSIGNED;
- spcm->stream[SNDRV_PCM_STREAM_CAPTURE].comp_id = COMP_ID_UNASSIGNED;
+
+ for_each_pcm_streams(stream) {
+ spcm->stream[stream].comp_id = COMP_ID_UNASSIGNED;
+ INIT_WORK(&spcm->stream[stream].period_elapsed_work,
+ snd_sof_pcm_period_elapsed_work);
+ }
spcm->pcm = *pcm;
dev_dbg(scomp->dev, "tplg: load pcm %s\n", pcm->dai_name);
@@ -2482,8 +2487,10 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index,
if (!spcm->pcm.playback)
goto capture;
+ stream = SNDRV_PCM_STREAM_PLAYBACK;
+
dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: playback d0i3:%d\n",
- spcm->pcm.pcm_name, spcm->stream[0].d0i3_compatible);
+ spcm->pcm.pcm_name, spcm->stream[stream].d0i3_compatible);
caps = &spcm->pcm.caps[stream];
@@ -2513,7 +2520,7 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index,
return ret;
dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: capture d0i3:%d\n",
- spcm->pcm.pcm_name, spcm->stream[1].d0i3_compatible);
+ spcm->pcm.pcm_name, spcm->stream[stream].d0i3_compatible);
caps = &spcm->pcm.caps[stream];
--
1.9.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] ASoC: SOF: fix uninitialised "work" with VirtIO
2020-03-20 12:36 [PATCH] ASoC: SOF: fix uninitialised "work" with VirtIO Guennadi Liakhovetski
@ 2020-03-20 16:31 ` Sridharan, Ranjani
2020-03-23 9:20 ` Guennadi Liakhovetski
0 siblings, 1 reply; 4+ messages in thread
From: Sridharan, Ranjani @ 2020-03-20 16:31 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: Linux-ALSA, Mark Brown, sound-open-firmware
On Fri, Mar 20, 2020 at 5:36 AM Guennadi Liakhovetski <
guennadi.liakhovetski@linux.intel.com> wrote:
> In the VirtIO case the sof_pcm_open() function isn't called on the
> host during guest streaming, which then leaves "work" structures
> uninitialised. However it is then used to handle position update
> messages from the DSP. Move their initialisation to immediately after
> allocation of the containing structure.
>
> Signed-off-by: Guennadi Liakhovetski <
> guennadi.liakhovetski@linux.intel.com>
> ---
>
> This is a re-send of "[PATCH 08/14] ASoC: SOF: fix uninitialised "work"
> with VirtIO" as suggested by Mark, also taking into account a comment
> from Ranjani - thanks. Note: I haven't sent patches before from mutt,
> hope this will work, if not - will have to re-send.
>
> sound/soc/sof/pcm.c | 4 +---
> sound/soc/sof/sof-audio.h | 3 +++
> sound/soc/sof/topology.c | 17 ++++++++++++-----
> 3 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
> index f4769e1..47cd741 100644
> --- a/sound/soc/sof/pcm.c
> +++ b/sound/soc/sof/pcm.c
> @@ -57,7 +57,7 @@ static int sof_pcm_dsp_params(struct snd_sof_pcm *spcm,
> struct snd_pcm_substream
> /*
> * sof pcm period elapse work
> */
> -static void sof_pcm_period_elapsed_work(struct work_struct *work)
> +void snd_sof_pcm_period_elapsed_work(struct work_struct *work)
> {
> struct snd_sof_pcm_stream *sps =
> container_of(work, struct snd_sof_pcm_stream,
> @@ -475,8 +475,6 @@ static int sof_pcm_open(struct snd_soc_component
> *component,
> dev_dbg(component->dev, "pcm: open stream %d dir %d\n",
> spcm->pcm.pcm_id, substream->stream);
>
> - INIT_WORK(&spcm->stream[substream->stream].period_elapsed_work,
> - sof_pcm_period_elapsed_work);
>
> caps = &spcm->pcm.caps[substream->stream];
>
> diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
> index eacd10e..bf65f31a 100644
> --- a/sound/soc/sof/sof-audio.h
> +++ b/sound/soc/sof/sof-audio.h
> @@ -11,6 +11,8 @@
> #ifndef __SOUND_SOC_SOF_AUDIO_H
> #define __SOUND_SOC_SOF_AUDIO_H
>
> +#include <linux/workqueue.h>
> +
> #include <sound/soc.h>
> #include <sound/control.h>
> #include <sound/sof/stream.h> /* needs to be included before control.h */
> @@ -189,6 +191,7 @@ struct snd_sof_pcm *snd_sof_find_spcm_comp(struct
> snd_soc_component *scomp,
> struct snd_sof_pcm *snd_sof_find_spcm_pcm_id(struct snd_soc_component
> *scomp,
> unsigned int pcm_id);
> void snd_sof_pcm_period_elapsed(struct snd_pcm_substream *substream);
> +void snd_sof_pcm_period_elapsed_work(struct work_struct *work);
>
> /*
> * Mixer IPC
> diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
> index 058de94..fe8ba3e 100644
> --- a/sound/soc/sof/topology.c
> +++ b/sound/soc/sof/topology.c
> @@ -9,6 +9,7 @@
> //
>
> #include <linux/firmware.h>
> +#include <linux/workqueue.h>
> #include <sound/tlv.h>
> #include <sound/pcm_params.h>
> #include <uapi/sound/sof/tokens.h>
> @@ -2448,7 +2449,7 @@ static int sof_dai_load(struct snd_soc_component
> *scomp, int index,
> struct snd_soc_tplg_stream_caps *caps;
> struct snd_soc_tplg_private *private = &pcm->priv;
> struct snd_sof_pcm *spcm;
> - int stream = SNDRV_PCM_STREAM_PLAYBACK;
> + int stream;
> int ret = 0;
>
> /* nothing to do for BEs atm */
> @@ -2460,8 +2461,12 @@ static int sof_dai_load(struct snd_soc_component
> *scomp, int index,
> return -ENOMEM;
>
> spcm->scomp = scomp;
> - spcm->stream[SNDRV_PCM_STREAM_PLAYBACK].comp_id =
> COMP_ID_UNASSIGNED;
> - spcm->stream[SNDRV_PCM_STREAM_CAPTURE].comp_id =
> COMP_ID_UNASSIGNED;
> +
> + for_each_pcm_streams(stream) {
> + spcm->stream[stream].comp_id = COMP_ID_UNASSIGNED;
> + INIT_WORK(&spcm->stream[stream].period_elapsed_work,
> + snd_sof_pcm_period_elapsed_work);
> + }
>
> spcm->pcm = *pcm;
> dev_dbg(scomp->dev, "tplg: load pcm %s\n", pcm->dai_name);
> @@ -2482,8 +2487,10 @@ static int sof_dai_load(struct snd_soc_component
> *scomp, int index,
> if (!spcm->pcm.playback)
> goto capture;
>
> + stream = SNDRV_PCM_STREAM_PLAYBACK;
> +
> dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: playback
> d0i3:%d\n",
> - spcm->pcm.pcm_name, spcm->stream[0].d0i3_compatible);
> + spcm->pcm.pcm_name, spcm->stream[stream].d0i3_compatible);
>
Hi Guennadi,
This cleanup is unrelated to the commit message (and the one below)? Should
it be a separate patch?
Thanks,
Ranjani
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ASoC: SOF: fix uninitialised "work" with VirtIO
2020-03-20 16:31 ` Sridharan, Ranjani
@ 2020-03-23 9:20 ` Guennadi Liakhovetski
2020-03-23 15:42 ` Sridharan, Ranjani
0 siblings, 1 reply; 4+ messages in thread
From: Guennadi Liakhovetski @ 2020-03-23 9:20 UTC (permalink / raw)
To: Sridharan, Ranjani; +Cc: Linux-ALSA, Mark Brown, sound-open-firmware
Hi Ranjani,
On Fri, Mar 20, 2020 at 09:31:50AM -0700, Sridharan, Ranjani wrote:
> On Fri, Mar 20, 2020 at 5:36 AM Guennadi Liakhovetski <
> guennadi.liakhovetski@linux.intel.com> wrote:
>
> > In the VirtIO case the sof_pcm_open() function isn't called on the
> > host during guest streaming, which then leaves "work" structures
> > uninitialised. However it is then used to handle position update
> > messages from the DSP. Move their initialisation to immediately after
> > allocation of the containing structure.
> >
> > Signed-off-by: Guennadi Liakhovetski <
> > guennadi.liakhovetski@linux.intel.com>
> > ---
> >
> > This is a re-send of "[PATCH 08/14] ASoC: SOF: fix uninitialised "work"
> > with VirtIO" as suggested by Mark, also taking into account a comment
> > from Ranjani - thanks. Note: I haven't sent patches before from mutt,
> > hope this will work, if not - will have to re-send.
> >
> > sound/soc/sof/pcm.c | 4 +---
> > sound/soc/sof/sof-audio.h | 3 +++
> > sound/soc/sof/topology.c | 17 ++++++++++++-----
> > 3 files changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
> > index f4769e1..47cd741 100644
> > --- a/sound/soc/sof/pcm.c
> > +++ b/sound/soc/sof/pcm.c
> > @@ -57,7 +57,7 @@ static int sof_pcm_dsp_params(struct snd_sof_pcm *spcm,
> > struct snd_pcm_substream
> > /*
> > * sof pcm period elapse work
> > */
> > -static void sof_pcm_period_elapsed_work(struct work_struct *work)
> > +void snd_sof_pcm_period_elapsed_work(struct work_struct *work)
> > {
> > struct snd_sof_pcm_stream *sps =
> > container_of(work, struct snd_sof_pcm_stream,
> > @@ -475,8 +475,6 @@ static int sof_pcm_open(struct snd_soc_component
> > *component,
> > dev_dbg(component->dev, "pcm: open stream %d dir %d\n",
> > spcm->pcm.pcm_id, substream->stream);
> >
> > - INIT_WORK(&spcm->stream[substream->stream].period_elapsed_work,
> > - sof_pcm_period_elapsed_work);
> >
> > caps = &spcm->pcm.caps[substream->stream];
> >
> > diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
> > index eacd10e..bf65f31a 100644
> > --- a/sound/soc/sof/sof-audio.h
> > +++ b/sound/soc/sof/sof-audio.h
> > @@ -11,6 +11,8 @@
> > #ifndef __SOUND_SOC_SOF_AUDIO_H
> > #define __SOUND_SOC_SOF_AUDIO_H
> >
> > +#include <linux/workqueue.h>
> > +
> > #include <sound/soc.h>
> > #include <sound/control.h>
> > #include <sound/sof/stream.h> /* needs to be included before control.h */
> > @@ -189,6 +191,7 @@ struct snd_sof_pcm *snd_sof_find_spcm_comp(struct
> > snd_soc_component *scomp,
> > struct snd_sof_pcm *snd_sof_find_spcm_pcm_id(struct snd_soc_component
> > *scomp,
> > unsigned int pcm_id);
> > void snd_sof_pcm_period_elapsed(struct snd_pcm_substream *substream);
> > +void snd_sof_pcm_period_elapsed_work(struct work_struct *work);
> >
> > /*
> > * Mixer IPC
> > diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
> > index 058de94..fe8ba3e 100644
> > --- a/sound/soc/sof/topology.c
> > +++ b/sound/soc/sof/topology.c
> > @@ -9,6 +9,7 @@
> > //
> >
> > #include <linux/firmware.h>
> > +#include <linux/workqueue.h>
> > #include <sound/tlv.h>
> > #include <sound/pcm_params.h>
> > #include <uapi/sound/sof/tokens.h>
> > @@ -2448,7 +2449,7 @@ static int sof_dai_load(struct snd_soc_component
> > *scomp, int index,
> > struct snd_soc_tplg_stream_caps *caps;
> > struct snd_soc_tplg_private *private = &pcm->priv;
> > struct snd_sof_pcm *spcm;
> > - int stream = SNDRV_PCM_STREAM_PLAYBACK;
> > + int stream;
> > int ret = 0;
> >
> > /* nothing to do for BEs atm */
> > @@ -2460,8 +2461,12 @@ static int sof_dai_load(struct snd_soc_component
> > *scomp, int index,
> > return -ENOMEM;
> >
> > spcm->scomp = scomp;
> > - spcm->stream[SNDRV_PCM_STREAM_PLAYBACK].comp_id =
> > COMP_ID_UNASSIGNED;
> > - spcm->stream[SNDRV_PCM_STREAM_CAPTURE].comp_id =
> > COMP_ID_UNASSIGNED;
> > +
> > + for_each_pcm_streams(stream) {
> > + spcm->stream[stream].comp_id = COMP_ID_UNASSIGNED;
> > + INIT_WORK(&spcm->stream[stream].period_elapsed_work,
> > + snd_sof_pcm_period_elapsed_work);
> > + }
> >
> > spcm->pcm = *pcm;
> > dev_dbg(scomp->dev, "tplg: load pcm %s\n", pcm->dai_name);
> > @@ -2482,8 +2487,10 @@ static int sof_dai_load(struct snd_soc_component
> > *scomp, int index,
> > if (!spcm->pcm.playback)
> > goto capture;
> >
> > + stream = SNDRV_PCM_STREAM_PLAYBACK;
> > +
> > dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: playback
> > d0i3:%d\n",
> > - spcm->pcm.pcm_name, spcm->stream[0].d0i3_compatible);
> > + spcm->pcm.pcm_name, spcm->stream[stream].d0i3_compatible);
> >
> Hi Guennadi,
>
> This cleanup is unrelated to the commit message (and the one below)? Should
> it be a separate patch?
I think it is a matter of judgement. It was your request to use
for_each_pcm_streams() which I did. While transitioning to it I noticed a
couple of related inconsistencies which I then aso fixed. So IMHO the
change is minor, obvious and related. But I agree that this now makes the
patch a bit controversial.
Thanks
Guennadi
> Thanks,
> Ranjani
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ASoC: SOF: fix uninitialised "work" with VirtIO
2020-03-23 9:20 ` Guennadi Liakhovetski
@ 2020-03-23 15:42 ` Sridharan, Ranjani
0 siblings, 0 replies; 4+ messages in thread
From: Sridharan, Ranjani @ 2020-03-23 15:42 UTC (permalink / raw)
To: Guennadi Liakhovetski; +Cc: Linux-ALSA, Mark Brown, sound-open-firmware
On Mon, Mar 23, 2020 at 2:20 AM Guennadi Liakhovetski <
guennadi.liakhovetski@linux.intel.com> wrote:
> Hi Ranjani,
>
> On Fri, Mar 20, 2020 at 09:31:50AM -0700, Sridharan, Ranjani wrote:
> > On Fri, Mar 20, 2020 at 5:36 AM Guennadi Liakhovetski <
> > guennadi.liakhovetski@linux.intel.com> wrote:
> >
> > > In the VirtIO case the sof_pcm_open() function isn't called on the
> > > host during guest streaming, which then leaves "work" structures
> > > uninitialised. However it is then used to handle position update
> > > messages from the DSP. Move their initialisation to immediately after
> > > allocation of the containing structure.
> > >
> > > Signed-off-by: Guennadi Liakhovetski <
> > > guennadi.liakhovetski@linux.intel.com>
> > > ---
> > >
> > > This is a re-send of "[PATCH 08/14] ASoC: SOF: fix uninitialised "work"
> > > with VirtIO" as suggested by Mark, also taking into account a comment
> > > from Ranjani - thanks. Note: I haven't sent patches before from mutt,
> > > hope this will work, if not - will have to re-send.
> > >
> > > sound/soc/sof/pcm.c | 4 +---
> > > sound/soc/sof/sof-audio.h | 3 +++
> > > sound/soc/sof/topology.c | 17 ++++++++++++-----
> > > 3 files changed, 16 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
> > > index f4769e1..47cd741 100644
> > > --- a/sound/soc/sof/pcm.c
> > > +++ b/sound/soc/sof/pcm.c
> > > @@ -57,7 +57,7 @@ static int sof_pcm_dsp_params(struct snd_sof_pcm
> *spcm,
> > > struct snd_pcm_substream
> > > /*
> > > * sof pcm period elapse work
> > > */
> > > -static void sof_pcm_period_elapsed_work(struct work_struct *work)
> > > +void snd_sof_pcm_period_elapsed_work(struct work_struct *work)
> > > {
> > > struct snd_sof_pcm_stream *sps =
> > > container_of(work, struct snd_sof_pcm_stream,
> > > @@ -475,8 +475,6 @@ static int sof_pcm_open(struct snd_soc_component
> > > *component,
> > > dev_dbg(component->dev, "pcm: open stream %d dir %d\n",
> > > spcm->pcm.pcm_id, substream->stream);
> > >
> > > - INIT_WORK(&spcm->stream[substream->stream].period_elapsed_work,
> > > - sof_pcm_period_elapsed_work);
> > >
> > > caps = &spcm->pcm.caps[substream->stream];
> > >
> > > diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
> > > index eacd10e..bf65f31a 100644
> > > --- a/sound/soc/sof/sof-audio.h
> > > +++ b/sound/soc/sof/sof-audio.h
> > > @@ -11,6 +11,8 @@
> > > #ifndef __SOUND_SOC_SOF_AUDIO_H
> > > #define __SOUND_SOC_SOF_AUDIO_H
> > >
> > > +#include <linux/workqueue.h>
> > > +
> > > #include <sound/soc.h>
> > > #include <sound/control.h>
> > > #include <sound/sof/stream.h> /* needs to be included before
> control.h */
> > > @@ -189,6 +191,7 @@ struct snd_sof_pcm *snd_sof_find_spcm_comp(struct
> > > snd_soc_component *scomp,
> > > struct snd_sof_pcm *snd_sof_find_spcm_pcm_id(struct snd_soc_component
> > > *scomp,
> > > unsigned int pcm_id);
> > > void snd_sof_pcm_period_elapsed(struct snd_pcm_substream *substream);
> > > +void snd_sof_pcm_period_elapsed_work(struct work_struct *work);
> > >
> > > /*
> > > * Mixer IPC
> > > diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
> > > index 058de94..fe8ba3e 100644
> > > --- a/sound/soc/sof/topology.c
> > > +++ b/sound/soc/sof/topology.c
> > > @@ -9,6 +9,7 @@
> > > //
> > >
> > > #include <linux/firmware.h>
> > > +#include <linux/workqueue.h>
> > > #include <sound/tlv.h>
> > > #include <sound/pcm_params.h>
> > > #include <uapi/sound/sof/tokens.h>
> > > @@ -2448,7 +2449,7 @@ static int sof_dai_load(struct snd_soc_component
> > > *scomp, int index,
> > > struct snd_soc_tplg_stream_caps *caps;
> > > struct snd_soc_tplg_private *private = &pcm->priv;
> > > struct snd_sof_pcm *spcm;
> > > - int stream = SNDRV_PCM_STREAM_PLAYBACK;
> > > + int stream;
> > > int ret = 0;
> > >
> > > /* nothing to do for BEs atm */
> > > @@ -2460,8 +2461,12 @@ static int sof_dai_load(struct snd_soc_component
> > > *scomp, int index,
> > > return -ENOMEM;
> > >
> > > spcm->scomp = scomp;
> > > - spcm->stream[SNDRV_PCM_STREAM_PLAYBACK].comp_id =
> > > COMP_ID_UNASSIGNED;
> > > - spcm->stream[SNDRV_PCM_STREAM_CAPTURE].comp_id =
> > > COMP_ID_UNASSIGNED;
> > > +
> > > + for_each_pcm_streams(stream) {
> > > + spcm->stream[stream].comp_id = COMP_ID_UNASSIGNED;
> > > + INIT_WORK(&spcm->stream[stream].period_elapsed_work,
> > > + snd_sof_pcm_period_elapsed_work);
> > > + }
> > >
> > > spcm->pcm = *pcm;
> > > dev_dbg(scomp->dev, "tplg: load pcm %s\n", pcm->dai_name);
> > > @@ -2482,8 +2487,10 @@ static int sof_dai_load(struct snd_soc_component
> > > *scomp, int index,
> > > if (!spcm->pcm.playback)
> > > goto capture;
> > >
> > > + stream = SNDRV_PCM_STREAM_PLAYBACK;
> > > +
> > > dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: playback
> > > d0i3:%d\n",
> > > - spcm->pcm.pcm_name, spcm->stream[0].d0i3_compatible);
> > > + spcm->pcm.pcm_name,
> spcm->stream[stream].d0i3_compatible);
> > >
> > Hi Guennadi,
> >
> > This cleanup is unrelated to the commit message (and the one below)?
> Should
> > it be a separate patch?
>
> I think it is a matter of judgement. It was your request to use
> for_each_pcm_streams() which I did. While transitioning to it I noticed a
> couple of related inconsistencies which I then aso fixed. So IMHO the
> change is minor, obvious and related. But I agree that this now makes the
> patch a bit controversial.
Hi Guennadi,
May I suggest to break it up into 2 patches: First, to move to using the
new for_each_pcm_stream_macro() and the second to add the add the work
init. That way there would be no confusion.
Thanks,
Ranjani
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-23 15:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-20 12:36 [PATCH] ASoC: SOF: fix uninitialised "work" with VirtIO Guennadi Liakhovetski
2020-03-20 16:31 ` Sridharan, Ranjani
2020-03-23 9:20 ` Guennadi Liakhovetski
2020-03-23 15:42 ` Sridharan, Ranjani
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.