Skip to contentSkip to conversation

ASoC: samsung: move s3c24xx over to dmaengine

RFC PATCH In review Arnd Bergmann 3 participants 6 messages Jun 5, 2014 – Jun 12, 2014

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/2 ASoC: samsung: move s3c24xx over to dmaengine

The s3c24xx sound support keeps giving randconfig build errors, this is a new attempt to solve it by moving the driver over to use dmaengine exclusively.

The removal of the legacy DMA support and the addition of the s3c24xx_dma_filter pointer are fairly obvious here. The part I'm not completely sure about is the removal of the s3c2410_dma_ctrl(..., S3C2410_DMAOP_STARTED) and dma_data->ops->started() calls. My understanding is that these are only required for drivers that do not support cyclic transfers, which the new dma engine driver now does, so we can simply remove them. This would also fix at least one bug in the ac97 driver on newer machines, which currently gives us a NULL pointer dereference from trying to call dma_data->ops->started().

Any insights about this, or testing would be very welcome.

Cc Heiko Stuebner <[email protected]>
Cc Sangbeom Kim <[email protected]>
Cc Ben Dooks <[email protected]>
Cc Kukjin Kim <[email protected]>
Cc linux-samsung-soc at vger.kernel.org
---
 sound/soc/samsung/Kconfig       |  13 --
 sound/soc/samsung/Makefile      |   4 +-
 sound/soc/samsung/ac97.c        |  16 --
 sound/soc/samsung/dma.c         | 454 ------------------------------------
 sound/soc/samsung/dmaengine.c   |   3 +
 sound/soc/samsung/s3c24xx-i2s.c |   1 -
 6 files changed, 4 insertions(+), 487 deletions(-)
sound/soc/samsung/Kconfig +0 -13
index 7745629..32bfcf1 100644
@@ -1,25 +1,15 @@
 config SND_SOC_SAMSUNG
 	tristate "ASoC support for Samsung"
 	depends on PLAT_SAMSUNG
-	select S3C2410_DMA if ARCH_S3C24XX
 	select S3C64XX_PL080 if ARCH_S3C64XX
-	select SND_S3C_DMA if !ARCH_S3C24XX
-	select SND_S3C_DMA_LEGACY if ARCH_S3C24XX
 	select SND_SOC_GENERIC_DMAENGINE_PCM if !ARCH_S3C24XX
 	help
 	  Say Y or M if you want to add support for codecs attached to
 	  the Samsung SoCs' Audio interfaces. You will also need to
 	  select the audio interfaces to support below.
 
-config SND_S3C_DMA
-	tristate
-
-config SND_S3C_DMA_LEGACY
-	tristate
-
 config SND_S3C24XX_I2S
 	tristate
-	select S3C24XX_DMA
 
 config SND_S3C_I2SV2_SOC
 	tristate
@@ -27,7 +17,6 @@ config SND_S3C_I2SV2_SOC
 config SND_S3C2412_SOC_I2S
 	tristate
 	select SND_S3C_I2SV2_SOC
-	select S3C2410_DMA
 
 config SND_SAMSUNG_PCM
 	tristate
@@ -83,7 +72,6 @@ config SND_SOC_SAMSUNG_SMDK_WM8994
 config SND_SOC_SAMSUNG_SMDK2443_WM9710
 	tristate "SoC AC97 Audio support for SMDK2443 - WM9710"
 	depends on SND_SOC_SAMSUNG && MACH_SMDK2443
-	select S3C2410_DMA
 	select AC97_BUS
 	select SND_SOC_AC97_CODEC
 	select SND_SAMSUNG_AC97
@@ -94,7 +82,6 @@ config SND_SOC_SAMSUNG_SMDK2443_WM9710
 config SND_SOC_SAMSUNG_LN2440SBC_ALC650
 	tristate "SoC AC97 Audio support for LN2440SBC - ALC650"
 	depends on SND_SOC_SAMSUNG && ARCH_S3C24XX
-	select S3C2410_DMA
 	select AC97_BUS
 	select SND_SOC_AC97_CODEC
 	select SND_SAMSUNG_AC97
sound/soc/samsung/Makefile +1 -3
index 6d0212b..f69f490 100644
@@ -1,6 +1,5 @@
 # S3c24XX Platform Support
 snd-soc-s3c-dma-objs := dmaengine.o
-snd-soc-s3c-dma-legacy-objs := dma.o
 snd-soc-idma-objs := idma.o
 snd-soc-s3c24xx-i2s-objs := s3c24xx-i2s.o
 snd-soc-s3c2412-i2s-objs := s3c2412-i2s.o
