Re: [PATCH v4] ASoC: soc-compress: add config item for soc-compress to make it compiled only when needed

[Charles Keepax <ckeepax@opensource.wolfsonmicro.com>]

On Tue, Oct 13, 2015 at 11:41:00PM +0800, Jie Yang wrote:
> We don't always need soc-compress in soc, here add a config item
> SND_SOC_COMPRESS, when nobody select it, the soc-compress will
> not be compiled.
> 
> Here also change Kconfig to 'select SND_SOC_COMPRESS' for drivers
> that needed soc-compress.
> 
> Signed-off-by: Jie Yang <yang.jie@intel.com>
> ---
>  include/sound/soc-dai.h                      |  2 +-
>  include/sound/soc.h                          |  4 +++-
>  sound/soc/Kconfig                            |  5 ++++-
>  sound/soc/Makefile                           |  3 ++-
>  sound/soc/intel/Kconfig                      |  1 +
>  sound/soc/intel/atom/sst-mfld-platform-pcm.c |  2 +-
>  sound/soc/soc-compress.c                     | 12 ++++++++++--
>  sound/soc/soc-core.c                         |  4 ++--
>  8 files changed, 24 insertions(+), 9 deletions(-)
> 
> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
> index 2df96b1..238200f 100644
> --- a/include/sound/soc-dai.h
> +++ b/include/sound/soc-dai.h
> @@ -214,7 +214,7 @@ struct snd_soc_dai_driver {
>  	int (*suspend)(struct snd_soc_dai *dai);
>  	int (*resume)(struct snd_soc_dai *dai);
>  	/* compress dai */
> -	bool compress_dai;
> +	int (*compress_new)(struct snd_soc_pcm_runtime *rtd, int num);

This feels a little awkward to be using a function pointer here.
It somewhat implies I might want to customise this function but
am I every going to want to set this to something other than
snd_soc_new_compress?

> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 3b471f9..24b0960 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1370,9 +1370,9 @@ static int soc_probe_link_dais(struct snd_soc_card *card, int num, int order)
>  		soc_dpcm_debugfs_add(rtd);
>  #endif
>  
> -	if (cpu_dai->driver->compress_dai) {
> +	if (cpu_dai->driver->compress_new) {

Would it maybe be nicer to just change this to:
if (IS_ENABLED(..)) && cpu_dai...)

Anyone else have any thoughts on that?

>  		/*create compress_device"*/
> -		ret = soc_new_compress(rtd, num);
> +		ret = cpu_dai->driver->compress_new(rtd, num);
>  		if (ret < 0) {
>  			dev_err(card->dev, "ASoC: can't create compress %s\n",
>  					 dai_link->stream_name);

Thanks,
Charles