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