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@intel.com
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;} }