| Message ID | 1432591459-22613-2-git-send-email-alexandre.belloni@free-electrons.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0ef9dc139db2fca304ce4eadb5b8fb40d3dedb5e |
| Headers | show |
On Tue, May 26, 2015 at 12:04:19AM +0200, Alexandre Belloni wrote: > It is currently possible to have CONFIG_SND_ATMEL_SOC_SSC=y with either > CONFIG_SND_ATMEL_SOC_PDC=m or CONFIG_SND_ATMEL_SOC_DMA=m. This results in a > driver that compiles but does not link with this kind of error: Applied, thanks.
Le 26/05/2015 00:04, Alexandre Belloni a écrit : > It is currently possible to have CONFIG_SND_ATMEL_SOC_SSC=y with either > CONFIG_SND_ATMEL_SOC_PDC=m or CONFIG_SND_ATMEL_SOC_DMA=m. This results in a > driver that compiles but does not link with this kind of error: > > sound/built-in.o: In function `atmel_ssc_set_audio': > (.text+0x87d90): undefined reference to `atmel_pcm_pdc_platform_register' > sound/built-in.o: In function `atmel_ssc_put_audio': > (.text+0x8879a): undefined reference to `atmel_pcm_pdc_platform_unregister' > > Solve that by compiling the selected PCM driver (PDC, DMA or both) in the > Atmel SSC DAI driver. > > Reported-by: Randy Dunlap <rdunlap@infradead.org> > > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> For the record: Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com> > --- > sound/soc/atmel/Kconfig | 4 ++-- > sound/soc/atmel/Makefile | 8 +++----- > 2 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig > index 93abe4e6d596..c3152072d682 100644 > --- a/sound/soc/atmel/Kconfig > +++ b/sound/soc/atmel/Kconfig > @@ -9,10 +9,10 @@ config SND_ATMEL_SOC > if SND_ATMEL_SOC > > config SND_ATMEL_SOC_PDC > - tristate > + bool > > config SND_ATMEL_SOC_DMA > - tristate > + bool > select SND_SOC_GENERIC_DMAENGINE_PCM > > config SND_ATMEL_SOC_SSC > diff --git a/sound/soc/atmel/Makefile b/sound/soc/atmel/Makefile > index b327e5cc8de3..4fa7ac91f972 100644 > --- a/sound/soc/atmel/Makefile > +++ b/sound/soc/atmel/Makefile > @@ -1,10 +1,8 @@ > # AT91 Platform Support > -snd-soc-atmel-pcm-pdc-objs := atmel-pcm-pdc.o > -snd-soc-atmel-pcm-dma-objs := atmel-pcm-dma.o > -snd-soc-atmel_ssc_dai-objs := atmel_ssc_dai.o > +snd-soc-atmel-pcm-$(CONFIG_SND_ATMEL_SOC_PDC) := atmel-pcm-pdc.o > +snd-soc-atmel-pcm-$(CONFIG_SND_ATMEL_SOC_DMA) += atmel-pcm-dma.o > +snd-soc-atmel_ssc_dai-objs := atmel_ssc_dai.o $(snd-soc-atmel-pcm-y) > > -obj-$(CONFIG_SND_ATMEL_SOC_PDC) += snd-soc-atmel-pcm-pdc.o > -obj-$(CONFIG_SND_ATMEL_SOC_DMA) += snd-soc-atmel-pcm-dma.o > obj-$(CONFIG_SND_ATMEL_SOC_SSC) += snd-soc-atmel_ssc_dai.o > > # AT91 Machine Support >
On Tuesday 26 May 2015 00:04:19 Alexandre Belloni wrote: > index b327e5cc8de3..4fa7ac91f972 100644 > --- a/sound/soc/atmel/Makefile > +++ b/sound/soc/atmel/Makefile > @@ -1,10 +1,8 @@ > # AT91 Platform Support > -snd-soc-atmel-pcm-pdc-objs := atmel-pcm-pdc.o > -snd-soc-atmel-pcm-dma-objs := atmel-pcm-dma.o > -snd-soc-atmel_ssc_dai-objs := atmel_ssc_dai.o > +snd-soc-atmel-pcm-$(CONFIG_SND_ATMEL_SOC_PDC) := atmel-pcm-pdc.o > +snd-soc-atmel-pcm-$(CONFIG_SND_ATMEL_SOC_DMA) += atmel-pcm-dma.o > +snd-soc-atmel_ssc_dai-objs := atmel_ssc_dai.o $(snd-soc-atmel-pcm-y) While technically correct, you could have written this (slightly) simpler as: snd-soc-atmel_ssc_dai-objs := atmel_ssc_dai.o snd-soc-atmel-ssc_dai-$(CONFIG_SND_ATMEL_SOC_PDC) += atmel-pcm-pdc.o snd-soc-atmel-ssc_dai-$(CONFIG_SND_ATMEL_SOC_DMA) += atmel-pcm-dma.o No need to change the patch again after it's applied now. Arnd
Now that this patch is already applied my remarks can only be addressed in a follow up patch. (Perhaps such a patch is already queued.) On Tue, 2015-05-26 at 00:04 +0200, Alexandre Belloni wrote: > --- a/sound/soc/atmel/Kconfig > +++ b/sound/soc/atmel/Kconfig > config SND_ATMEL_SOC_PDC > - tristate > + bool > > config SND_ATMEL_SOC_DMA > - tristate > + bool > select SND_SOC_GENERIC_DMAENGINE_PCM > --- a/sound/soc/atmel/Makefile > +++ b/sound/soc/atmel/Makefile > -snd-soc-atmel-pcm-pdc-objs := atmel-pcm-pdc.o > -snd-soc-atmel-pcm-dma-objs := atmel-pcm-dma.o > -snd-soc-atmel_ssc_dai-objs := atmel_ssc_dai.o > +snd-soc-atmel-pcm-$(CONFIG_SND_ATMEL_SOC_PDC) := atmel-pcm-pdc.o > +snd-soc-atmel-pcm-$(CONFIG_SND_ATMEL_SOC_DMA) += atmel-pcm-dma.o > +snd-soc-atmel_ssc_dai-objs := atmel_ssc_dai.o $(snd-soc-atmel-pcm-y) > > -obj-$(CONFIG_SND_ATMEL_SOC_PDC) += snd-soc-atmel-pcm-pdc.o > -obj-$(CONFIG_SND_ATMEL_SOC_DMA) += snd-soc-atmel-pcm-dma.o > obj-$(CONFIG_SND_ATMEL_SOC_SSC) += snd-soc-atmel_ssc_dai.o The code in atmel-pcm-pdc.c and atmel-pcm-dma.c will now either be part of the snd-soc-atmel_ssc_dai.ko or be built-in. That means, I think, that: - the (in total) four uses of EXPORT_SYMBOL() in these two files can be dropped; - MODULE_AUTHOR() and friends, and probably also the include of linux/module.h, can be dropped from these two files. Furthermore, the references to CONFIG_SND_ATMEL_SOC_PDC_MODULE and CONFIG_SND_ATMEL_SOC_DMA_MODULE in atmel-pcm.h can be removed now. Thanks, Paul Bolle
On 27/05/2015 at 09:50:24 +0200, Paul Bolle wrote : > Now that this patch is already applied my remarks can only be addressed > in a follow up patch. (Perhaps such a patch is already queued.) > > On Tue, 2015-05-26 at 00:04 +0200, Alexandre Belloni wrote: > > --- a/sound/soc/atmel/Kconfig > > +++ b/sound/soc/atmel/Kconfig > > > config SND_ATMEL_SOC_PDC > > - tristate > > + bool > > > > config SND_ATMEL_SOC_DMA > > - tristate > > + bool > > select SND_SOC_GENERIC_DMAENGINE_PCM > > > --- a/sound/soc/atmel/Makefile > > +++ b/sound/soc/atmel/Makefile > > > -snd-soc-atmel-pcm-pdc-objs := atmel-pcm-pdc.o > > -snd-soc-atmel-pcm-dma-objs := atmel-pcm-dma.o > > -snd-soc-atmel_ssc_dai-objs := atmel_ssc_dai.o > > +snd-soc-atmel-pcm-$(CONFIG_SND_ATMEL_SOC_PDC) := atmel-pcm-pdc.o > > +snd-soc-atmel-pcm-$(CONFIG_SND_ATMEL_SOC_DMA) += atmel-pcm-dma.o > > +snd-soc-atmel_ssc_dai-objs := atmel_ssc_dai.o $(snd-soc-atmel-pcm-y) > > > > -obj-$(CONFIG_SND_ATMEL_SOC_PDC) += snd-soc-atmel-pcm-pdc.o > > -obj-$(CONFIG_SND_ATMEL_SOC_DMA) += snd-soc-atmel-pcm-dma.o > > obj-$(CONFIG_SND_ATMEL_SOC_SSC) += snd-soc-atmel_ssc_dai.o > > The code in atmel-pcm-pdc.c and atmel-pcm-dma.c will now either be part > of the snd-soc-atmel_ssc_dai.ko or be built-in. That means, I think, > that: > - the (in total) four uses of EXPORT_SYMBOL() in these two files can be > dropped; > - MODULE_AUTHOR() and friends, and probably also the include of > linux/module.h, can be dropped from these two files. > Yeah, I as not sure how to merge those MODULE_AUTHOR but I checked and the information is correctly included. > Furthermore, the references to CONFIG_SND_ATMEL_SOC_PDC_MODULE and > CONFIG_SND_ATMEL_SOC_DMA_MODULE in atmel-pcm.h can be removed now. > Indeed. However, the Kconfig maintainer found a way to do the right thing so we may as well drop that patch and keep those as modules. Nicolas, what do you think?
On Wed, 2015-05-27 at 11:19 +0200, Alexandre Belloni wrote: > However, the Kconfig maintainer found a way to do the right thing so we > may as well drop that patch and keep those as modules. Perhaps I missed a message: do you have a link? (I fiddled a bit with the build setup of these drivers too, but then noticed that the patch was already applied, and abandoned that.) Thanks, Paul Bolle
diff --git a/sound/soc/atmel/Kconfig b/sound/soc/atmel/Kconfig index 93abe4e6d596..c3152072d682 100644 --- a/sound/soc/atmel/Kconfig +++ b/sound/soc/atmel/Kconfig @@ -9,10 +9,10 @@ config SND_ATMEL_SOC if SND_ATMEL_SOC config SND_ATMEL_SOC_PDC - tristate + bool config SND_ATMEL_SOC_DMA - tristate + bool select SND_SOC_GENERIC_DMAENGINE_PCM config SND_ATMEL_SOC_SSC diff --git a/sound/soc/atmel/Makefile b/sound/soc/atmel/Makefile index b327e5cc8de3..4fa7ac91f972 100644 --- a/sound/soc/atmel/Makefile +++ b/sound/soc/atmel/Makefile @@ -1,10 +1,8 @@ # AT91 Platform Support -snd-soc-atmel-pcm-pdc-objs := atmel-pcm-pdc.o -snd-soc-atmel-pcm-dma-objs := atmel-pcm-dma.o -snd-soc-atmel_ssc_dai-objs := atmel_ssc_dai.o +snd-soc-atmel-pcm-$(CONFIG_SND_ATMEL_SOC_PDC) := atmel-pcm-pdc.o +snd-soc-atmel-pcm-$(CONFIG_SND_ATMEL_SOC_DMA) += atmel-pcm-dma.o +snd-soc-atmel_ssc_dai-objs := atmel_ssc_dai.o $(snd-soc-atmel-pcm-y) -obj-$(CONFIG_SND_ATMEL_SOC_PDC) += snd-soc-atmel-pcm-pdc.o -obj-$(CONFIG_SND_ATMEL_SOC_DMA) += snd-soc-atmel-pcm-dma.o obj-$(CONFIG_SND_ATMEL_SOC_SSC) += snd-soc-atmel_ssc_dai.o # AT91 Machine Support
It is currently possible to have CONFIG_SND_ATMEL_SOC_SSC=y with either CONFIG_SND_ATMEL_SOC_PDC=m or CONFIG_SND_ATMEL_SOC_DMA=m. This results in a driver that compiles but does not link with this kind of error: sound/built-in.o: In function `atmel_ssc_set_audio': (.text+0x87d90): undefined reference to `atmel_pcm_pdc_platform_register' sound/built-in.o: In function `atmel_ssc_put_audio': (.text+0x8879a): undefined reference to `atmel_pcm_pdc_platform_unregister' Solve that by compiling the selected PCM driver (PDC, DMA or both) in the Atmel SSC DAI driver. Reported-by: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> --- sound/soc/atmel/Kconfig | 4 ++-- sound/soc/atmel/Makefile | 8 +++----- 2 files changed, 5 insertions(+), 7 deletions(-)