@@ -10,8 +9,7 @@ snd-soc-samsung-spdif-objs := spdif.o
 snd-soc-pcm-objs := pcm.o
 snd-soc-i2s-objs := i2s.o
 
-obj-$(CONFIG_SND_S3C_DMA) += snd-soc-s3c-dma.o
-obj-$(CONFIG_SND_S3C_DMA_LEGACY) += snd-soc-s3c-dma-legacy.o
+obj-$(CONFIG_SND_SOC_SAMSUNG) += snd-soc-s3c-dma.o
 obj-$(CONFIG_SND_S3C24XX_I2S) += snd-soc-s3c24xx-i2s.o
 obj-$(CONFIG_SND_SAMSUNG_AC97) += snd-soc-ac97.o
 obj-$(CONFIG_SND_S3C2412_SOC_I2S) += snd-soc-s3c2412-i2s.o
sound/soc/samsung/ac97.c +0 -16
index 68d9303..a674827 100644
@@ -225,9 +225,6 @@ static int s3c_ac97_trigger(struct snd_pcm_substream *substream, int cmd,
 				struct snd_soc_dai *dai)
 {
 	u32 ac_glbctrl;
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct s3c_dma_params *dma_data =
-		snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
 
 	ac_glbctrl = readl(s3c_ac97.regs + S3C_AC97_GLBCTRL);
 	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
@@ -253,11 +250,6 @@ static int s3c_ac97_trigger(struct snd_pcm_substream *substream, int cmd,
 
 	writel(ac_glbctrl, s3c_ac97.regs + S3C_AC97_GLBCTRL);
 
-	if (!dma_data->ops)
-		dma_data->ops = samsung_dma_get_ops();
-
-	dma_data->ops->started(dma_data->channel);
-
 	return 0;
 }
 
@@ -265,9 +257,6 @@ static int s3c_ac97_mic_trigger(struct snd_pcm_substream *substream,
 				    int cmd, struct snd_soc_dai *dai)
 {
 	u32 ac_glbctrl;
-	struct snd_soc_pcm_runtime *rtd = substream->private_data;
-	struct s3c_dma_params *dma_data =
-		snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
 
 	ac_glbctrl = readl(s3c_ac97.regs + S3C_AC97_GLBCTRL);
 	ac_glbctrl &= ~S3C_AC97_GLBCTRL_MICINTM_MASK;
@@ -287,11 +276,6 @@ static int s3c_ac97_mic_trigger(struct snd_pcm_substream *substream,
 
 	writel(ac_glbctrl, s3c_ac97.regs + S3C_AC97_GLBCTRL);
 
-	if (!dma_data->ops)
-		dma_data->ops = samsung_dma_get_ops();
-
-	dma_data->ops->started(dma_data->channel);
-
 	return 0;
 }
 
sound/soc/samsung/dma.c deleted +0 -454

454 lines not on this page · raw message

sound/soc/samsung/dmaengine.c +3 -0
index a0e4e79..506f5bf 100644
@@ -17,6 +17,7 @@
 
 #include <linux/module.h>
 #include <linux/amba/pl08x.h>
+#include <linux/platform_data/dma-s3c24xx.h>
 
 #include <sound/core.h>
 #include <sound/pcm.h>
@@ -29,6 +30,8 @@
 
 #ifdef CONFIG_ARCH_S3C64XX
 #define filter_fn pl08x_filter_id
+#elif defined(CONFIG_ARCH_S3C24XX)
+#define filter_fn s3c24xx_dma_filter
 #else
 #define filter_fn NULL
 #endif
sound/soc/samsung/s3c24xx-i2s.c +0 -1
index 9aba9fb..890d241 100644
@@ -290,7 +290,6 @@ static int s3c24xx_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 		else
 			s3c24xx_snd_txctrl(1);
 
-		s3c2410_dma_ctrl(dma_data->channel, S3C2410_DMAOP_STARTED);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 	case SNDRV_PCM_TRIGGER_SUSPEND:
#1 lore raw

RFC PATCH 2/2 ASoC: samsung: remove unused DMA data

The s3c_dma_client structures and the 'ch' and 'ops' members in s3c_dma_params were only used by the legacy DMA driver and serve no function any more. This removes any reference to them.

Cc Heiko Stuebner <[email protected]>
Cc Sangbeom Kim <[email protected]>
Cc Ben Dooks <[email protected]>
Cc Kukjin Kim <[email protected]>
---
 sound/soc/samsung/ac97.c        | 15 ---------------
 sound/soc/samsung/dma.h         |  7 -------
 sound/soc/samsung/i2s.c         |  6 ------
 sound/soc/samsung/pcm.c         | 12 ------------
 sound/soc/samsung/s3c-i2s-v2.c  | 10 ----------
 sound/soc/samsung/s3c2412-i2s.c | 10 ----------
 sound/soc/samsung/s3c24xx-i2s.c | 10 ----------
 sound/soc/samsung/spdif.c       |  5 -----
 8 files changed, 75 deletions(-)
sound/soc/samsung/ac97.c +0 -15
index a674827..c96babc 100644
@@ -39,30 +39,15 @@ struct s3c_ac97_info {
 };
 static struct s3c_ac97_info s3c_ac97;
 
-static struct s3c_dma_client s3c_dma_client_out = {
-	.name = "AC97 PCMOut"
-};
-
-static struct s3c_dma_client s3c_dma_client_in = {
-	.name = "AC97 PCMIn"
-};
-
-static struct s3c_dma_client s3c_dma_client_micin = {
-	.name = "AC97 MicIn"
-};
-
 static struct s3c_dma_params s3c_ac97_pcm_out = {
-	.client		= &s3c_dma_client_out,
 	.dma_size	= 4,
 };
 
 static struct s3c_dma_params s3c_ac97_pcm_in = {
-	.client		= &s3c_dma_client_in,
 	.dma_size	= 4,
 };
 
 static struct s3c_dma_params s3c_ac97_mic_in = {
-	.client		= &s3c_dma_client_micin,
 	.dma_size	= 4,
 };
 
sound/soc/samsung/dma.h +0 -7
index 070ab0f..0e85dcf 100644
@@ -14,17 +14,10 @@
 
 #include <sound/dmaengine_pcm.h>
 
-struct s3c_dma_client {
-	char *name;
-};
-
 struct s3c_dma_params {
-	struct s3c_dma_client *client;	/* stream identifier */
 	int channel;				/* Channel ID */
 	dma_addr_t dma_addr;
 	int dma_size;			/* Size of the DMA transfer */
-	unsigned ch;
-	struct samsung_dma_ops *ops;
 	char *ch_name;
 	struct snd_dmaengine_dai_dma_data dma_data;
 };
sound/soc/samsung/i2s.c +0 -6
index 2ac76fa..6b9de74 100644
@@ -1216,11 +1216,7 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 
 	pri_dai->dma_playback.dma_addr = regs_base + I2STXD;
 	pri_dai->dma_capture.dma_addr = regs_base + I2SRXD;
-	pri_dai->dma_playback.client =
-		(struct s3c_dma_client *)&pri_dai->dma_playback;
 	pri_dai->dma_playback.ch_name = "tx";
-	pri_dai->dma_capture.client =
-		(struct s3c_dma_client *)&pri_dai->dma_capture;
 	pri_dai->dma_capture.ch_name = "rx";
 	pri_dai->dma_playback.dma_size = 4;
 	pri_dai->dma_capture.dma_size = 4;
@@ -1238,8 +1234,6 @@ static int samsung_i2s_probe(struct platform_device *pdev)
 			goto err;
 		}
 		sec_dai->dma_playback.dma_addr = regs_base + I2STXDS;
-		sec_dai->dma_playback.client =
-			(struct s3c_dma_client *)&sec_dai->dma_playback;
 		sec_dai->dma_playback.ch_name = "tx-sec";
 
 		if (!np) {
sound/soc/samsung/pcm.c +0 -12
index 4c5f97f..bac034b 100644
@@ -131,32 +131,20 @@ struct s3c_pcm_info {
 	struct s3c_dma_params	*dma_capture;
 };
 
-static struct s3c_dma_client s3c_pcm_dma_client_out = {
-	.name		= "PCM Stereo out"
-};
-
-static struct s3c_dma_client s3c_pcm_dma_client_in = {
-	.name		= "PCM Stereo in"
-};
-
 static struct s3c_dma_params s3c_pcm_stereo_out[] = {
 	[0] = {
-		.client		= &s3c_pcm_dma_client_out,
 		.dma_size	= 4,
 	},
 	[1] = {
-		.client		= &s3c_pcm_dma_client_out,
 		.dma_size	= 4,
 	},
 };
 
 static struct s3c_dma_params s3c_pcm_stereo_in[] = {
 	[0] = {
-		.client		= &s3c_pcm_dma_client_in,
 		.dma_size	= 4,
 	},
 	[1] = {
-		.client		= &s3c_pcm_dma_client_in,
 		.dma_size	= 4,
 	},
 };
sound/soc/samsung/s3c-i2s-v2.c +0 -10
index 0ff4bbe..113e5cb 100644
@@ -22,8 +22,6 @@
 #include <sound/soc.h>
 #include <sound/pcm_params.h>
 
-#include <mach/dma.h>
-
 #include "regs-i2s-v2.h"
 #include "s3c-i2s-v2.h"
 #include "dma.h"
@@ -423,14 +421,6 @@ static int s3c2412_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
 			s3c2412_snd_txctrl(i2s, 1);
 
 		local_irq_restore(irqs);
-
-		/*
-		 * Load the next buffer to DMA to meet the reqirement
-		 * of the auto reload mechanism of S3C24XX.
-		 * This call won't bother S3C64XX.
-		 */
-		s3c2410_dma_ctrl(dma_data->channel, S3C2410_DMAOP_STARTED);
-
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
sound/soc/samsung/s3c2412-i2s.c +0 -10
index 08c059b..c8d2421 100644
@@ -33,23 +33,13 @@
 #include "regs-i2s-v2.h"
 #include "s3c2412-i2s.h"
 
-static struct s3c_dma_client s3c2412_dma_client_out = {
-	.name		= "I2S PCM Stereo out"
-};
-
-static struct s3c_dma_client s3c2412_dma_client_in = {
-	.name		= "I2S PCM Stereo in"
-};
-
 static struct s3c_dma_params s3c2412_i2s_pcm_stereo_out = {
-	.client		= &s3c2412_dma_client_out,
 	.channel	= DMACH_I2S_OUT,
 	.dma_addr	= S3C2410_PA_IIS + S3C2412_IISTXD,
 	.dma_size	= 4,
 };
 
 static struct s3c_dma_params s3c2412_i2s_pcm_stereo_in = {
-	.client		= &s3c2412_dma_client_in,
 	.channel	= DMACH_I2S_IN,
 	.dma_addr	= S3C2410_PA_IIS + S3C2412_IISRXD,
 	.dma_size	= 4,
sound/soc/samsung/s3c24xx-i2s.c +0 -10
index 890d241..2d8203f 100644
@@ -31,23 +31,13 @@
 #include "dma.h"
 #include "s3c24xx-i2s.h"
 
-static struct s3c_dma_client s3c24xx_dma_client_out = {
-	.name = "I2S PCM Stereo out"
-};
-
-static struct s3c_dma_client s3c24xx_dma_client_in = {
-	.name = "I2S PCM Stereo in"
-};
-
 static struct s3c_dma_params s3c24xx_i2s_pcm_stereo_out = {
-	.client		= &s3c24xx_dma_client_out,
 	.channel	= DMACH_I2S_OUT,
 	.dma_addr	= S3C2410_PA_IIS + S3C2410_IISFIFO,
 	.dma_size	= 2,
 };
 
 static struct s3c_dma_params s3c24xx_i2s_pcm_stereo_in = {
-	.client		= &s3c24xx_dma_client_in,
 	.channel	= DMACH_I2S_IN,
 	.dma_addr	= S3C2410_PA_IIS + S3C2410_IISFIFO,
 	.dma_size	= 2,
sound/soc/samsung/spdif.c +0 -5
index d9ffc48..d7d2e20 100644
@@ -93,10 +93,6 @@ struct samsung_spdif_info {
 	struct s3c_dma_params	*dma_playback;
 };
 
-static struct s3c_dma_client spdif_dma_client_out = {
-	.name		= "S/PDIF Stereo out",
-};
-
 static struct s3c_dma_params spdif_stereo_out;
 static struct samsung_spdif_info spdif_info;
 
@@ -435,7 +431,6 @@ static int spdif_probe(struct platform_device *pdev)
 	}
 
 	spdif_stereo_out.dma_size = 2;
-	spdif_stereo_out.client = &spdif_dma_client_out;
 	spdif_stereo_out.dma_addr = mem_res->start + DATA_OUTBUF;
 	spdif_stereo_out.channel = dma_res->start;
 
#2 lore raw
In reply to Arnd Bergmann
Quoted 14 lines · Arnd Bergmann

The s3c24xx sound support keeps giving randconfig build errors, this is a new attempt to solve it by moving the driver over to use dmaengine exclusively.

The removal of the legacy DMA support and the addition of the s3c24xx_dma_filter pointer are fairly obvious here. The part I'm not completely sure about is the removal of the s3c2410_dma_ctrl(..., S3C2410_DMAOP_STARTED) and dma_data->ops->started() calls. My understanding is that these are only required for drivers that do not support cyclic transfers, which the new dma engine driver now does, so we can simply remove them. This would also fix at least one bug in the ac97 driver on newer machines, which currently gives us a NULL pointer dereference from trying to call dma_data->ops->started().

Quoted 2 lines

Any insights about this, or testing would be very welcome.

There is already a very similar patch for this, see:
http://mailman.alsa-project.org/pipermail/alsa-devel/2014-June/077327.html

I think that one is scheduled to be merged.

- Lars

maintainer
#3 lore raw
In reply to Arnd Bergmann
Quoted 3 lines · Arnd Bergmann

The s3c24xx sound support keeps giving randconfig build errors, this is a new attempt to solve it by moving the driver over to use dmaengine exclusively.

We already have a patch pending for this, it's just waiting until after the merge window to avoid bisection breakage since it depends on dmaengine changes.

#4 lore raw
Quoted 8 lines · On Thursday 05 June 2014 14:36:06 Mark Brown wrote:
Quoted 3 lines · Arnd Bergmann

The s3c24xx sound support keeps giving randconfig build errors, this is a new attempt to solve it by moving the driver over to use dmaengine exclusively.

We already have a patch pending for this, it's just waiting until after the merge window to avoid bisection breakage since it depends on dmaengine changes.

Ok, very good.

Arnd

#5 lore raw
Quoted 11 lines · On Thursday 05 June 2014 16:04:03 Arnd Bergmann wrote:
Quoted 8 lines · On Thursday 05 June 2014 14:36:06 Mark Brown wrote:
Quoted 3 lines · Arnd Bergmann

The s3c24xx sound support keeps giving randconfig build errors, this is a new attempt to solve it by moving the driver over to use dmaengine exclusively.

We already have a patch pending for this, it's just waiting until after the merge window to avoid bisection breakage since it depends on dmaengine changes.

Ok, very good.

Actually, I think it would still be good to do something about the samsung/ac97.c file, which is the only reason we are keeping the SAMSUNG_DMADEV code around that is part of the dependency chain Russell posted, and it's definitely broken on anything but s3c24xx.

If we do something like the patch below, we can just disable SAMSUNG_DMADEV already, which simplifies the dependency chains a lot. We can then remove that code entirely in 3.17.

Note that for !CONFIG_S3C24XX_DMA, the code is currently broken, as samsung_dma_get_ops()->started is a NULL pointer for dmadev.

Arnd

sound/soc/samsung/ac97.c +10 -8
index 68d9303..1db1028 100644
@@ -225,9 +225,11 @@ static int s3c_ac97_trigger(struct snd_pcm_substream *substream, int cmd,
 				struct snd_soc_dai *dai)
 {
 	u32 ac_glbctrl;
+#ifdef CONFIG_S3C24XX_DMA
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct s3c_dma_params *dma_data =
 		snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
+#endif
 
 	ac_glbctrl = readl(s3c_ac97.regs + S3C_AC97_GLBCTRL);
 	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
@@ -253,10 +255,9 @@ static int s3c_ac97_trigger(struct snd_pcm_substream *substream, int cmd,
 
 	writel(ac_glbctrl, s3c_ac97.regs + S3C_AC97_GLBCTRL);
 
-	if (!dma_data->ops)
-		dma_data->ops = samsung_dma_get_ops();
-
-	dma_data->ops->started(dma_data->channel);
+#ifdef CONFIG_S3C24XX_DMA
+	s3c2410_dma_ctrl(dma_data->channel, S3C2410_DMAOP_STARTED);
+#endif
 
 	return 0;
 }
@@ -265,9 +266,11 @@ static int s3c_ac97_mic_trigger(struct snd_pcm_substream *substream,
 				    int cmd, struct snd_soc_dai *dai)
 {
 	u32 ac_glbctrl;
+#ifdef CONFIG_S3C24XX_DMA
 	struct snd_soc_pcm_runtime *rtd = substream->private_data;
 	struct s3c_dma_params *dma_data =
 		snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
+#endif
 
 	ac_glbctrl = readl(s3c_ac97.regs + S3C_AC97_GLBCTRL);
 	ac_glbctrl &= ~S3C_AC97_GLBCTRL_MICINTM_MASK;
@@ -287,10 +290,9 @@ static int s3c_ac97_mic_trigger(struct snd_pcm_substream *substream,
 
 	writel(ac_glbctrl, s3c_ac97.regs + S3C_AC97_GLBCTRL);
 
-	if (!dma_data->ops)
-		dma_data->ops = samsung_dma_get_ops();
-
-	dma_data->ops->started(dma_data->channel);
+#ifdef CONFIG_S3C24XX_DMA
+	s3c2410_dma_ctrl(dma_data->channel, S3C2410_DMAOP_STARTED);
+#endif
 
 	return 0;
 }