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

[Jie, Yang]

> -----Original Message-----
> From: Charles Keepax [mailto:ckeepax at opensource.wolfsonmicro.com]
> Sent: Wednesday, October 14, 2015 5:21 PM
> To: Jie, Yang
> Cc: broonie at kernel.org; alsa-devel at alsa-project.org; Girdwood, Liam R
> Subject: Re: [alsa-devel] [PATCH v4] ASoC: soc-compress: add config item for
> soc-compress to make it compiled only when needed
> 
> 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 at 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?

Hi Charles, we used compress_dai bool before(in v2), but for that we need
add empty inline soc_compress_new(), Takashi suggest we use callback
func pointer here, it can be compress_new or any other creation here.
This is flexible and can be extended if any new stuff has to be handled.

> 
> > 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...)

This is roll back to the old compress_dai bool? 

~Keyon

> 
> 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