alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 1/3] ASoC: restructure dmaengine_pcm_request_chan_of()
@ 2013-12-03 21:26 Stephen Warren
  2013-12-03 21:26 ` [PATCH V2 2/3] ASoC: don't leak on error in snd_dmaengine_pcm_register Stephen Warren
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Stephen Warren @ 2013-12-03 21:26 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Lars-Peter Clausen; +Cc: alsa-devel, Stephen Warren

From: Stephen Warren <swarren@nvidia.com>

Restructure the internals of dmaengine_pcm_request_chan_of() as a loop
over all channels to be allocated. This makes it easier to add logic
that applies to all allocated channels, without having to duplicate that
logic in each of the half-duplex/full-duplex paths.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2 (new as a separate patch):
* Re-ordered patches, so that patch 3/3 in this series could be available
  in the ASoC tree without having to wait for the patches to the DMA tree
  the implement deferred probe during channel allocation.

This whole series is a dependency for some patches that will go through
the Tegra tree. If it could be applied to a topic branch in the ASoC tree
that would be great. the overall dependency structure will be:

a) This series.
   Can be applied on top of e.g. v3.13-rc1.
b) Patches to the DMA tree, which implement deferred probe.
   Can be applied on top of e.g. v3.13-rc1.
c) ASoC modifications to support deferred probe during DMA ch allocation
   This will depend on both (a) and (b).
d) Tegra modifications that depend on (c).
---
 sound/soc/soc-generic-dmaengine-pcm.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
index cbc9c96ce1f4..88b011b80058 100644
--- a/sound/soc/soc-generic-dmaengine-pcm.c
+++ b/sound/soc/soc-generic-dmaengine-pcm.c
@@ -288,21 +288,26 @@ static void dmaengine_pcm_request_chan_of(struct dmaengine_pcm *pcm,
 	struct device *dev)
 {
 	unsigned int i;
+	const char *name;
 
 	if ((pcm->flags & (SND_DMAENGINE_PCM_FLAG_NO_DT |
 			   SND_DMAENGINE_PCM_FLAG_CUSTOM_CHANNEL_NAME)) ||
 	    !dev->of_node)
 		return;
 
-	if (pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX) {
-		pcm->chan[0] = dma_request_slave_channel(dev, "rx-tx");
-		pcm->chan[1] = pcm->chan[0];
-	} else {
-		for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_CAPTURE; i++) {
-			pcm->chan[i] = dma_request_slave_channel(dev,
-					dmaengine_pcm_dma_channel_names[i]);
-		}
+	for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_CAPTURE;
+	     i++) {
+		if (pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX)
+			name = "rx-tx";
+		else
+			name = dmaengine_pcm_dma_channel_names[i];
+		pcm->chan[i] = dma_request_slave_channel(dev, name);
+		if (pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX)
+			break;
 	}
+
+	if (pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX)
+		pcm->chan[1] = pcm->chan[0];
 }
 
 /**
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH V2 2/3] ASoC: don't leak on error in snd_dmaengine_pcm_register
  2013-12-03 21:26 [PATCH V2 1/3] ASoC: restructure dmaengine_pcm_request_chan_of() Stephen Warren
@ 2013-12-03 21:26 ` Stephen Warren
  2013-12-06  9:30   ` Lars-Peter Clausen
  2013-12-09 18:43   ` Mark Brown
  2013-12-03 21:26 ` [PATCH V2 3/3] ASoC: dmaengine: add custom DMA config to snd_dmaengine_pcm_config Stephen Warren
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 8+ messages in thread
From: Stephen Warren @ 2013-12-03 21:26 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Lars-Peter Clausen; +Cc: alsa-devel, Stephen Warren

From: Stephen Warren <swarren@nvidia.com>

If snd_dmaengine_pcm_register()'s call to snd_soc_add_platform() fails,
all objects allocated during registration are leaked. Fix this by adding
error-handling code.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
v2 (new as a separate patch):
* Re-ordered patches, so that patch 3/3 in this series could be available
  in the ASoC tree without having to wait for the patches to the DMA tree
  the implement deferred probe during channel allocation.
---
 sound/soc/soc-generic-dmaengine-pcm.c | 38 +++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
index 88b011b80058..a66bb03b64cd 100644
--- a/sound/soc/soc-generic-dmaengine-pcm.c
+++ b/sound/soc/soc-generic-dmaengine-pcm.c
@@ -310,6 +310,20 @@ static void dmaengine_pcm_request_chan_of(struct dmaengine_pcm *pcm,
 		pcm->chan[1] = pcm->chan[0];
 }
 
+static void dmaengine_pcm_release_chan(struct dmaengine_pcm *pcm)
+{
+	unsigned int i;
+
+	for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_CAPTURE;
+	     i++) {
+		if (!pcm->chan[i])
+			continue;
+		dma_release_channel(pcm->chan[i]);
+		if (pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX)
+			break;
+	}
+}
+
 /**
  * snd_dmaengine_pcm_register - Register a dmaengine based PCM device
  * @dev: The parent device for the PCM device
@@ -320,6 +334,7 @@ int snd_dmaengine_pcm_register(struct device *dev,
 	const struct snd_dmaengine_pcm_config *config, unsigned int flags)
 {
 	struct dmaengine_pcm *pcm;
+	int ret;
 
 	pcm = kzalloc(sizeof(*pcm), GFP_KERNEL);
 	if (!pcm)
@@ -331,11 +346,20 @@ int snd_dmaengine_pcm_register(struct device *dev,
 	dmaengine_pcm_request_chan_of(pcm, dev);
 
 	if (flags & SND_DMAENGINE_PCM_FLAG_NO_RESIDUE)
-		return snd_soc_add_platform(dev, &pcm->platform,
+		ret = snd_soc_add_platform(dev, &pcm->platform,
 				&dmaengine_no_residue_pcm_platform);
 	else
-		return snd_soc_add_platform(dev, &pcm->platform,
+		ret = snd_soc_add_platform(dev, &pcm->platform,
 				&dmaengine_pcm_platform);
+	if (ret)
+		goto err_free_dma;
+
+	return 0;
+
+err_free_dma:
+	dmaengine_pcm_release_chan(pcm);
+	kfree(pcm);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_register);
 
@@ -350,7 +374,6 @@ void snd_dmaengine_pcm_unregister(struct device *dev)
 {
 	struct snd_soc_platform *platform;
 	struct dmaengine_pcm *pcm;
-	unsigned int i;
 
 	platform = snd_soc_lookup_platform(dev);
 	if (!platform)
@@ -358,15 +381,8 @@ void snd_dmaengine_pcm_unregister(struct device *dev)
 
 	pcm = soc_platform_to_pcm(platform);
 
-	for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_CAPTURE; i++) {
-		if (pcm->chan[i]) {
-			dma_release_channel(pcm->chan[i]);
-			if (pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX)
-				break;
-		}
-	}
-
 	snd_soc_remove_platform(platform);
+	dmaengine_pcm_release_chan(pcm);
 	kfree(pcm);
 }
 EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_unregister);
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH V2 3/3] ASoC: dmaengine: add custom DMA config to snd_dmaengine_pcm_config
  2013-12-03 21:26 [PATCH V2 1/3] ASoC: restructure dmaengine_pcm_request_chan_of() Stephen Warren
  2013-12-03 21:26 ` [PATCH V2 2/3] ASoC: don't leak on error in snd_dmaengine_pcm_register Stephen Warren
