On 11/10/22 08:13, Cezary Rojewski wrote:
> To improve performance and overall system stability, suspend/resume
> operations for ASoC cards always return success status and defer the
> actual work.
>
> Because of that, if a substream fails to resume, userspace may still
> attempt to invoke commands on it as from their perspective the operation
> completed successfully. Set substream's state to DISCONNECTED to ensure
> no further commands are attempted.
>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
> ---
> sound/soc/intel/avs/pcm.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/intel/avs/pcm.c b/sound/soc/intel/avs/pcm.c
> index ca624fbb5c0d..f95c530ffeb1 100644
> --- a/sound/soc/intel/avs/pcm.c
> +++ b/sound/soc/intel/avs/pcm.c
> @@ -934,8 +934,11 @@ static int avs_component_pm_op(struct snd_soc_component *component, bool be,
> rtd = snd_pcm_substream_chip(data->substream);
> if (rtd->dai_link->no_pcm == be && !rtd->dai_link->ignore_suspend) {
> ret = op(dai, data);
> - if (ret < 0)
> + if (ret < 0) {
> + data->substream->runtime->status->state =
> + SNDRV_PCM_STATE_DISCONNECTED;
should runtime->state be used instead of runtime->status->state?
A quick grep shows the former seems more popular in drivers,
status->seems to be only used in pcm_native.c?
Another plumbing question is whether it's actually ok to change the
state of the runtime in a driver, are you not going to have locking
issues? Very few drivers change the internal state and I wonder how
legit/safe this is.
> return ret;
> + }
> }
> }
>
> @@ -944,8 +947,11 @@ static int avs_component_pm_op(struct snd_soc_component *component, bool be,
> rtd = snd_pcm_substream_chip(data->substream);
> if (rtd->dai_link->no_pcm == be && !rtd->dai_link->ignore_suspend) {
> ret = op(dai, data);
> - if (ret < 0)
> + if (ret < 0) {
> + data->substream->runtime->status->state =
> + SNDRV_PCM_STATE_DISCONNECTED;
> return ret;
> + }
> }
> }
> }
