[alsa-devel] [PATCH V2 1/3] ASoC: restructure dmaengine_pcm_request_chan_of()

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]; }
/**

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);

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

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.

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,

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.

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

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.
participants (3)
-
Lars-Peter Clausen
-
Mark Brown
-
Stephen Warren