@ 2013-12-03 21:26 ` Stephen Warren
  2013-12-09 18:45   ` Mark Brown
  2013-12-06  9:30 ` [PATCH V2 1/3] ASoC: restructure dmaengine_pcm_request_chan_of() Lars-Peter Clausen
  2013-12-09 18:43 ` Mark Brown
  3 siblings, 1 reply; 8+ messages in thread
From: Stephen Warren @ 2013-12-03 21:26 UTC (permalink / raw)
  To: Mark Brown, Liam Girdwood, Lars-Peter Clausen; +Cc: alsa-devel, Stephen Warren

From: Stephen Warren <swarren@nvidia.com>

Add fields to struct snd_dmaengine_pcm_config to allow custom:

- DMA channel names.

  This is useful when the default "tx" and "rx" channel names don't
  apply, for example if a HW module supports multiple channels, each
  having different DMA channel names. This is the case with the FIFOs
  in Tegra's AHUB. This new facility can replace
  SND_DMAENGINE_PCM_FLAG_CUSTOM_CHANNEL_NAME.

- DMA device

  This allows requesting DMA channels for a device other than the device
  which is registering the "PCM" driver. This is quite unusual, but is
  currently useful on Tegra. In much HW, and in Tegra20, each DAI HW
  module contains its own FIFOs which DMA writes to. However, in Tegra30,
  the DMA FIFOs were split out AHUB HW module, which then routes the data
  through a cross-bar, and into the DAI HW modules. However, the current
  ASoC driver structure does not expose this detail, and acts as if the
  FIFOs are still part of the DAI HW modules. Consequently, the "PCM"
  driver is registered with the DAI HW module, yet the DMA channels must
  be looked up in the AHUB HW module's device tree node. This new config
  field allows that to happen. Eventually, the Tegra drivers will be
  reworked to fully expose the AHUB, and this config field can be
  removed.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Lars-Peter Clausen <lars@metafoo.de>
---
v2:
* Re-ordered patches, so that patch 3/3 in this series could be available
  in the ASoC tree without having to wait for the patches to the DMA tree
  the implement deferred probe during channel allocation.
* dev_warn() in dmaengine_pcm_request_chan_of() if config->dev is set;
  drivers generally shouldn't need this feature, and should be re-
  structured if they do.
---
 include/sound/dmaengine_pcm.h         |  6 ++++++
 sound/soc/soc-generic-dmaengine-pcm.c | 18 ++++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
index 15017311f2e9..87e9d481d7b6 100644
--- a/include/sound/dmaengine_pcm.h
+++ b/include/sound/dmaengine_pcm.h
@@ -114,6 +114,10 @@ void snd_dmaengine_pcm_set_config_from_dai_data(
  * @compat_filter_fn: Will be used as the filter function when requesting a
  *  channel for platforms which do not use devicetree. The filter parameter
  *  will be the DAI's DMA data.
+ * @dma_dev: If set, request DMA channel on this device rather than the DAI
+ *  device.
+ * @chan_names: If set, these custom DMA channel names will be requested at
+ *  registration time.
  * @pcm_hardware: snd_pcm_hardware struct to be used for the PCM.
  * @prealloc_buffer_size: Size of the preallocated audio buffer.
  *
@@ -130,6 +134,8 @@ struct snd_dmaengine_pcm_config {
 			struct snd_soc_pcm_runtime *rtd,
 			struct snd_pcm_substream *substream);
 	dma_filter_fn compat_filter_fn;
+	struct device *dma_dev;
+	const char *chan_names[SNDRV_PCM_STREAM_LAST + 1];
 
 	const struct snd_pcm_hardware *pcm_hardware;
 	unsigned int prealloc_buffer_size;
diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
index a66bb03b64cd..ac16259490ee 100644
--- a/sound/soc/soc-generic-dmaengine-pcm.c
+++ b/sound/soc/soc-generic-dmaengine-pcm.c
@@ -285,7 +285,7 @@ static const char * const dmaengine_pcm_dma_channel_names[] = {
 };
 
 static void dmaengine_pcm_request_chan_of(struct dmaengine_pcm *pcm,
-	struct device *dev)
+	struct device *dev, const struct snd_dmaengine_pcm_config *config)
 {
 	unsigned int i;
 	const char *name;
@@ -295,12 +295,26 @@ static void dmaengine_pcm_request_chan_of(struct dmaengine_pcm *pcm,
 	    !dev->of_node)
 		return;
 
+	if (config->dma_dev) {
+		/*
+		 * If this warning is seen, it probably means that your Linux
+		 * device structure does not match your HW device structure.
+		 * It would be best to refactor the Linux device structure to
+		 * correctly match the HW structure.
+		 */
+		dev_warn(dev, "DMA channels sourced from device %s",
+			 dev_name(config->dma_dev));
+		dev = config->dma_dev;
+	}
+
 	for (i = SNDRV_PCM_STREAM_PLAYBACK; i <= SNDRV_PCM_STREAM_CAPTURE;
 	     i++) {
 		if (pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX)
 			name = "rx-tx";
 		else
 			name = dmaengine_pcm_dma_channel_names[i];
+		if (config->chan_names[i])
+			name = config->chan_names[i];
 		pcm->chan[i] = dma_request_slave_channel(dev, name);
 		if (pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX)
 			break;
@@ -343,7 +357,7 @@ int snd_dmaengine_pcm_register(struct device *dev,
 	pcm->config = config;
 	pcm->flags = flags;
 
-	dmaengine_pcm_request_chan_of(pcm, dev);
+	dmaengine_pcm_request_chan_of(pcm, dev, config);
 
 	if (flags & SND_DMAENGINE_PCM_FLAG_NO_RESIDUE)
 		ret = snd_soc_add_platform(dev, &pcm->platform,
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH V2 1/3] ASoC: restructure dmaengine_pcm_request_chan_of()
  2013-12-03 21:26 [PATCH V2 1/3] ASoC: restructure dmaengine_pcm_request_chan_of() Stephen Warren
  2013-12-03 21:26 ` [PATCH V2 2/3] ASoC: don't leak on error in snd_dmaengine_pcm_register Stephen Warren
  2013-12-03 21:26 ` [PATCH V2 3/3] ASoC: dmaengine: add custom DMA config to snd_dmaengine_pcm_config Stephen Warren
@ 2013-12-06  9:30 ` Lars-Peter Clausen
  2013-12-09 18:43 ` Mark Brown
  3 siblings, 0 replies; 8+ messages in thread
From: Lars-Peter Clausen @ 2013-12-06  9:30 UTC (permalink / raw)
  To: Stephen Warren; +Cc: alsa-devel, Mark Brown, Stephen Warren, Liam Girdwood

On 12/03/2013 10:26 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Restructure the internals of dmaengine_pcm_request_chan_of() as a loop
> over all channels to be allocated. This makes it easier to add logic
> that applies to all allocated channels, without having to duplicate that
> logic in each of the half-duplex/full-duplex paths.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Looks good, makes sense.

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2 2/3] ASoC: don't leak on error in snd_dmaengine_pcm_register
  2013-12-03 21:26 ` [PATCH V2 2/3] ASoC: don't leak on error in snd_dmaengine_pcm_register Stephen Warren
@ 2013-12-06  9:30   ` Lars-Peter Clausen
  2013-12-09 18:43   ` Mark Brown
  1 sibling, 0 replies; 8+ messages in thread
From: Lars-Peter Clausen @ 2013-12-06  9:30 UTC (permalink / raw)
  To: Stephen Warren; +Cc: alsa-devel, Mark Brown, Stephen Warren, Liam Girdwood

On 12/03/2013 10:26 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> If snd_dmaengine_pcm_register()'s call to snd_soc_add_platform() fails,
> all objects allocated during registration are leaked. Fix this by adding
> error-handling code.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Yep, looks good. Thanks. This one should probably go in as a fix.

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2 1/3] ASoC: restructure dmaengine_pcm_request_chan_of()
  2013-12-03 21:26 [PATCH V2 1/3] ASoC: restructure dmaengine_pcm_request_chan_of() Stephen Warren
                   ` (2 preceding siblings ...)
  2013-12-06  9:30 ` [PATCH V2 1/3] ASoC: restructure dmaengine_pcm_request_chan_of() Lars-Peter Clausen
@ 2013-12-09 18:43 ` Mark Brown
  3 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2013-12-09 18:43 UTC (permalink / raw)
  To: Stephen Warren
  Cc: alsa-devel, Lars-Peter Clausen, Stephen Warren, Liam Girdwood


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

On Tue, Dec 03, 2013 at 02:26:32PM -0700, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Restructure the internals of dmaengine_pcm_request_chan_of() as a loop
> over all channels to be allocated. This makes it easier to add logic

Applied, thanks.

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

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2 2/3] ASoC: don't leak on error in snd_dmaengine_pcm_register
  2013-12-03 21:26 ` [PATCH V2 2/3] ASoC: don't leak on error in snd_dmaengine_pcm_register Stephen Warren
  2013-12-06  9:30   ` Lars-Peter Clausen
@ 2013-12-09 18:43   ` Mark Brown
  1 sibling, 0 replies; 8+ messages in thread
From: Mark Brown @ 2013-12-09 18:43 UTC (permalink / raw)
  To: Stephen Warren
  Cc: alsa-devel, Lars-Peter Clausen, Stephen Warren, Liam Girdwood


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

On Tue, Dec 03, 2013 at 02:26:33PM -0700, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> If snd_dmaengine_pcm_register()'s call to snd_soc_add_platform() fails,
> all objects allocated during registration are leaked. Fix this by adding
> error-handling code.

Applied, thanks.

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

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V2 3/3] ASoC: dmaengine: add custom DMA config to snd_dmaengine_pcm_config
  2013-12-03 21:26 ` [PATCH V2 3/3] ASoC: dmaengine: add custom DMA config to snd_dmaengine_pcm_config Stephen Warren
@ 2013-12-09 18:45   ` Mark Brown
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Brown @ 2013-12-09 18:45 UTC (permalink / raw)
  To: Stephen Warren
  Cc: alsa-devel, Lars-Peter Clausen, Stephen Warren, Liam Girdwood


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

On Tue, Dec 03, 2013 at 02:26:34PM -0700, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Add fields to struct snd_dmaengine_pcm_config to allow custom:

Applied, thanks.

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

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-12-09 18:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-03 21:26 [PATCH V2 1/3] ASoC: restructure dmaengine_pcm_request_chan_of() Stephen Warren
2013-12-03 21:26 ` [PATCH V2 2/3] ASoC: don't leak on error in snd_dmaengine_pcm_register Stephen Warren
2013-12-06  9:30   ` Lars-Peter Clausen
2013-12-09 18:43   ` Mark Brown
2013-12-03 21:26 ` [PATCH V2 3/3] ASoC: dmaengine: add custom DMA config to snd_dmaengine_pcm_config Stephen Warren
2013-12-09 18:45   ` Mark Brown
2013-12-06  9:30 ` [PATCH V2 1/3] ASoC: restructure dmaengine_pcm_request_chan_of() Lars-Peter Clausen
2013-12-09 18:43 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).