It's reasonable that DAI parameters should be cleared if current DAI becomes
inactive.
Only check DAI active status in soc_pcm_hw_free() is not enough since there's
the risk that DAI parameters never be cleared if there're more than one stream
[see A]. Only check DAI active status in soc_pcm_close() is also not enough
since it will cause the cleanup just happening in soc_pcm_close() [see B].
[A] For example, we have stream1 and stream2 for DAI about to stop. stream2
executes soc_pcm_hw_free() before stream1 executes soc_pcm_close(). At the
moment, stream2 should clear current DAI parameters because stream1 passed the
cleanup stage in soc_pcm_hw_free() and stream2 in fact is the only active
stream which has a chance to do clean up. Since DAI active status is not yet
updated by stream1 in soc_pcm_close(), stream2 doesn't know itself should clear
DAI parameters. In result both stream1 and stream2 don't clear the parameters.
[B] Suppose a case: aplay -Dhw:0 44100.wav 48000.wav
In this case, we call soc_pcm_open()->soc_pcm_hw_params()->soc_pcm_hw_free()
->soc_pcm_hw_params()->soc_pcm_hw_free()->soc_pcm_close() in order. The DAI
parameters would be remained in the system even if the playback of 44100.wav is
finished.
In conclusion, it's better to check DAI active status in both soc_pcm_hw_free()
and soc_pcm_close() which makes sure DAI parameters cleared if the DAI becomes
inactive.
changes in v2:
- Patch v1 link: https://lore.kernel.org/all/[email protected]/
Patch v1 tries to introduce a usage count called hw_params_count to fix issue
on DAI parameters cleanup. However it's not a good fix because not
considering hw_params() and hw_free() are not symmetrical and hw_params()
might be called multilpe times by user.
- Both check DAI active status in soc_pcm_hw_free() and soc_pcm_close() which
makes sure DAI parameters cleared if the DAI becomes inactive.
Chancel Liu (1):
ASoC: soc-pcm.c: Make sure DAI parameters cleared if the DAI becomes
inactive
sound/soc/soc-pcm.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
--
2.25.1
On Wed, Sep 20, 2023 at 11:36:20PM +0800, Chancel Liu wrote:
> It's reasonable that DAI parameters should be cleared if current DAI becomes
> inactive.
>
> Only check DAI active status in soc_pcm_hw_free() is not enough since there's
> the risk that DAI parameters never be cleared if there're more than one stream
> [see A]. Only check DAI active status in soc_pcm_close() is also not enough
> since it will cause the cleanup just happening in soc_pcm_close() [see B].
Please don't send cover letters for single patches, if there is anything
that needs saying put it in the changelog of the patch or after the ---
if it's administrative stuff. This reduces mail volume and ensures that
any important information is recorded in the changelog rather than being
lost.
On Wed, 20 Sep 2023 23:36:20 +0800, Chancel Liu wrote:
> It's reasonable that DAI parameters should be cleared if current DAI becomes
> inactive.
>
> Only check DAI active status in soc_pcm_hw_free() is not enough since there's
> the risk that DAI parameters never be cleared if there're more than one stream
> [see A]. Only check DAI active status in soc_pcm_close() is also not enough
> since it will cause the cleanup just happening in soc_pcm_close() [see B].
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: soc-pcm.c: Make sure DAI parameters cleared if the DAI becomes inactive
commit: 3efcb471f871cc095841d411f98c593228ecbac6
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark