From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
soc_pcm_hw_params() does rollback when failed (A), but, it is almost same as soc_pcm_hw_free().
static int soc_pcm_hw_params(xxx) { ... if (ret < 0) goto xxx_err; ... return ret;
^ component_err: | ... | interface_err: (A) ... | codec_err: | ... v return ret; }
The difference is soc_pcm_hw_free() is for all dai/component/substream, rollback is for succeeded part only.
This kind of duplicated code can be a hotbed of bugs, thus, we want to share soc_pcm_hw_free() and rollback.
Now, soc_pcm_hw_params/free() are handling => 1) snd_soc_link_hw_params/free() 2) snd_soc_pcm_component_hw_params/free() 3) snd_soc_dai_hw_params/free()
This patch is for 1) snd_soc_link_hw_params/free().
The idea of having bit-flag or counter is not enough for this purpose. For example if one DAI is used for 2xPlaybacks for some reasons, and if 1st Playback was succeeded but 2nd Playback was failed, 2nd Playback rollback doesn't need to call shutdown. But it has succeeded bit-flag or counter via 1st Playback, thus, 2nd Playback rollback will call unneeded shutdown. And 1st Playback's necessary shutdown will not be called, because bit-flag or counter was cleared by wrong 2nd Playback rollback.
To avoid such case, this patch marks substream pointer when hw_params() was succeeded. If rollback needed, it will check rollback flag and marked substream pointer.
One note here ist that it cares *previous* hw_params() only now, but we might want to check *whole* marked substream in the future. This patch is using macro named "push/pop", so that it can be easily update.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- include/sound/soc-link.h | 3 ++- include/sound/soc.h | 1 + sound/soc/soc-link.c | 12 +++++++++++- sound/soc/soc-pcm.c | 4 ++-- 4 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/include/sound/soc-link.h b/include/sound/soc-link.h index dac6c0ce6ede..eff34fc7d3d3 100644 --- a/include/sound/soc-link.h +++ b/include/sound/soc-link.h @@ -19,7 +19,8 @@ void snd_soc_link_shutdown(struct snd_pcm_substream *substream, int snd_soc_link_prepare(struct snd_pcm_substream *substream); int snd_soc_link_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params); -void snd_soc_link_hw_free(struct snd_pcm_substream *substream); +void snd_soc_link_hw_free(struct snd_pcm_substream *substream, + int rollback); int snd_soc_link_trigger(struct snd_pcm_substream *substream, int cmd);
int snd_soc_link_compr_startup(struct snd_compr_stream *cstream); diff --git a/include/sound/soc.h b/include/sound/soc.h index 7541c71c9eb8..fa6ce936f899 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -1161,6 +1161,7 @@ struct snd_soc_pcm_runtime {
/* function mark */ struct snd_pcm_substream *mark_startup; + struct snd_pcm_substream *mark_hw_params;
/* bit field */ unsigned int pop_wait:1; diff --git a/sound/soc/soc-link.c b/sound/soc/soc-link.c index 2a8881978930..409ae4940da3 100644 --- a/sound/soc/soc-link.c +++ b/sound/soc/soc-link.c @@ -119,16 +119,26 @@ int snd_soc_link_hw_params(struct snd_pcm_substream *substream, rtd->dai_link->ops->hw_params) ret = rtd->dai_link->ops->hw_params(substream, params);
+ /* mark substream if succeeded */ + if (ret == 0) + soc_link_mark_push(rtd, substream, hw_params); + return soc_link_ret(rtd, ret); }
-void snd_soc_link_hw_free(struct snd_pcm_substream *substream) +void snd_soc_link_hw_free(struct snd_pcm_substream *substream, int rollback) { struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
+ if (rollback && !soc_link_mark_match(rtd, substream, hw_params)) + return; + if (rtd->dai_link->ops && rtd->dai_link->ops->hw_free) rtd->dai_link->ops->hw_free(substream); + + /* remove marked substream */ + soc_link_mark_pop(rtd, substream, hw_params); }
int snd_soc_link_trigger(struct snd_pcm_substream *substream, int cmd) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 65a6eebafefc..969f5774cd00 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -885,7 +885,7 @@ static int soc_pcm_hw_free(struct snd_pcm_substream *substream) }
/* free any machine hw params */ - snd_soc_link_hw_free(substream); + snd_soc_link_hw_free(substream, 0);
/* free any component resources */ snd_soc_pcm_component_hw_free(substream, NULL); @@ -1027,7 +1027,7 @@ static int soc_pcm_hw_params(struct snd_pcm_substream *substream, codec_dai->rate = 0; }
- snd_soc_link_hw_free(substream); + snd_soc_link_hw_free(substream, 1);
mutex_unlock(&rtd->card->pcm_mutex); return ret;