Re: [PATCH v3 2/2] ASoC: soc-compress: split soc-compress to a module

[Takashi Iwai <tiwai@suse.de>]

On Tue, 13 Oct 2015 14:42:11 +0200,
Jie, Yang wrote:
> 
> > -----Original Message-----
> > From: Koul, Vinod
> > Sent: Tuesday, October 13, 2015 7:50 PM
> > To: Jie, Yang; broonie@kernel.org
> > Cc: Girdwood, Liam R; alsa-devel@alsa-project.org
> > Subject: Re: [alsa-devel] [PATCH v3 2/2] ASoC: soc-compress: split soc-
> > compress to a module
> > 
> > On Tue, 2015-10-13 at 17:11 +0800, Jie Yang wrote:
> > > +/* Module information */
> > > +MODULE_AUTHOR("Namarta Kohli <namartax.kohli@intel.com>");
> > > +MODULE_AUTHOR("Ramesh Babu K V <ramesh.babu@linux.intel.com>");
> > > +MODULE_AUTHOR("Vinod Koul <vinod.koul@linux.intel.com>");
> > 
> > Vinod Koul <vinod.koul@intel.com> please
> > 
> 
> OK, I will change this one. 
> 
> > I need to move other instances too :(
> > 
> > 
> > > +MODULE_DESCRIPTION("ALSA SoC Compress");
> > MODULE_LICENSE("GPL");
> > > +MODULE_ALIAS("platform:soc-compress");
> > > diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index
> > > ff8bda4..9e32151 100644
> > > --- a/sound/soc/soc-dapm.c
> > > +++ b/sound/soc/soc-dapm.c
> > > @@ -3896,6 +3896,7 @@ void snd_soc_dapm_stream_event(struct
> > > snd_soc_pcm_runtime *rtd, int stream,
> > >  	soc_dapm_stream_event(rtd, stream, event);
> > >  	mutex_unlock(&card->dapm_mutex);
> > >  }
> > > +EXPORT_SYMBOL_GPL(snd_soc_dapm_stream_event);
> > 
> > These exports should be a new patch
> > 
> > And as we agreed this should have Documentation if not already done :)
> 
> It's not what I really want to do, it introduce 13 exports
> (EXPORT_SYMBOL_GPL) when we want split soc-compress.c to a separate
> Module, and adding documentation to all of these 13 functions(only for
> soc-compress.c to use them) looks somewhat ungraceful :(
> 
> Hi Takashi, what's your opinion? Maybe we should keep soc-compress.c
> in snd-soc-core.ko, which means skip/ignore this patch and don’t split it?

Well, I'm open about it.  Of course, making it as a module would make
sense, even for normal use cases.  But allowing the selective build of
soc-compress.c should be enough for most cases.  (In that case, the
new kconfig should be a boolean.)

As Liam suggested, if the exported functions are supposed to be
internal use only, it's also the very reason you have to write
documentation, too.  The function description itself can be concise,
then.

Another possibility is to refactor soc-compress.c.  If you compare the
code with soc-pcm.c, you see very much similarity.  But it's a path
that takes much longer, although it might be cleaner.


Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel