On Mon, 27 Jun 2022 05:04:38 +0200,
Kuninori Morimoto wrote:
>
>
> Hi Takashi
>
> May I ask about this patch ?
>
> ASoC: soc-pcm: Fix and cleanup DPCM locking
> b7898396f4bbe160f546d0c5e9fa17cca9a7d153
Sorry for the very late response. It's been completely overlooked.
> Q1.
>
> It exchanges many function parameters
>
> - func(substream, xxx)
> + func(rtd, substream, xxx)
> ^^^^
>
> I guess the purpose is it want to call snd_soc_dpcm_mutex_assert_held()
> inside the func ? But it looks very verbose.
> we can do like this, but am I misunderstanding ?
>
> func(substream, xxx) {
> struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
>
> snd_soc_dpcm_mutex_assert_held(rtd);
>
> ...
> }
This could be changed in that way, too.
I just took the current code as the compiler produced a bit better
code.
> Q2.
>
> It added new __soc_pcm_close().
> But soc_pcm_close() is using soc_pcm_clean() instead of __soc_pcm_close().
> Is this just a mistake ?
>
> static int soc_pcm_close(struct snd_pcm_substream *substream)
> {
> struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
>
> snd_soc_dpcm_mutex_lock(rtd);
> - soc_pcm_clean(substream, 0);
> + __soc_pcm_close(substream);
> snd_soc_dpcm_mutex_unlock(rtd);
> return 0;
> }
Not really a mistake, as you can see, that's the very same code :)
That said, it's fine to call __soc_pcm_close() there instead of the
open code, too, as long as the resultant binary becomes same (or
better).
thanks,
Takashi
