[PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

[PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

[Arnd Bergmann]

Since commit 204dec93eaa "ASoC: fsl: Allow to select individual common
options", it is possible to enable SND_SOC_FSL_SSI and SND_SOC_FSL_SPDIF
manually. However, these two options are for drivers that both
require the imx-pcm-dma framework and result in a link error if that
is not provided.

This changes Kconfig to automatically select SND_SOC_IMX_PCM_DMA
to avoid the problem, as we do for other drivers that need the same
framework.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index fb26345..25abd21 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -9,6 +9,7 @@ config SND_SOC_FSL_SAI
 
 config SND_SOC_FSL_SSI
 	tristate "Synchronous Serial Interface module support"
+	select SND_SOC_IMX_PCM_DMA
 	help
 	  Say Y if you want to add Synchronous Serial Interface (SSI)
 	  support for the Freescale CPUs.
@@ -17,6 +18,7 @@ config SND_SOC_FSL_SSI
 
 config SND_SOC_FSL_SPDIF
 	tristate "Sony/Philips Digital Interface module support"
+	select SND_SOC_IMX_PCM_DMA
 	select REGMAP_MMIO
 	help
 	  Say Y if you want to add Sony/Philips Digital Interface (SPDIF)

Re: [PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

[Arnd Bergmann]

On Monday 28 April 2014 22:46:10 Alexander Shiyan wrote:
> Mon, 28 Apr 2014 16:46:30 +0200 от Arnd Bergmann <arnd@arndb.de>:
> > Since commit 204dec93eaa "ASoC: fsl: Allow to select individual common
> > options", it is possible to enable SND_SOC_FSL_SSI and SND_SOC_FSL_SPDIF
> > manually. However, these two options are for drivers that both
> > require the imx-pcm-dma framework and result in a link error if that
> > is not provided.
> > 
> > This changes Kconfig to automatically select SND_SOC_IMX_PCM_DMA
> > to avoid the problem, as we do for other drivers that need the same
> > framework.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Can you give an exact error?


sound/built-in.o: In function `fsl_ssi_probe':
:(.text+0x51fb8): undefined reference to `imx_pcm_dma_init'
sound/built-in.o: In function `fsl_spdif_probe':
:(.text+0x52e20): undefined reference to `imx_pcm_dma_init'


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

Re: [PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

[Arnd Bergmann]

On Tuesday 29 April 2014 00:35:41 Alexander Shiyan wrote:
> Mon, 28 Apr 2014 21:52:16 +0200 от Arnd Bergmann <arnd@arndb.de>:
> > On Monday 28 April 2014 22:46:10 Alexander Shiyan wrote:
> > > Mon, 28 Apr 2014 16:46:30 +0200 от Arnd Bergmann <arnd@arndb.de>:
> > > > Since commit 204dec93eaa "ASoC: fsl: Allow to select individual common
> > > > options", it is possible to enable SND_SOC_FSL_SSI and SND_SOC_FSL_SPDIF
> > > > manually. However, these two options are for drivers that both
> > > > require the imx-pcm-dma framework and result in a link error if that
> > > > is not provided.
> > > > 
> > > > This changes Kconfig to automatically select SND_SOC_IMX_PCM_DMA
> > > > to avoid the problem, as we do for other drivers that need the same
> > > > framework.
> > > > 
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > 
> > > Can you give an exact error?
> > 
> > 
> > sound/built-in.o: In function `fsl_ssi_probe':
> > :(.text+0x51fb8): undefined reference to `imx_pcm_dma_init'
> > sound/built-in.o: In function `fsl_spdif_probe':
> > :(.text+0x52e20): undefined reference to `imx_pcm_dma_init'
> 
> This call is defined in imx-pcm.h as:
> #if IS_ENABLED(CONFIG_SND_SOC_IMX_PCM_DMA)
> int imx_pcm_dma_init(struct platform_device *pdev);
> #else
> static inline int imx_pcm_dma_init(struct platform_device *pdev)
> {
>   return -ENODEV;
> }
> #endif
> 
> So, I don't understand why this error happen, as well as I can not
> reproduce this...

It's probably CONFIG_SND_SOC_IMX_PCM_DMA=m and CONFIG_SND_SOC_FSL_SSI=y then.
What is the intended behavior in this case? Should CONFIG_SND_SOC_FSL_SSI
be forced to be a module as well?

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

Re: [PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

[Arnd Bergmann]

On Tuesday 29 April 2014 07:07:33 Alexander Shiyan wrote:
> Mon, 28 Apr 2014 23:12:14 +0200 от Arnd Bergmann <arnd@arndb.de>:
> > On Tuesday 29 April 2014 00:35:41 Alexander Shiyan wrote:
> > > 
> > > So, I don't understand why this error happen, as well as I can not
> > > reproduce this...
> > 
> > It's probably CONFIG_SND_SOC_IMX_PCM_DMA=m and CONFIG_SND_SOC_FSL_SSI=y then.
> > What is the intended behavior in this case? Should CONFIG_SND_SOC_FSL_SSI
> > be forced to be a module as well?
> 
> Hmm, yes...
> I thought that I had already solved a similar problem for the earlier
> version of the patch ...

How about this?

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index fb26345..af0bb92 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -9,6 +9,7 @@ config SND_SOC_FSL_SAI
 
 config SND_SOC_FSL_SSI
 	tristate "Synchronous Serial Interface module support"
+	depends on m || SND_SOC_IMX_PCM_DMA != m
 	help
 	  Say Y if you want to add Synchronous Serial Interface (SSI)
 	  support for the Freescale CPUs.
@@ -18,6 +19,7 @@ config SND_SOC_FSL_SSI
 config SND_SOC_FSL_SPDIF
 	tristate "Sony/Philips Digital Interface module support"
 	select REGMAP_MMIO
+	depends on m || SND_SOC_IMX_PCM_DMA != m
 	help
 	  Say Y if you want to add Sony/Philips Digital Interface (SPDIF)
 	  support for the Freescale CPUs.

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

Re: [PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

[Lars-Peter Clausen]

On 04/29/2014 12:37 PM, Arnd Bergmann wrote:
> On Tuesday 29 April 2014 07:07:33 Alexander Shiyan wrote:
>> Mon, 28 Apr 2014 23:12:14 +0200 от Arnd Bergmann <arnd@arndb.de>:
>>> On Tuesday 29 April 2014 00:35:41 Alexander Shiyan wrote:
>>>>
>>>> So, I don't understand why this error happen, as well as I can not
>>>> reproduce this...
>>>
>>> It's probably CONFIG_SND_SOC_IMX_PCM_DMA=m and CONFIG_SND_SOC_FSL_SSI=y then.
>>> What is the intended behavior in this case? Should CONFIG_SND_SOC_FSL_SSI
>>> be forced to be a module as well?
>>
>> Hmm, yes...
>> I thought that I had already solved a similar problem for the earlier
>> version of the patch ...
>
> How about this?

Having FSL_SSI/FSL_SPDIF, but not SND_SOC_IMX_PCM_DMA does not make sense on 
iMX.
So how about:

	select SND_SOC_IMX_PCM_DMA if SND_IMX_SOC

and remove all the other 'select SND_SOC_IMX_PCM_DMA' statements. That's in 
my opinion much nicer.

- Lars


>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
> index fb26345..af0bb92 100644
> --- a/sound/soc/fsl/Kconfig
> +++ b/sound/soc/fsl/Kconfig
> @@ -9,6 +9,7 @@ config SND_SOC_FSL_SAI
>
>   config SND_SOC_FSL_SSI
>   	tristate "Synchronous Serial Interface module support"
> +	depends on m || SND_SOC_IMX_PCM_DMA != m
>   	help
>   	  Say Y if you want to add Synchronous Serial Interface (SSI)
>   	  support for the Freescale CPUs.
> @@ -18,6 +19,7 @@ config SND_SOC_FSL_SSI
>   config SND_SOC_FSL_SPDIF
>   	tristate "Sony/Philips Digital Interface module support"
>   	select REGMAP_MMIO
> +	depends on m || SND_SOC_IMX_PCM_DMA != m
>   	help
>   	  Say Y if you want to add Sony/Philips Digital Interface (SPDIF)
>   	  support for the Freescale CPUs.
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>

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

Re: [PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

[Arnd Bergmann]

On Tuesday 29 April 2014 12:57:24 Lars-Peter Clausen wrote:
> On 04/29/2014 12:37 PM, Arnd Bergmann wrote:
> > On Tuesday 29 April 2014 07:07:33 Alexander Shiyan wrote:
> >> Mon, 28 Apr 2014 23:12:14 +0200 от Arnd Bergmann <arnd@arndb.de>:
> >>> On Tuesday 29 April 2014 00:35:41 Alexander Shiyan wrote:
> >>>>
> >>>> So, I don't understand why this error happen, as well as I can not
> >>>> reproduce this...
> >>>
> >>> It's probably CONFIG_SND_SOC_IMX_PCM_DMA=m and CONFIG_SND_SOC_FSL_SSI=y then.
> >>> What is the intended behavior in this case? Should CONFIG_SND_SOC_FSL_SSI
> >>> be forced to be a module as well?
> >>
> >> Hmm, yes...
> >> I thought that I had already solved a similar problem for the earlier
> >> version of the patch ...
> >
> > How about this?
> 
> Having FSL_SSI/FSL_SPDIF, but not SND_SOC_IMX_PCM_DMA does not make sense on 
> iMX.
> So how about:
> 
>         select SND_SOC_IMX_PCM_DMA if SND_IMX_SOC
> 
> and remove all the other 'select SND_SOC_IMX_PCM_DMA' statements. That's in 
> my opinion much nicer.

Yes, makes sense. Should I do another version of the patch?

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

Re: [alsa-devel] [PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

[Lars-Peter Clausen]

On 04/29/2014 03:30 PM, Arnd Bergmann wrote:
> On Tuesday 29 April 2014 12:57:24 Lars-Peter Clausen wrote:
>> On 04/29/2014 12:37 PM, Arnd Bergmann wrote:
>>> On Tuesday 29 April 2014 07:07:33 Alexander Shiyan wrote:
>>>> Mon, 28 Apr 2014 23:12:14 +0200 от Arnd Bergmann <arnd@arndb.de>:
>>>>> On Tuesday 29 April 2014 00:35:41 Alexander Shiyan wrote:
>>>>>>
>>>>>> So, I don't understand why this error happen, as well as I can not
>>>>>> reproduce this...
>>>>>
>>>>> It's probably CONFIG_SND_SOC_IMX_PCM_DMA=m and CONFIG_SND_SOC_FSL_SSI=y then.
>>>>> What is the intended behavior in this case? Should CONFIG_SND_SOC_FSL_SSI
>>>>> be forced to be a module as well?
>>>>
>>>> Hmm, yes...
>>>> I thought that I had already solved a similar problem for the earlier
>>>> version of the patch ...
>>>
>>> How about this?
>>
>> Having FSL_SSI/FSL_SPDIF, but not SND_SOC_IMX_PCM_DMA does not make sense on
>> iMX.
>> So how about:
>>
>>          select SND_SOC_IMX_PCM_DMA if SND_IMX_SOC
>>
>> and remove all the other 'select SND_SOC_IMX_PCM_DMA' statements. That's in
>> my opinion much nicer.
>
> Yes, makes sense. Should I do another version of the patch?

Yes. The patch should also add a select for SND_SOC_IMX_PCM_FIQ as that is 
used in the same way in the fsl_ssi driver. If anybody is afraid that 
selecting both SND_SOC_IMX_PCM_FIQ and SND_SOC_IMX_PCM_DMA by default adds 
too much overhead you could add boolean sub-options that allow to 
enable/disable support individually.

- Lars


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 575 bytes --]

On Tue, Apr 29, 2014 at 04:30:43PM +0200, Lars-Peter Clausen wrote:

> Yes. The patch should also add a select for SND_SOC_IMX_PCM_FIQ as that is
> used in the same way in the fsl_ssi driver. If anybody is afraid that
> selecting both SND_SOC_IMX_PCM_FIQ and SND_SOC_IMX_PCM_DMA by default adds
> too much overhead you could add boolean sub-options that allow to
> enable/disable support individually.

Yes, the overhead should be negligable since the FIQ code is tiny.
Arnd, please CC maintainers on patches - you've sent this to my
Linaro address again and not added Liam.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

[Arnd Bergmann]

On Tuesday 29 April 2014 09:56:30 Mark Brown wrote:
> On Tue, Apr 29, 2014 at 04:30:43PM +0200, Lars-Peter Clausen wrote:
> 
> > Yes. The patch should also add a select for SND_SOC_IMX_PCM_FIQ as that is
> > used in the same way in the fsl_ssi driver. If anybody is afraid that
> > selecting both SND_SOC_IMX_PCM_FIQ and SND_SOC_IMX_PCM_DMA by default adds
> > too much overhead you could add boolean sub-options that allow to
> > enable/disable support individually.
> 
> Yes, the overhead should be negligable since the FIQ code is tiny.
> Arnd, please CC maintainers on patches - you've sent this to my
> Linaro address again and not added Liam.

I've tried yet another approach now, this should also work and
has simpler dependencies. I'll follow up with the suggested patch
and let you pick one or the other.

Sorry for using the wrong Cc list, I too the addresses out of the
patch that introduced the problem and didn't think about it.

	Arnd

8<-----------
>From e07c95b1519c2103a9b42fd12e993440dafba9d6 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 28 Apr 2014 16:28:01 +0200
Subject: [PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

Since commit 204dec93eaa "ASoC: fsl: Allow to select individual common
options", it is possible to enable SND_SOC_FSL_SSI and SND_SOC_FSL_SPDIF
manually, either as loadable modules or built-in. This unfortunately
leads to a link error if one or both of them are built-in, while
the imx-pcm-dma framework is a loadable module:

sound/built-in.o: In function `fsl_ssi_probe':
:(.text+0x51fb8): undefined reference to `imx_pcm_dma_init'
sound/built-in.o: In function `fsl_spdif_probe':
:(.text+0x52e20): undefined reference to `imx_pcm_dma_init'

This changes Kconfig to prevent this case by ensuring the imx-pcm-dma
code is built-in if the i.MX SoC support is enabled and at least
one of the two drivers is built-in.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index fb26345..5e0a58c 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -34,6 +34,7 @@ config SND_SOC_FSL_UTILS
 
 config SND_SOC_IMX_PCM_DMA
 	tristate
+	default y if SND_IMX_SOC!=n && (SND_SOC_FSL_SSI=y || SND_SOC_FSL_SPDIF=y)
 	select SND_SOC_GENERIC_DMAENGINE_PCM
 
 config SND_SOC_IMX_AUDMUX

Re: [PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

[Arnd Bergmann]

Since commit 204dec93eaa "ASoC: fsl: Allow to select individual common
options", it is possible to enable SND_SOC_FSL_SSI and SND_SOC_FSL_SPDIF
manually, either as loadable modules or built-in. This unfortunately
leads to a link error if one or both of them are built-in, while
the imx-pcm-dma framework is a loadable module:

sound/built-in.o: In function `fsl_ssi_probe':
:(.text+0x51fb8): undefined reference to `imx_pcm_dma_init'
sound/built-in.o: In function `fsl_spdif_probe':
:(.text+0x52e20): undefined reference to `imx_pcm_dma_init'

This changes Kconfig to prevent this case by using 'select' to turn
on the imx-pcm-dma code from both drivers. For consistency, we also
turn on the imx-pcm-fiq code, which is an alternative to the dma
implementation.

Note that imx-pcm-fiq is platform dependent, so we must not enable
that unless we are building a kernel for i.MX. Note also the
"if SND_IMX_SOC != n" syntax as opposed to the normal "if SND_IMX_SOC".
This is needed to avoid turning on the options as 'm' if 'SND_IMX_SOC'
is a module.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index fb26345..b2a1ade 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -9,6 +9,8 @@ config SND_SOC_FSL_SAI
 
 config SND_SOC_FSL_SSI
 	tristate "Synchronous Serial Interface module support"
+	select SND_SOC_IMX_PCM_DMA if SND_IMX_SOC != n
+	select SND_SOC_IMX_PCM_FIQ if SND_IMX_SOC != n && ARCH_MXC
 	help
 	  Say Y if you want to add Synchronous Serial Interface (SSI)
 	  support for the Freescale CPUs.
@@ -18,6 +20,8 @@ config SND_SOC_FSL_SSI
 config SND_SOC_FSL_SPDIF
 	tristate "Sony/Philips Digital Interface module support"
 	select REGMAP_MMIO
+	select SND_SOC_IMX_PCM_DMA if SND_IMX_SOC != n
+	select SND_SOC_IMX_PCM_FIQ if SND_IMX_SOC != n && ARCH_MXC
 	help
 	  Say Y if you want to add Sony/Philips Digital Interface (SPDIF)
 	  support for the Freescale CPUs.

Re: [PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 633 bytes --]

On Tue, Apr 29, 2014 at 09:36:22PM +0200, Arnd Bergmann wrote:
> Since commit 204dec93eaa "ASoC: fsl: Allow to select individual common
> options", it is possible to enable SND_SOC_FSL_SSI and SND_SOC_FSL_SPDIF
> manually, either as loadable modules or built-in. This unfortunately
> leads to a link error if one or both of them are built-in, while
> the imx-pcm-dma framework is a loadable module:

Applied, thanks.  Please don't send patches in reply to existing threads
(especially with the Re - it makes it look like ongoing discussion, I at
least often leave these discussions among people working on the platform
to grind on).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]