Re: [alsa-devel] [PATCH] ASoC: fsl: select SND_SOC_IMX_PCM_DMA where needed
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.
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
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_SOCand 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
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_SOCand 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
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.
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
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.
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).
participants (3)
-
Arnd Bergmann -
Lars-Peter Clausen -
Mark Brown