[alsa-devel] [PATCH 10/25] ASoC: soc-component: add snd_soc_component_ioctrl()

[Charles Keepax]

On Wed, Jul 24, 2019 at 10:52:05AM +0900, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
> 
> Current ALSA SoC is directly using component->driver->ops->xxx,
> thus, it is deep nested, and makes code difficult to read,
> and is not good for encapsulation.
> This patch adds new snd_soc_component_ioctrl() and use it.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
> ---
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 7ab68de..064d16c 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -1116,6 +1116,7 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
>  		component = rtdcom->component;
>  
>  		offset = snd_soc_component_pointer(component, substream);
> +

This whitespace change belongs in the previous patch.

>  		/* FIXME: use 1st pointer */
>  		if (offset > 0)
>  			break;
> @@ -2455,16 +2456,15 @@ static int soc_pcm_ioctl(struct snd_pcm_substream *substream,
>  	struct snd_soc_pcm_runtime *rtd = substream->private_data;
>  	struct snd_soc_component *component;
>  	struct snd_soc_rtdcom_list *rtdcom;
> +	int ret;
>  
>  	for_each_rtdcom(rtd, rtdcom) {
>  		component = rtdcom->component;
>  
> -		if (!component->driver->ops ||
> -		    !component->driver->ops->ioctl)
> -			continue;
> -
> +		ret = snd_soc_component_ioctl(component, substream, cmd, arg);
>  		/* FIXME: use 1st ioctl */
> -		return component->driver->ops->ioctl(substream, cmd, arg);
> +		if (ret != -ENOTSUPP)
> +			return ret;

This feels a little forced, and also changes behaviour if any
ioctl callbacks already report -ENOTSUPP.

I wonder if for some/all of these functions it might be worth
abstracting them at a higher level so the whole for_each_rtdcom
loop moves into the helper function similar to what I did with
compress stuff, for example in these patches:

1e57b82891ad ("ASoC: compress: Add helper functions for component open/free")
4ef0ecb80e34 ("ASoC: compress: Add helper functions for component trigger/set_params")

Thanks,
Charles