| Message ID | 1444727464-10385-3-git-send-email-yang.jie@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 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 :)
> -----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? ~Keyon > > -- > ~Vinod
On Tue, 2015-10-13 at 13:42 +0100, 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 :( > If these are internal functions only (and used only by one user) then adding some small kernel doc comments above each function should be enough. Liam > 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? > > ~Keyon > > > > -- > > ~Vinod --------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
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
> -----Original Message----- > From: Takashi Iwai [mailto:tiwai@suse.de] > Sent: Tuesday, October 13, 2015 10:19 PM > To: Jie, Yang > Cc: Koul, Vinod; broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, > Liam R > Subject: Re: [alsa-devel] [PATCH v3 2/2] ASoC: soc-compress: split soc- > compress to a module > > 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. Thank you, Takashi, then I prefer only allowing the selective build of soc-compress.c here. Will update patch soon. Thanks, ~Keyon > > > Takashi
diff --git a/sound/soc/Makefile b/sound/soc/Makefile index a3a1505..dacddf4 100644 --- a/sound/soc/Makefile +++ b/sound/soc/Makefile @@ -1,9 +1,8 @@ snd-soc-core-objs := soc-core.o soc-dapm.o soc-jack.o soc-cache.o soc-utils.o snd-soc-core-objs += soc-pcm.o soc-io.o soc-devres.o soc-ops.o -ifneq ($(CONFIG_SND_SOC_COMPRESS),) -snd-soc-core-objs += soc-compress.o -endif +snd-soc-compress-objs := soc-compress.o +obj-$(CONFIG_SND_SOC_COMPRESS) += snd-soc-compress.o ifneq ($(CONFIG_SND_SOC_TOPOLOGY),) snd-soc-core-objs += soc-topology.o diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index a672d9c..28bb1b2 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -19,6 +19,7 @@ #include <linux/delay.h> #include <linux/slab.h> #include <linux/workqueue.h> +#include <linux/module.h> #include <sound/core.h> #include <sound/compress_params.h> #include <sound/compress_driver.h> @@ -704,3 +705,11 @@ compr_err: return ret; } EXPORT_SYMBOL_GPL(soc_new_compress); + +/* 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>"); +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); /** * snd_soc_dapm_enable_pin_unlocked - enable pin. diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 3173958..517a9d4 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -86,6 +86,7 @@ void snd_soc_runtime_activate(struct snd_soc_pcm_runtime *rtd, int stream) rtd->codec_dais[i]->component->active++; } } +EXPORT_SYMBOL_GPL(snd_soc_runtime_activate); /** * snd_soc_runtime_deactivate() - Decrement active count for PCM runtime components @@ -121,6 +122,7 @@ void snd_soc_runtime_deactivate(struct snd_soc_pcm_runtime *rtd, int stream) rtd->codec_dais[i]->active--; } } +EXPORT_SYMBOL_GPL(snd_soc_runtime_deactivate); /** * snd_soc_runtime_ignore_pmdown_time() - Check whether to ignore the power down delay @@ -144,6 +146,7 @@ bool snd_soc_runtime_ignore_pmdown_time(struct snd_soc_pcm_runtime *rtd) return rtd->cpu_dai->component->ignore_pmdown_time && ignore; } +EXPORT_SYMBOL_GPL(snd_soc_runtime_ignore_pmdown_time); /** * snd_soc_set_runtime_hwparams - set the runtime hardware parameters @@ -188,6 +191,7 @@ int dpcm_dapm_stream_event(struct snd_soc_pcm_runtime *fe, int dir, return 0; } +EXPORT_SYMBOL_GPL(dpcm_dapm_stream_event); static int soc_pcm_apply_symmetry(struct snd_pcm_substream *substream, struct snd_soc_dai *soc_dai) @@ -1209,6 +1213,7 @@ void dpcm_be_disconnect(struct snd_soc_pcm_runtime *fe, int stream) kfree(dpcm); } } +EXPORT_SYMBOL_GPL(dpcm_be_disconnect); /* get BE for DAI widget and stream */ static struct snd_soc_pcm_runtime *dpcm_get_be(struct snd_soc_card *card, @@ -1293,6 +1298,7 @@ int dpcm_path_get(struct snd_soc_pcm_runtime *fe, return paths; } +EXPORT_SYMBOL_GPL(dpcm_path_get); static int dpcm_prune_paths(struct snd_soc_pcm_runtime *fe, int stream, struct snd_soc_dapm_widget_list **list_) @@ -1405,6 +1411,7 @@ int dpcm_process_paths(struct snd_soc_pcm_runtime *fe, else return dpcm_prune_paths(fe, stream, list); } +EXPORT_SYMBOL_GPL(dpcm_process_paths); void dpcm_clear_pending_state(struct snd_soc_pcm_runtime *fe, int stream) { @@ -1414,6 +1421,7 @@ void dpcm_clear_pending_state(struct snd_soc_pcm_runtime *fe, int stream) dpcm->be->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO; } +EXPORT_SYMBOL_GPL(dpcm_clear_pending_state); static void dpcm_be_dai_startup_unwind(struct snd_soc_pcm_runtime *fe, int stream) @@ -1530,6 +1538,7 @@ unwind: return err; } +EXPORT_SYMBOL_GPL(dpcm_be_dai_startup); static void dpcm_init_runtime_hw(struct snd_pcm_runtime *runtime, struct snd_soc_pcm_stream *stream, @@ -1693,6 +1702,7 @@ int dpcm_be_dai_shutdown(struct snd_soc_pcm_runtime *fe, int stream) } return 0; } +EXPORT_SYMBOL_GPL(dpcm_be_dai_shutdown); static int dpcm_fe_dai_shutdown(struct snd_pcm_substream *substream) { @@ -1758,6 +1768,7 @@ int dpcm_be_dai_hw_free(struct snd_soc_pcm_runtime *fe, int stream) return 0; } +EXPORT_SYMBOL_GPL(dpcm_be_dai_hw_free); static int dpcm_fe_dai_hw_free(struct snd_pcm_substream *substream) { @@ -1865,6 +1876,7 @@ unwind: return ret; } +EXPORT_SYMBOL_GPL(dpcm_be_dai_hw_params); static int dpcm_fe_dai_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) @@ -2134,6 +2146,7 @@ int dpcm_be_dai_prepare(struct snd_soc_pcm_runtime *fe, int stream) } return ret; } +EXPORT_SYMBOL_GPL(dpcm_be_dai_prepare); static int dpcm_fe_dai_prepare(struct snd_pcm_substream *substream) {
Split soc-compress into a separate module so we only compile/load it when it's needed. Signed-off-by: Jie Yang <yang.jie@intel.com> --- sound/soc/Makefile | 5 ++--- sound/soc/soc-compress.c | 9 +++++++++ sound/soc/soc-dapm.c | 1 + sound/soc/soc-pcm.c | 13 +++++++++++++ 4 files changed, 25 insertions(+), 3 deletions(-)