Re: [PATCH 001/112] ALSA: add snd_pcm_is_playback/capture() macro

[Takashi Sakamoto <o-takashi@sakamocchi.jp>]

Hi,

On Wed, Jul 24, 2024 at 01:59:48AM +0000, Kuninori Morimoto wrote:
> Many drivers are using below code to know the direction.
> 
> 	if (direction == SNDRV_PCM_STREAM_PLAYBACK)
> 
> Add snd_pcm_is_playback/capture() macro to handle it.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  include/sound/pcm.h | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index 3edd7a7346daa..b3d4a928e41a4 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -501,6 +501,41 @@ struct snd_pcm_substream {
>  
>  #define SUBSTREAM_BUSY(substream) ((substream)->ref_count > 0)
>  
> +static inline int snd_pcm_direction_is_playback(const int stream)
> +{
> +	return stream == SNDRV_PCM_STREAM_PLAYBACK;
> +}
> +
> +static inline int snd_pcm_direction_is_capture(const int stream)
> +{
> +	return stream == SNDRV_PCM_STREAM_CAPTURE;
> +}
> +
> +static inline int snd_pcm_substream_is_playback(const struct snd_pcm_substream *substream)
> +{
> +	return snd_pcm_direction_is_playback(substream->stream);
> +}
> +
> +static inline int snd_pcm_substream_is_capture(const struct snd_pcm_substream *substream)
> +{
> +	return snd_pcm_direction_is_capture(substream->stream);
> +}
> +
> +#define snd_pcm_is_playback(x) _Generic((x), \
> +		 int			: snd_pcm_direction_is_playback, \
> +	unsigned int			: snd_pcm_direction_is_playback, \
> +	unsigned char			: snd_pcm_direction_is_playback, \
> +	unsigned short			: snd_pcm_direction_is_playback, \
> +	      struct snd_pcm_substream *: snd_pcm_substream_is_playback, \
> +	const struct snd_pcm_substream *: snd_pcm_substream_is_playback)(x)
> +
> +#define snd_pcm_is_capture(x) _Generic((x),				\
> +		 int			: snd_pcm_direction_is_capture, \
> +	unsigned int			: snd_pcm_direction_is_capture, \
> +	unsigned char			: snd_pcm_direction_is_capture, \
> +	unsigned short			: snd_pcm_direction_is_capture, \
> +	      struct snd_pcm_substream *: snd_pcm_substream_is_capture, \
> +	const struct snd_pcm_substream *: snd_pcm_substream_is_capture)(x)
>  
>  struct snd_pcm_str {
>  	int stream;				/* stream (direction) */
> -- 
> 2.43.0

In my opinion, it is not so important to distinguish some types which can
be converted to integer implicitly/explicitly in the above case. The
'default' association is available in such case, like:

+#define snd_pcm_is_playback(x) _Generic((x), \
+	      struct snd_pcm_substream *: snd_pcm_substream_is_playback, \
+	const struct snd_pcm_substream *: snd_pcm_substream_is_playback, \
+	                         default: snd_pcm_direction_is_playback)(x)

The association would match [u|i][8|16|32|64] and f[32|64] types, and would
not match to any type of pointers. However, it depends on your preference.


Regards

Takashi Sakamoto