On Fri, Mar 20, 2020 at 5:36 AM Guennadi Liakhovetski <
guennadi.liakhovetski@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> ---
>
> 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
