[alsa-devel] [PATCH 00/31] ARM: tegra: use common reset and DMA bindings
From: Stephen Warren swarren@nvidia.com
This series implements a common reset framework driver for Tegra, and updates all relevant Tegra drivers to use it. It also removes the custom DMA bindings and replaced them with the standard DMA DT bindings.
Historically, the Tegra clock driver has exported a custom API for module reset. This series removes that API, and transitions DT and drivers to the new reset framework.
The custom API used a "struct clk" to identify which module to reset, and consequently some DT bindings and drivers required clocks to be provided where they really needed just a reset identifier instead. Due to this known deficiency, I have always considered most Tegra bindings to be unstable. This series removes this excuse for instability, although I still consider some Tegra bindings unstable due to the need to convert to the common DMA bindings.
Historically, Tegra DMA channels have been represented in DT using a custom nvidia,dma-request-selector property. Now that standard DMA DT bindings exist, convert all Tegra bindings, DTs, and drivers to use the standard instead.
This series makes a DT-ABI-incompatible change to: - Require reset specifiers in DT where relevant. - Require standard DMA specifiers. - Remove clock specifiers from DT where they were only needed for reset. - Remove legacy DMA specifier properties.
I anticipate merging this whole series into the Tegra and arm-soc trees as its own branch, due to internal dependencies. This branch will be stable and can then be merged into any other subsystem trees should any conflicts arise.
This series depends on Peter's Tegra clock driver rework, available at git://nv-tegra.nvidia.com/user/pdeschrijver/linux tegra-clk-tegra124-0 (or whatever version of that gets included in 3.14)
Cc: ac100@lists.launchpad.net Cc: Alan Stern stern@rowland.harvard.edu Cc: alsa-devel@alsa-project.org Cc: Bjorn Helgaas bhelgaas@google.com Cc: Dan Williams dan.j.williams@intel.com Cc: David Airlie airlied@linux.ie Cc: devel@driverdev.osuosl.org Cc: devicetree@vger.kernel.org Cc: Dmitry Torokhov dmitry.torokhov@gmail.com Cc: Dmitry Torokhov dtor@mail.ru Cc: dri-devel@lists.freedesktop.org Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: Ian Campbell ijc+devicetree@hellion.org.uk Cc: Julian Andres Klode jak@jak-linux.org Cc: Liam Girdwood lgirdwood@gmail.com Cc: linux-arm-kernel@lists.infradead.org Cc: linux-i2c@vger.kernel.org Cc: linux-input@vger.kernel.org Cc: linux-pci@vger.kernel.org Cc: linux-serial@vger.kernel.org Cc: linux-spi@vger.kernel.org Cc: linux-tegra@vger.kernel.org Cc: linux-usb@vger.kernel.org Cc: Marc Dietrich marvin24@gmx.de Cc: Mark Brown broonie@kernel.org Cc: Mark Rutland mark.rutland@arm.com Cc: Mike Turquette mturquette@linaro.org Cc: Pawel Moll pawel.moll@arm.com Cc: pdeschrijver@nvidia.com Cc: Rob Herring rob.herring@calxeda.com Cc: Terje Bergström tbergstrom@nvidia.com Cc: treding@nvidia.com Cc: Wolfram Sang wsa@the-dreams.de
Stephen Warren (31): ARM: tegra: add missing clock documentation to DT bindings ARM: tegra: document reset properties in DT bindings ARM: tegra: document use of standard DMA DT bindings ARM: tegra: update DT files to add reset properties ARM: tegra: update DT files to add DMA properties ARM: tegra: select the reset framework clk: tegra: implement a reset driver pci: tegra: use reset framework drm/tegra: use reset framework ARM: tegra: pass reset to tegra_powergate_sequence_power_up() dma: add channel request API that supports deferred probe dma: tegra: use reset framework dma: tegra: register as an OF DMA controller ASoC: dmaengine: support deferred probe for DMA channels ASoC: dmaengine: add custom DMA config to snd_dmaengine_pcm_config ASoC: tegra: use reset framework ASoC: tegra: call pm_runtime APIs around register accesses ASoC: tegra: allocate AHUB FIFO during probe() not startup() ASoC: tegra: convert to standard DMA DT bindings i2c: tegra: use reset framework staging: nvec: use reset framework spi: tegra: use reset framework spi: tegra: convert to standard DMA DT bindings serial: tegra: use reset framework serial: tegra: convert to standard DMA DT bindings Input: tegra-kbc - use reset framework USB: EHCI: tegra: use reset framework ARM: tegra: remove legacy clock entries from DT ARM: tegra: remove legacy DMA entries from DT clk: tegra: remove legacy reset APIs clk: tegra: remove bogus PCIE_XCLK
.../bindings/arm/tegra/nvidia,tegra20-pmc.txt | 1 + .../bindings/clock/nvidia,tegra114-car.txt | 4 + .../bindings/clock/nvidia,tegra124-car.txt | 4 + .../bindings/clock/nvidia,tegra20-car.txt | 4 + .../bindings/clock/nvidia,tegra30-car.txt | 4 + .../devicetree/bindings/dma/tegra20-apbdma.txt | 9 ++ .../bindings/gpu/nvidia,tegra20-host1x.txt | 124 +++++++++++++++ .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt | 27 +++- .../bindings/input/nvidia,tegra20-kbc.txt | 9 ++ .../bindings/mmc/nvidia,tegra20-sdhci.txt | 9 ++ .../devicetree/bindings/nvec/nvidia,nvec.txt | 12 ++ .../bindings/pci/nvidia,tegra20-pcie.txt | 28 ++-- .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt | 9 ++ .../devicetree/bindings/rtc/nvidia,tegra20-rtc.txt | 3 + .../bindings/serial/nvidia,tegra20-hsuart.txt | 19 ++- .../bindings/sound/nvidia,tegra-audio-alc5632.txt | 7 +- .../bindings/sound/nvidia,tegra-audio-rt5640.txt | 7 +- .../bindings/sound/nvidia,tegra-audio-wm8753.txt | 7 +- .../bindings/sound/nvidia,tegra-audio-wm8903.txt | 7 +- .../bindings/sound/nvidia,tegra-audio-wm9712.txt | 7 +- .../bindings/sound/nvidia,tegra20-ac97.txt | 20 ++- .../bindings/sound/nvidia,tegra20-i2s.txt | 19 ++- .../bindings/sound/nvidia,tegra30-ahub.txt | 54 +++++-- .../bindings/sound/nvidia,tegra30-i2s.txt | 11 +- .../bindings/spi/nvidia,tegra114-spi.txt | 24 ++- .../bindings/spi/nvidia,tegra20-sflash.txt | 20 ++- .../bindings/spi/nvidia,tegra20-slink.txt | 20 ++- .../bindings/timer/nvidia,tegra20-timer.txt | 3 + .../bindings/timer/nvidia,tegra30-timer.txt | 3 + .../bindings/usb/nvidia,tegra20-ehci.txt | 7 +- arch/arm/boot/dts/tegra114.dtsi | 142 ++++++++++++++--- arch/arm/boot/dts/tegra20-paz00.dts | 2 + arch/arm/boot/dts/tegra20.dtsi | 132 ++++++++++++++-- arch/arm/boot/dts/tegra30.dtsi | 171 +++++++++++++++++---- arch/arm/mach-tegra/Kconfig | 2 + arch/arm/mach-tegra/powergate.c | 8 +- drivers/clk/tegra/clk-periph-gate.c | 22 --- drivers/clk/tegra/clk-periph.c | 40 ----- drivers/clk/tegra/clk-tegra114.c | 3 +- drivers/clk/tegra/clk-tegra124.c | 2 +- drivers/clk/tegra/clk-tegra20.c | 9 +- drivers/clk/tegra/clk-tegra30.c | 10 +- drivers/clk/tegra/clk.c | 55 ++++++- drivers/clk/tegra/clk.h | 3 +- drivers/dma/acpi-dma.c | 12 +- drivers/dma/dmaengine.c | 44 +++++- drivers/dma/of-dma.c | 12 +- drivers/dma/tegra20-apb-dma.c | 49 +++++- drivers/gpu/drm/tegra/Kconfig | 1 + drivers/gpu/drm/tegra/dc.c | 9 +- drivers/gpu/drm/tegra/drm.h | 3 + drivers/gpu/drm/tegra/gr3d.c | 22 ++- drivers/gpu/drm/tegra/hdmi.c | 14 +- drivers/i2c/busses/i2c-tegra.c | 13 +- drivers/input/keyboard/tegra-kbc.c | 13 +- drivers/pci/host/pci-tegra.c | 52 +++++-- drivers/spi/Kconfig | 3 + drivers/spi/spi-tegra114.c | 66 ++++---- drivers/spi/spi-tegra20-sflash.c | 18 ++- drivers/spi/spi-tegra20-slink.c | 66 ++++---- drivers/staging/nvec/nvec.c | 11 +- drivers/staging/nvec/nvec.h | 5 +- drivers/tty/serial/serial-tegra.c | 86 +++++------ drivers/usb/host/ehci-tegra.c | 14 +- include/dt-bindings/clock/tegra20-car.h | 2 +- include/dt-bindings/clock/tegra30-car.h | 2 +- include/linux/clk/tegra.h | 7 - include/linux/dmaengine.h | 7 + include/linux/of_dma.h | 9 +- include/linux/tegra-powergate.h | 4 +- include/sound/dmaengine_pcm.h | 6 + sound/soc/soc-generic-dmaengine-pcm.c | 82 +++++++--- sound/soc/tegra/Kconfig | 2 + sound/soc/tegra/tegra20_ac97.c | 11 -- sound/soc/tegra/tegra20_i2s.c | 20 +-- sound/soc/tegra/tegra30_ahub.c | 125 +++++++++------ sound/soc/tegra/tegra30_ahub.h | 11 +- sound/soc/tegra/tegra30_i2s.c | 97 ++++++------ sound/soc/tegra/tegra30_i2s.h | 3 + sound/soc/tegra/tegra_pcm.c | 17 +- sound/soc/tegra/tegra_pcm.h | 5 + 81 files changed, 1448 insertions(+), 558 deletions(-)
From: Stephen Warren swarren@nvidia.com
Enhance dmaengine_pcm_request_chan_of() to support deferred probe for DMA channels, by using the new dma_request_slave_channel_or_err() API. This prevents snd_dmaengine_pcm_register() from succeeding without acquiring DMA channels due to the relevant DMA controller not yet being registered.
Cc: treding@nvidia.com Cc: linux-tegra@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: alsa-devel@alsa-project.org Signed-off-by: Stephen Warren swarren@nvidia.com --- This patch is part of a series with strong internal depdendencies. I'm looking for an ack so that I can take the entire series through the Tegra and arm-soc trees. The series will be part of a stable branch that can be merged into other subsystems if needed to avoid/resolve dependencies. --- sound/soc/soc-generic-dmaengine-pcm.c | 77 +++++++++++++++++++++++++---------- 1 file changed, 55 insertions(+), 22 deletions(-)
diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c index cbc9c96ce1f4..f7e65e1552ef 100644 --- a/sound/soc/soc-generic-dmaengine-pcm.c +++ b/sound/soc/soc-generic-dmaengine-pcm.c @@ -284,24 +284,52 @@ static const char * const dmaengine_pcm_dma_channel_names[] = { [SNDRV_PCM_STREAM_CAPTURE] = "rx", };
-static void dmaengine_pcm_request_chan_of(struct dmaengine_pcm *pcm, - struct device *dev) +static int dmaengine_pcm_request_chan_of(struct dmaengine_pcm *pcm, + struct device *dev) { unsigned int i; + const char *name; + struct dma_chan *chan;
if ((pcm->flags & (SND_DMAENGINE_PCM_FLAG_NO_DT | SND_DMAENGINE_PCM_FLAG_CUSTOM_CHANNEL_NAME)) || !dev->of_node) - return; + return 0; + + 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]; + chan = dma_request_slave_channel_or_err(dev, name); + if (IS_ERR(chan)) { + if (PTR_ERR(pcm->chan[i]) == -EPROBE_DEFER) + return -EPROBE_DEFER; + pcm->chan[i] = NULL; + } else + pcm->chan[i] = chan; + if (pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX) + break; + }
- if (pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX) { - pcm->chan[0] = dma_request_slave_channel(dev, "rx-tx"); + if (pcm->flags & SND_DMAENGINE_PCM_FLAG_HALF_DUPLEX) 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]); - } + + return 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; } }
@@ -315,6 +343,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) @@ -323,14 +352,26 @@ int snd_dmaengine_pcm_register(struct device *dev, pcm->config = config; pcm->flags = flags;
- dmaengine_pcm_request_chan_of(pcm, dev); + ret = dmaengine_pcm_request_chan_of(pcm, dev); + if (ret) + goto err_free_pcm;
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); +err_free_pcm: + kfree(pcm); + return ret; } EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_register);
@@ -345,7 +386,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) @@ -353,15 +393,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 Fri, Nov 15, 2013 at 01:54:09PM -0700, Stephen Warren wrote:
This patch is part of a series with strong internal depdendencies. I'm looking for an ack so that I can take the entire series through the Tegra and arm-soc trees. The series will be part of a stable branch that can be merged into other subsystems if needed to avoid/resolve dependencies.
Acked-by: Mark Brown broonie@linaro.org
but please do provide the branch (ideally just the ASoC core stuff) since other drivers ought to be being enhanced to use this.
On 11/15/2013 09:54 PM, Stephen Warren wrote:
@@ -315,6 +343,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)
@@ -323,14 +352,26 @@ int snd_dmaengine_pcm_register(struct device *dev, pcm->config = config; pcm->flags = flags;
- dmaengine_pcm_request_chan_of(pcm, dev);
- ret = dmaengine_pcm_request_chan_of(pcm, dev);
- if (ret)
goto err_free_pcm;
We should still call dmaengine_pcm_release_chan() in case requesting the first channel succeeded, but the second did not.
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);
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);
+err_free_pcm:
- kfree(pcm);
- return ret;
} EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_register);
[...]
On 11/16/2013 03:49 AM, Lars-Peter Clausen wrote:
On 11/15/2013 09:54 PM, Stephen Warren wrote:
@@ -315,6 +343,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)
@@ -323,14 +352,26 @@ int snd_dmaengine_pcm_register(struct device *dev, pcm->config = config; pcm->flags = flags;
- dmaengine_pcm_request_chan_of(pcm, dev);
- ret = dmaengine_pcm_request_chan_of(pcm, dev);
- if (ret)
goto err_free_pcm;We should still call dmaengine_pcm_release_chan() in case requesting the first channel succeeded, but the second did not.
Oh yes, I'd meant to do that. I'll modify that goto, and remove the now-unused err_free_pcm label. Thanks.
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);
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);
+err_free_pcm:
- kfree(pcm);
- return ret;
} EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_register);
[...]
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.
Cc: treding@nvidia.com Cc: linux-tegra@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: alsa-devel@alsa-project.org Signed-off-by: Stephen Warren swarren@nvidia.com --- This patch is part of a series with strong internal depdendencies. I'm looking for an ack so that I can take the entire series through the Tegra and arm-soc trees. The series will be part of a stable branch that can be merged into other subsystems if needed to avoid/resolve dependencies. --- include/sound/dmaengine_pcm.h | 6 ++++++ sound/soc/soc-generic-dmaengine-pcm.c | 9 +++++++-- 2 files changed, 13 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 f7e65e1552ef..40e0943a06b8 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 int 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; @@ -296,12 +296,17 @@ static int dmaengine_pcm_request_chan_of(struct dmaengine_pcm *pcm, !dev->of_node) return 0;
+ if (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]; chan = dma_request_slave_channel_or_err(dev, name); if (IS_ERR(chan)) { if (PTR_ERR(pcm->chan[i]) == -EPROBE_DEFER) @@ -352,7 +357,7 @@ int snd_dmaengine_pcm_register(struct device *dev, pcm->config = config; pcm->flags = flags;
- ret = dmaengine_pcm_request_chan_of(pcm, dev); + ret = dmaengine_pcm_request_chan_of(pcm, dev, config); if (ret) goto err_free_pcm;
On Fri, Nov 15, 2013 at 01:54:10PM -0700, Stephen Warren wrote:
- 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
Acked-by: Mark Brown broonie@linaro.org
but this one especially I'd like to get into ASoC as soon as possible since this one should be being used by other things.
I'm a bit concerned about anything actually using dma_dev since it indicates that something is being worked around, it'd be a bit nicer to print a warning when doing this to give people a hint that they might not be doing the right thing if they use it (unless someone comes up with a system that has a clear use case for it).
On 11/16/2013 02:44 AM, Mark Brown wrote:
On Fri, Nov 15, 2013 at 01:54:10PM -0700, Stephen Warren wrote:
- 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
...
I'm a bit concerned about anything actually using dma_dev since it indicates that something is being worked around, it'd be a bit nicer to print a warning when doing this to give people a hint that they might not be doing the right thing if they use it (unless someone comes up with a system that has a clear use case for it).
What if I squash the following into that patch:
diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c index 1160d1cba133..0e2645dee96a 100644 --- a/sound/soc/soc-generic-dmaengine-pcm.c +++ b/sound/soc/soc-generic-dmaengine-pcm.c @@ -296,8 +296,17 @@ static int dmaengine_pcm_request_chan_of(struct dmaengine_pcm *pcm, !dev->of_node) return 0;
- if (config->dma_dev)
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++) {
(a few patches later) That yields the following warning on Tegra, for example:
[ 2.629623] tegra30-i2s 70080400.i2s: DMA channels sourced from device 70080000.ahub
On 11/15/2013 09:54 PM, Stephen Warren wrote:
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.
Cc: treding@nvidia.com Cc: linux-tegra@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: alsa-devel@alsa-project.org Signed-off-by: Stephen Warren swarren@nvidia.com
Acked-by: Lars-Peter Clausen lars@metafoo.de
From: Stephen Warren swarren@nvidia.com
Tegra's clock driver now provides an implementation of the common reset API (include/linux/reset.h). Use this instead of the old Tegra- specific API; that will soon be removed.
This change also renames "clock"/"clk" to "modules"/"mod" in symbols related to entries in configlink_clocks[], since: - We don't care about clock handles any more, but rather reset handles, so the old name isn't applicable. - It really is a list of modules on the bus, about which we currently only care about reset handles. If we start caring about any other aspect of the modules in the future, we won't have to rename all these symbols again.
Cc: treding@nvidia.com Cc: pdeschrijver@nvidia.com Cc: linux-tegra@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: alsa-devel@alsa-project.org Signed-off-by: Stephen Warren swarren@nvidia.com --- This patch is part of a series with strong internal depdendencies. I'm looking for an ack so that I can take the entire series through the Tegra and arm-soc trees. The series will be part of a stable branch that can be merged into other subsystems if needed to avoid/resolve dependencies. --- sound/soc/tegra/Kconfig | 2 ++ sound/soc/tegra/tegra30_ahub.c | 70 +++++++++++++++++++++++------------------- sound/soc/tegra/tegra30_ahub.h | 2 +- 3 files changed, 41 insertions(+), 33 deletions(-)
diff --git a/sound/soc/tegra/Kconfig b/sound/soc/tegra/Kconfig index 8fc653ca3ab4..896292bb853f 100644 --- a/sound/soc/tegra/Kconfig +++ b/sound/soc/tegra/Kconfig @@ -1,6 +1,8 @@ config SND_SOC_TEGRA tristate "SoC Audio for the Tegra System-on-Chip" depends on (ARCH_TEGRA && TEGRA20_APB_DMA) || COMPILE_TEST + depends on COMMON_CLK + depends on RESET_CONTROLLER select REGMAP_MMIO select SND_SOC_GENERIC_DMAENGINE_PCM help diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c index 31154338c1eb..5ce00dc48c44 100644 --- a/sound/soc/tegra/tegra30_ahub.c +++ b/sound/soc/tegra/tegra30_ahub.c @@ -24,6 +24,7 @@ #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/regmap.h> +#include <linux/reset.h> #include <linux/slab.h> #include <linux/clk/tegra.h> #include <sound/soc.h> @@ -301,27 +302,27 @@ int tegra30_ahub_unset_rx_cif_source(enum tegra30_ahub_rxcif rxcif) } EXPORT_SYMBOL_GPL(tegra30_ahub_unset_rx_cif_source);
-#define CLK_LIST_MASK_TEGRA30 BIT(0) -#define CLK_LIST_MASK_TEGRA114 BIT(1) +#define MOD_LIST_MASK_TEGRA30 BIT(0) +#define MOD_LIST_MASK_TEGRA114 BIT(1)
-#define CLK_LIST_MASK_TEGRA30_OR_LATER \ - (CLK_LIST_MASK_TEGRA30 | CLK_LIST_MASK_TEGRA114) +#define MOD_LIST_MASK_TEGRA30_OR_LATER \ + (MOD_LIST_MASK_TEGRA30 | MOD_LIST_MASK_TEGRA114)
static const struct { - const char *clk_name; - u32 clk_list_mask; -} configlink_clocks[] = { - { "i2s0", CLK_LIST_MASK_TEGRA30_OR_LATER }, - { "i2s1", CLK_LIST_MASK_TEGRA30_OR_LATER }, - { "i2s2", CLK_LIST_MASK_TEGRA30_OR_LATER }, - { "i2s3", CLK_LIST_MASK_TEGRA30_OR_LATER }, - { "i2s4", CLK_LIST_MASK_TEGRA30_OR_LATER }, - { "dam0", CLK_LIST_MASK_TEGRA30_OR_LATER }, - { "dam1", CLK_LIST_MASK_TEGRA30_OR_LATER }, - { "dam2", CLK_LIST_MASK_TEGRA30_OR_LATER }, - { "spdif_in", CLK_LIST_MASK_TEGRA30_OR_LATER }, - { "amx", CLK_LIST_MASK_TEGRA114 }, - { "adx", CLK_LIST_MASK_TEGRA114 }, + const char *rst_name; + u32 mod_list_mask; +} configlink_mods[] = { + { "i2s0", MOD_LIST_MASK_TEGRA30_OR_LATER }, + { "i2s1", MOD_LIST_MASK_TEGRA30_OR_LATER }, + { "i2s2", MOD_LIST_MASK_TEGRA30_OR_LATER }, + { "i2s3", MOD_LIST_MASK_TEGRA30_OR_LATER }, + { "i2s4", MOD_LIST_MASK_TEGRA30_OR_LATER }, + { "dam0", MOD_LIST_MASK_TEGRA30_OR_LATER }, + { "dam1", MOD_LIST_MASK_TEGRA30_OR_LATER }, + { "dam2", MOD_LIST_MASK_TEGRA30_OR_LATER }, + { "spdif", MOD_LIST_MASK_TEGRA30_OR_LATER }, + { "amx", MOD_LIST_MASK_TEGRA114 }, + { "adx", MOD_LIST_MASK_TEGRA114 }, };
#define LAST_REG(name) \ @@ -450,17 +451,17 @@ static const struct regmap_config tegra30_ahub_ahub_regmap_config = { };
static struct tegra30_ahub_soc_data soc_data_tegra30 = { - .clk_list_mask = CLK_LIST_MASK_TEGRA30, + .mod_list_mask = MOD_LIST_MASK_TEGRA30, .set_audio_cif = tegra30_ahub_set_cif, };
static struct tegra30_ahub_soc_data soc_data_tegra114 = { - .clk_list_mask = CLK_LIST_MASK_TEGRA114, + .mod_list_mask = MOD_LIST_MASK_TEGRA114, .set_audio_cif = tegra30_ahub_set_cif, };
static struct tegra30_ahub_soc_data soc_data_tegra124 = { - .clk_list_mask = CLK_LIST_MASK_TEGRA114, + .mod_list_mask = MOD_LIST_MASK_TEGRA114, .set_audio_cif = tegra124_ahub_set_cif, };
@@ -475,7 +476,7 @@ static int tegra30_ahub_probe(struct platform_device *pdev) { const struct of_device_id *match; const struct tegra30_ahub_soc_data *soc_data; - struct clk *clk; + struct reset_control *rst; int i; struct resource *res0, *res1, *region; u32 of_dma[2]; @@ -495,19 +496,24 @@ static int tegra30_ahub_probe(struct platform_device *pdev) * operate correctly, all devices on this bus must be out of reset. * Ensure that here. */ - for (i = 0; i < ARRAY_SIZE(configlink_clocks); i++) { - if (!(configlink_clocks[i].clk_list_mask & - soc_data->clk_list_mask)) + for (i = 0; i < ARRAY_SIZE(configlink_mods); i++) { + if (!(configlink_mods[i].mod_list_mask & + soc_data->mod_list_mask)) continue; - clk = clk_get(&pdev->dev, configlink_clocks[i].clk_name); - if (IS_ERR(clk)) { - dev_err(&pdev->dev, "Can't get clock %s\n", - configlink_clocks[i].clk_name); - ret = PTR_ERR(clk); + + rst = reset_control_get(&pdev->dev, + configlink_mods[i].rst_name); + if (IS_ERR(rst)) { + dev_err(&pdev->dev, "Can't get reset %s\n", + configlink_mods[i].rst_name); + ret = PTR_ERR(rst); goto err; } - tegra_periph_reset_deassert(clk); - clk_put(clk); + + ret = reset_control_deassert(rst); + reset_control_put(rst); + if (ret) + goto err; }
ahub = devm_kzalloc(&pdev->dev, sizeof(struct tegra30_ahub), diff --git a/sound/soc/tegra/tegra30_ahub.h b/sound/soc/tegra/tegra30_ahub.h index d67321d90faa..1383f8cd3572 100644 --- a/sound/soc/tegra/tegra30_ahub.h +++ b/sound/soc/tegra/tegra30_ahub.h @@ -502,7 +502,7 @@ void tegra124_ahub_set_cif(struct regmap *regmap, unsigned int reg, struct tegra30_ahub_cif_conf *conf);
struct tegra30_ahub_soc_data { - u32 clk_list_mask; + u32 mod_list_mask; void (*set_audio_cif)(struct regmap *regmap, unsigned int reg, struct tegra30_ahub_cif_conf *conf);
On Fri, Nov 15, 2013 at 01:54:11PM -0700, Stephen Warren wrote:
@@ -1,6 +1,8 @@ config SND_SOC_TEGRA tristate "SoC Audio for the Tegra System-on-Chip" depends on (ARCH_TEGRA && TEGRA20_APB_DMA) || COMPILE_TEST
- depends on COMMON_CLK
- depends on RESET_CONTROLLER
Do you depend on COMMON_CLK here? I only noticed reset controller API dependencies here but perhaps I missed this (or it's fixing a dependency that should be there already).
On 11/16/2013 02:55 AM, Mark Brown wrote:
On Fri, Nov 15, 2013 at 01:54:11PM -0700, Stephen Warren wrote:
@@ -1,6 +1,8 @@ config SND_SOC_TEGRA tristate "SoC Audio for the Tegra System-on-Chip" depends on (ARCH_TEGRA && TEGRA20_APB_DMA) || COMPILE_TEST + depends on COMMON_CLK + depends on RESET_CONTROLLER
Do you depend on COMMON_CLK here? I only noticed reset controller API dependencies here but perhaps I missed this (or it's fixing a dependency that should be there already).
It's fixing a dependency that should already be there, in the COMPILE_TEST case. In the (ARCH_TEGRA && TEGRA20_APB_DMA) case, COMMON_CLOCK is always selected.
Do you want me to split this out into a separate patch? If so, I'd prefer not to apply that separate patch immediately to 3.13 as a fix, since then it'd delay applying this series until after -rc2 is out, unless you can get the fix into -rc1 quickly...
On Mon, Nov 18, 2013 at 10:21:16AM -0700, Stephen Warren wrote:
It's fixing a dependency that should already be there, in the COMPILE_TEST case. In the (ARCH_TEGRA && TEGRA20_APB_DMA) case, COMMON_CLOCK is always selected.
Do you want me to split this out into a separate patch? If so, I'd prefer not to apply that separate patch immediately to 3.13 as a fix, since then it'd delay applying this series until after -rc2 is out, unless you can get the fix into -rc1 quickly...
I don't really care, it was just that I was looking for something to do with clocks in the patch and couldn't find anything. Perhaps a note in the changelog if you need to respin so I don't forget and say the same thing again.
On 11/18/2013 11:37 AM, Mark Brown wrote:
On Mon, Nov 18, 2013 at 10:21:16AM -0700, Stephen Warren wrote:
It's fixing a dependency that should already be there, in the COMPILE_TEST case. In the (ARCH_TEGRA && TEGRA20_APB_DMA) case, COMMON_CLOCK is always selected.
Do you want me to split this out into a separate patch? If so, I'd prefer not to apply that separate patch immediately to 3.13 as a fix, since then it'd delay applying this series until after -rc2 is out, unless you can get the fix into -rc1 quickly...
I don't really care, it was just that I was looking for something to do with clocks in the patch and couldn't find anything. Perhaps a note in the changelog if you need to respin so I don't forget and say the same thing again.
I added the following to the changelog:
---------- Note: The addition of "depends COMMON_CLOCK" is something that was missing before, not a new requirement. ----------
You didn't ack this one patch; I assume that was just an oversight?
On Mon, Nov 25, 2013 at 02:56:23PM -0700, Stephen Warren wrote:
You didn't ack this one patch; I assume that was just an oversight?
No, it was because it looked incorrect based on the lack of tie in between the description and the code.
On 11/26/2013 06:14 AM, Mark Brown wrote:
On Mon, Nov 25, 2013 at 02:56:23PM -0700, Stephen Warren wrote:
You didn't ack this one patch; I assume that was just an oversight?
No, it was because it looked incorrect based on the lack of tie in between the description and the code.
Hmm. You had asked:
@@ -1,6 +1,8 @@ config SND_SOC_TEGRA tristate "SoC Audio for the Tegra System-on-Chip" depends on (ARCH_TEGRA && TEGRA20_APB_DMA) || COMPILE_TEST + depends on COMMON_CLK + depends on RESET_CONTROLLER
Do you depend on COMMON_CLK here? I only noticed reset controller API dependencies here but perhaps I missed this (or it's fixing a dependency that should be there already).
I responded:
It's fixing a dependency that should already be there, in the COMPILE_TEST case. In the (ARCH_TEGRA && TEGRA20_APB_DMA) case, COMMON_CLOCK is always selected.
Do you want me to split this out into a separate patch? If so, I'd prefer not to apply that separate patch immediately to 3.13 as a fix, since then it'd delay applying this series until after -rc2 is out, unless you can get the fix into -rc1 quickly...
(although at this point in time, the DMA patches which this depend on aren't likely to be ready soon enough for the delay to matter, so I could send the addition of depends COMMON_CLK as a separate patch for -rc2 if you want)
and you said:
I don't really care, it was just that I was looking for something to do with clocks in the patch and couldn't find anything. Perhaps a note in the changelog if you need to respin so I don't forget and say the same thing again.
... so, I thought you were OK with that one issue. Were there other issues you didn't mention before?
On Tue, Nov 26, 2013 at 09:31:25AM -0700, Stephen Warren wrote:
... so, I thought you were OK with that one issue. Were there other issues you didn't mention before?
Probably not but I'd need to reread it, I don't think I got that much further than noticing that there weren't any clock changes (the fact that the clock dependency was getting added in a DMA series set off alarm bells). To be honest given the number of revisions that seem to be required I'd been expecting to see a respin of the series, the ASoC generic DMA changes did need a respin before they can be merged and I do want to get them into the ASoC tree.
On 11/26/2013 11:37 AM, Mark Brown wrote:
On Tue, Nov 26, 2013 at 09:31:25AM -0700, Stephen Warren wrote:
... so, I thought you were OK with that one issue. Were there other issues you didn't mention before?
Probably not but I'd need to reread it, I don't think I got that much further than noticing that there weren't any clock changes (the fact that the clock dependency was getting added in a DMA series set off alarm bells). To be honest given the number of revisions that seem to be required I'd been expecting to see a respin of the series, the ASoC generic DMA changes did need a respin before they can be merged and I do want to get them into the ASoC tree.
OK, I'll repost.
I'm waiting to repost the ASoC core changes until the dmaengine API change settles. Hopefully very soon.
From: Stephen Warren swarren@nvidia.com
Call pm_runtime_get_sync() before all register accesses; the HW requires clocks to be running when accessing registers.
This hasn't been needed to date, since all register IO was performed while playback was active, and hence the ASoC core had already called pm_runtime_get(). However, an imminent future commit will allocate and set up the FIFOs and routing during probe(), when that "protection" won't be in place.
Cc: treding@nvidia.com Cc: linux-tegra@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: alsa-devel@alsa-project.org Signed-off-by: Stephen Warren swarren@nvidia.com --- This patch is part of a series with strong internal depdendencies. I'm looking for an ack so that I can take the entire series through the Tegra and arm-soc trees. The series will be part of a stable branch that can be merged into other subsystems if needed to avoid/resolve dependencies. --- sound/soc/tegra/tegra30_ahub.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c index 5ce00dc48c44..2f1a566dd9cc 100644 --- a/sound/soc/tegra/tegra30_ahub.c +++ b/sound/soc/tegra/tegra30_ahub.c @@ -115,6 +115,8 @@ int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif, (channel * TEGRA30_AHUB_CHANNEL_RXFIFO_STRIDE); *reqsel = ahub->dma_sel + channel;
+ pm_runtime_get_sync(ahub->dev); + reg = TEGRA30_AHUB_CHANNEL_CTRL + (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE); val = tegra30_apbif_read(reg); @@ -141,6 +143,8 @@ int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif, (channel * TEGRA30_AHUB_CIF_RX_CTRL_STRIDE); ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
+ pm_runtime_put(ahub->dev); + return 0; } EXPORT_SYMBOL_GPL(tegra30_ahub_allocate_rx_fifo); @@ -150,12 +154,16 @@ int tegra30_ahub_enable_rx_fifo(enum tegra30_ahub_rxcif rxcif) int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0; int reg, val;
+ pm_runtime_get_sync(ahub->dev); + reg = TEGRA30_AHUB_CHANNEL_CTRL + (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE); val = tegra30_apbif_read(reg); val |= TEGRA30_AHUB_CHANNEL_CTRL_RX_EN; tegra30_apbif_write(reg, val);
+ pm_runtime_put(ahub->dev); + return 0; } EXPORT_SYMBOL_GPL(tegra30_ahub_enable_rx_fifo); @@ -165,12 +173,16 @@ int tegra30_ahub_disable_rx_fifo(enum tegra30_ahub_rxcif rxcif) int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0; int reg, val;
+ pm_runtime_get_sync(ahub->dev); + reg = TEGRA30_AHUB_CHANNEL_CTRL + (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE); val = tegra30_apbif_read(reg); val &= ~TEGRA30_AHUB_CHANNEL_CTRL_RX_EN; tegra30_apbif_write(reg, val);
+ pm_runtime_put(ahub->dev); + return 0; } EXPORT_SYMBOL_GPL(tegra30_ahub_disable_rx_fifo); @@ -205,6 +217,8 @@ int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif, (channel * TEGRA30_AHUB_CHANNEL_TXFIFO_STRIDE); *reqsel = ahub->dma_sel + channel;
+ pm_runtime_get_sync(ahub->dev); + reg = TEGRA30_AHUB_CHANNEL_CTRL + (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE); val = tegra30_apbif_read(reg); @@ -231,6 +245,8 @@ int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif, (channel * TEGRA30_AHUB_CIF_TX_CTRL_STRIDE); ahub->soc_data->set_audio_cif(ahub->regmap_apbif, reg, &cif_conf);
+ pm_runtime_put(ahub->dev); + return 0; } EXPORT_SYMBOL_GPL(tegra30_ahub_allocate_tx_fifo); @@ -240,12 +256,16 @@ int tegra30_ahub_enable_tx_fifo(enum tegra30_ahub_txcif txcif) int channel = txcif - TEGRA30_AHUB_TXCIF_APBIF_TX0; int reg, val;
+ pm_runtime_get_sync(ahub->dev); + reg = TEGRA30_AHUB_CHANNEL_CTRL + (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE); val = tegra30_apbif_read(reg); val |= TEGRA30_AHUB_CHANNEL_CTRL_TX_EN; tegra30_apbif_write(reg, val);
+ pm_runtime_put(ahub->dev); + return 0; } EXPORT_SYMBOL_GPL(tegra30_ahub_enable_tx_fifo); @@ -255,12 +275,16 @@ int tegra30_ahub_disable_tx_fifo(enum tegra30_ahub_txcif txcif) int channel = txcif - TEGRA30_AHUB_TXCIF_APBIF_TX0; int reg, val;
+ pm_runtime_get_sync(ahub->dev); + reg = TEGRA30_AHUB_CHANNEL_CTRL + (channel * TEGRA30_AHUB_CHANNEL_CTRL_STRIDE); val = tegra30_apbif_read(reg); val &= ~TEGRA30_AHUB_CHANNEL_CTRL_TX_EN; tegra30_apbif_write(reg, val);
+ pm_runtime_put(ahub->dev); + return 0; } EXPORT_SYMBOL_GPL(tegra30_ahub_disable_tx_fifo); @@ -281,10 +305,14 @@ int tegra30_ahub_set_rx_cif_source(enum tegra30_ahub_rxcif rxcif, int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0; int reg;
+ pm_runtime_get_sync(ahub->dev); + reg = TEGRA30_AHUB_AUDIO_RX + (channel * TEGRA30_AHUB_AUDIO_RX_STRIDE); tegra30_audio_write(reg, 1 << txcif);
+ pm_runtime_put(ahub->dev); + return 0; } EXPORT_SYMBOL_GPL(tegra30_ahub_set_rx_cif_source); @@ -294,10 +322,14 @@ int tegra30_ahub_unset_rx_cif_source(enum tegra30_ahub_rxcif rxcif) int channel = rxcif - TEGRA30_AHUB_RXCIF_APBIF_RX0; int reg;
+ pm_runtime_get_sync(ahub->dev); + reg = TEGRA30_AHUB_AUDIO_RX + (channel * TEGRA30_AHUB_AUDIO_RX_STRIDE); tegra30_audio_write(reg, 0);
+ pm_runtime_put(ahub->dev); + return 0; } EXPORT_SYMBOL_GPL(tegra30_ahub_unset_rx_cif_source);
On Fri, Nov 15, 2013 at 01:54:12PM -0700, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Call pm_runtime_get_sync() before all register accesses; the HW requires clocks to be running when accessing registers.
This hasn't been needed to date, since all register IO was performed while playback was active, and hence the ASoC core had already called pm_runtime_get(). However, an imminent future commit will allocate and set up the FIFOs and routing during probe(), when that "protection" won't be in place.
Acked-by: Mark Brown broonie@linaro.org
However should we fix this at the regmap level in the same way that we do for clocks? That would need to be using _put_autosuspend() to avoid being horrific. Or alternatively should the driver be making the device cache only when runtime PM is disabled?
On 11/16/2013 03:02 AM, Mark Brown wrote:
On Fri, Nov 15, 2013 at 01:54:12PM -0700, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Call pm_runtime_get_sync() before all register accesses; the HW requires clocks to be running when accessing registers.
This hasn't been needed to date, since all register IO was performed while playback was active, and hence the ASoC core had already called pm_runtime_get(). However, an imminent future commit will allocate and set up the FIFOs and routing during probe(), when that "protection" won't be in place.
Acked-by: Mark Brown broonie@linaro.org
However should we fix this at the regmap level in the same way that we do for clocks? That would need to be using _put_autosuspend() to avoid being horrific.
I did wonder about that, but it seemed like rather a lot of overhead?
Or alternatively should the driver be making the device cache only when runtime PM is disabled?
The regmap is already cache-only when runtime-suspended. However the registers don't get flushed during resume. I suppose that would require only adding one extra call to the PM resume function?
For some reason, my gut prefers this current solution, but I could be persuaded.
On Mon, Nov 18, 2013 at 10:25:46AM -0700, Stephen Warren wrote:
On 11/16/2013 03:02 AM, Mark Brown wrote:
Or alternatively should the driver be making the device cache only when runtime PM is disabled?
The regmap is already cache-only when runtime-suspended. However the registers don't get flushed during resume. I suppose that would require only adding one extra call to the PM resume function?
Yup, should probably do that anyway since that's going to bite someone otherwise. We probably want a single operation to flush and enable writes now I think about it, it's the common case.
For some reason, my gut prefers this current solution, but I could be persuaded.
I'm not dead set against doing it this way, it's just that it feels like if it's required something's not working as well as it should.
On 11/18/2013 11:39 AM, Mark Brown wrote:
On Mon, Nov 18, 2013 at 10:25:46AM -0700, Stephen Warren wrote:
On 11/16/2013 03:02 AM, Mark Brown wrote:
Or alternatively should the driver be making the device cache only when runtime PM is disabled?
The regmap is already cache-only when runtime-suspended. However the registers don't get flushed during resume. I suppose that would require only adding one extra call to the PM resume function?
Yup, should probably do that anyway since that's going to bite someone otherwise. We probably want a single operation to flush and enable writes now I think about it, it's the common case.
For some reason, my gut prefers this current solution, but I could be persuaded.
I'm not dead set against doing it this way, it's just that it feels like if it's required something's not working as well as it should.
The problems with the following approach:
static int tegra30_ahub_runtime_resume(struct device *dev) ... regcache_cache_only(ahub->regmap_apbif, false); regcache_cache_only(ahub->regmap_ahub, false); + regcache_sync(ahub->regmap_apbif, false); + regcache_sync(ahub->regmap_ahub, false);
(or an automated variant of it, whereby regmap automatically does the sync inside cache_only(false)) are:
1) regcache_sync() doesn't mean "if the cache is dirty, flush the dirty registers to HW", but rather, "if the cache is dirty, write any registers that don't match HW defaults to HW". If the HW was in cache-only mode because simply because clocks were turned off but not power, then the register values were retained, and the current HW register values may not be "HW defaults", and you may in fact /need/ to write a value to HW that matches the HW default, yet is different from the current retained register content.
2) tegra30_ahub_allocate_tx_fifo() does a read-modify-write of a control register, and may execute when cache_only(true). If that register was never touched while cache_only(false), which is the case the first time the r-m-w happens, then there is no cached value, so the regmap_read() for it will fail. This will either cause tegra30_ahub_allocate_tx_fifo() to use uninitialized return data from regmap_read() (if error-checking is missing, which it is), or fail (if the error-checking were present). Neither is a good choice. Possible solutions are:
a) Make regmap creation read the initial HW state to use as HW defaults when the regmap is created. IIRC, this is done for some regmap configurations but not others. That said, this doesn't seem correct, since there's no guarantee that the HW state when the regmap is created /is/ the default HW state.
b) Add a table of HW register defaults. Aside from this issue, there's no need for this, so I'm not really motivated to do this. Besides, the number of registers is rather large.
c) Simply put pm_runtime_get()/put() around the body of tegra30_ahub_allocate_tx_fifo(), which works fine, and was my original patch.
d) Have regmap_read/write do a pm_runtime_get()/put() themselves. I know you mentioned this earlier in the thread, but you'd rejected this approach when I first upstreamed this AHUB driver, and said to rely on cache_only instead.
I think I still prefer option (c).
On Mon, Nov 18, 2013 at 03:38:36PM -0700, Stephen Warren wrote:
- regcache_sync() doesn't mean "if the cache is dirty, flush the
dirty registers to HW", but rather, "if the cache is dirty, write any registers that don't match HW defaults to HW". If the HW was in cache-only mode because simply because clocks were turned off but not power, then the register values were retained, and the current HW register values may not be "HW defaults", and you may in fact /need/ to write a value to HW that matches the HW default, yet is different from the current retained register content.
OK, that's a potential problem in general... Fortunately most of the uses actually cut power.
a) Make regmap creation read the initial HW state to use as HW defaults when the regmap is created. IIRC, this is done for some regmap configurations but not others. That said, this doesn't seem correct, since there's no guarantee that the HW state when the regmap is created /is/ the default HW state.
Right, this is why we don't initialise the cache by default.
I think I still prefer option (c).
there's also the option of doing an explicit read on that register to get it into the cache. But yeah, like I say I'm not totally against this and I did ack it already.
From: Stephen Warren swarren@nvidia.com
Teh Tegra30 I2S driver currently allocates DMA FIFOs from the AHUB only when an audio stream starts playback. This is theoretically nice for resource sharing, but makes no practical difference for any configuration the drivers currently support. However, this deferral prevents conversion to the standard DMA DT bindings, since conversion requires knowledge of the specific DMA channel to be allocated, which in turn depends on which specific FIFO was allocated.
For this reason, move the FIFO allocate into probe() to allow later conversion to the standard DMA DT bindings.
Cc: treding@nvidia.com Cc: linux-tegra@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: alsa-devel@alsa-project.org Signed-off-by: Stephen Warren swarren@nvidia.com --- This patch is part of a series with strong internal depdendencies. I'm looking for an ack so that I can take the entire series through the Tegra and arm-soc trees. The series will be part of a stable branch that can be merged into other subsystems if needed to avoid/resolve dependencies. --- sound/soc/tegra/tegra30_i2s.c | 91 ++++++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 44 deletions(-)
diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c index 231a785b3921..531a1ff2101d 100644 --- a/sound/soc/tegra/tegra30_i2s.c +++ b/sound/soc/tegra/tegra30_i2s.c @@ -73,47 +73,6 @@ static int tegra30_i2s_runtime_resume(struct device *dev) return 0; }
-static int tegra30_i2s_startup(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai); - int ret; - - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - ret = tegra30_ahub_allocate_tx_fifo(&i2s->playback_fifo_cif, - &i2s->playback_dma_data.addr, - &i2s->playback_dma_data.slave_id); - i2s->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; - i2s->playback_dma_data.maxburst = 4; - tegra30_ahub_set_rx_cif_source(i2s->playback_i2s_cif, - i2s->playback_fifo_cif); - } else { - ret = tegra30_ahub_allocate_rx_fifo(&i2s->capture_fifo_cif, - &i2s->capture_dma_data.addr, - &i2s->capture_dma_data.slave_id); - i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; - i2s->capture_dma_data.maxburst = 4; - tegra30_ahub_set_rx_cif_source(i2s->capture_fifo_cif, - i2s->capture_i2s_cif); - } - - return ret; -} - -static void tegra30_i2s_shutdown(struct snd_pcm_substream *substream, - struct snd_soc_dai *dai) -{ - struct tegra30_i2s *i2s = snd_soc_dai_get_drvdata(dai); - - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { - tegra30_ahub_unset_rx_cif_source(i2s->playback_i2s_cif); - tegra30_ahub_free_tx_fifo(i2s->playback_fifo_cif); - } else { - tegra30_ahub_unset_rx_cif_source(i2s->capture_fifo_cif); - tegra30_ahub_free_rx_fifo(i2s->capture_fifo_cif); - } -} - static int tegra30_i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) { @@ -317,8 +276,6 @@ static int tegra30_i2s_probe(struct snd_soc_dai *dai) }
static struct snd_soc_dai_ops tegra30_i2s_dai_ops = { - .startup = tegra30_i2s_startup, - .shutdown = tegra30_i2s_shutdown, .set_fmt = tegra30_i2s_set_fmt, .hw_params = tegra30_i2s_hw_params, .trigger = tegra30_i2s_trigger, @@ -499,12 +456,44 @@ static int tegra30_i2s_platform_probe(struct platform_device *pdev) goto err_pm_disable; }
+ i2s->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; + i2s->playback_dma_data.maxburst = 4; + ret = tegra30_ahub_allocate_tx_fifo(&i2s->playback_fifo_cif, + &i2s->playback_dma_data.addr, + &i2s->playback_dma_data.slave_id); + if (ret) { + dev_err(&pdev->dev, "Could not alloc TX FIFO: %d\n", ret); + goto err_suspend; + } + ret = tegra30_ahub_set_rx_cif_source(i2s->playback_i2s_cif, + i2s->playback_fifo_cif); + if (ret) { + dev_err(&pdev->dev, "Could not route TX FIFO: %d\n", ret); + goto err_free_tx_fifo; + } + + i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; + i2s->capture_dma_data.maxburst = 4; + ret = tegra30_ahub_allocate_rx_fifo(&i2s->capture_fifo_cif, + &i2s->capture_dma_data.addr, + &i2s->capture_dma_data.slave_id); + if (ret) { + dev_err(&pdev->dev, "Could not alloc RX FIFO: %d\n", ret); + goto err_unroute_tx_fifo; + } + ret = tegra30_ahub_set_rx_cif_source(i2s->capture_fifo_cif, + i2s->capture_i2s_cif); + if (ret) { + dev_err(&pdev->dev, "Could not route TX FIFO: %d\n", ret); + goto err_free_rx_fifo; + } + ret = snd_soc_register_component(&pdev->dev, &tegra30_i2s_component, &i2s->dai, 1); if (ret) { dev_err(&pdev->dev, "Could not register DAI: %d\n", ret); ret = -ENOMEM; - goto err_suspend; + goto err_unroute_rx_fifo; }
ret = tegra_pcm_platform_register(&pdev->dev); @@ -517,6 +506,14 @@ static int tegra30_i2s_platform_probe(struct platform_device *pdev)
err_unregister_component: snd_soc_unregister_component(&pdev->dev); +err_unroute_rx_fifo: + tegra30_ahub_unset_rx_cif_source(i2s->capture_fifo_cif); +err_free_rx_fifo: + tegra30_ahub_free_rx_fifo(i2s->capture_fifo_cif); +err_unroute_tx_fifo: + tegra30_ahub_unset_rx_cif_source(i2s->playback_i2s_cif); +err_free_tx_fifo: + tegra30_ahub_free_tx_fifo(i2s->playback_fifo_cif); err_suspend: if (!pm_runtime_status_suspended(&pdev->dev)) tegra30_i2s_runtime_suspend(&pdev->dev); @@ -539,6 +536,12 @@ static int tegra30_i2s_platform_remove(struct platform_device *pdev) tegra_pcm_platform_unregister(&pdev->dev); snd_soc_unregister_component(&pdev->dev);
+ tegra30_ahub_unset_rx_cif_source(i2s->capture_fifo_cif); + tegra30_ahub_free_rx_fifo(i2s->capture_fifo_cif); + + tegra30_ahub_unset_rx_cif_source(i2s->playback_i2s_cif); + tegra30_ahub_free_tx_fifo(i2s->playback_fifo_cif); + clk_put(i2s->clk_i2s);
return 0;
On Fri, Nov 15, 2013 at 01:54:13PM -0700, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Teh Tegra30 I2S driver currently allocates DMA FIFOs from the AHUB only when an audio stream starts playback. This is theoretically nice for resource sharing, but makes no practical difference for any configuration
Acked-by: Mark Brown broonie@linaro.org
On Fri, Nov 15, 2013 at 01:54:13PM -0700, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
Teh Tegra30 I2S driver currently allocates DMA FIFOs from the AHUB only
s/Teh/The/
when an audio stream starts playback. This is theoretically nice for resource sharing, but makes no practical difference for any configuration the drivers currently support. However, this deferral prevents conversion to the standard DMA DT bindings, since conversion requires knowledge of the specific DMA channel to be allocated, which in turn depends on which specific FIFO was allocated.
For this reason, move the FIFO allocate into probe() to allow later
s/allocate/allocation/?
[...]
- i2s->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
- i2s->playback_dma_data.maxburst = 4;
- ret = tegra30_ahub_allocate_tx_fifo(&i2s->playback_fifo_cif,
&i2s->playback_dma_data.addr,&i2s->playback_dma_data.slave_id);- if (ret) {
dev_err(&pdev->dev, "Could not alloc TX FIFO: %d\n", ret);goto err_suspend;- }
- ret = tegra30_ahub_set_rx_cif_source(i2s->playback_i2s_cif,
i2s->playback_fifo_cif);- if (ret) {
dev_err(&pdev->dev, "Could not route TX FIFO: %d\n", ret);goto err_free_tx_fifo;- }
- i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
- i2s->capture_dma_data.maxburst = 4;
- ret = tegra30_ahub_allocate_rx_fifo(&i2s->capture_fifo_cif,
&i2s->capture_dma_data.addr,&i2s->capture_dma_data.slave_id);- if (ret) {
dev_err(&pdev->dev, "Could not alloc RX FIFO: %d\n", ret);goto err_unroute_tx_fifo;- }
- ret = tegra30_ahub_set_rx_cif_source(i2s->capture_fifo_cif,
i2s->capture_i2s_cif);- if (ret) {
dev_err(&pdev->dev, "Could not route TX FIFO: %d\n", ret);goto err_free_rx_fifo;- }
It could be useful to have these in a separate function so as not to make the .probe() any larger. It's already pretty big as it is.
Thierry
On 11/29/2013 07:40 AM, Thierry Reding wrote:
On Fri, Nov 15, 2013 at 01:54:13PM -0700, Stephen Warren wrote:
Teh Tegra30 I2S driver currently allocates DMA FIFOs from the AHUB only when an audio stream starts playback. This is theoretically nice for resource sharing, but makes no practical difference for any configuration the drivers currently support. However, this deferral prevents conversion to the standard DMA DT bindings, since conversion requires knowledge of the specific DMA channel to be allocated, which in turn depends on which specific FIFO was allocated.
For this reason, move the FIFO allocate into probe() to allow later
...
- i2s->playback_dma_data.addr_width =
DMA_SLAVE_BUSWIDTH_4_BYTES; + i2s->playback_dma_data.maxburst = 4; + ret = tegra30_ahub_allocate_tx_fifo(&i2s->playback_fifo_cif, + &i2s->playback_dma_data.addr, + &i2s->playback_dma_data.slave_id); + if (ret) { + dev_err(&pdev->dev, "Could not alloc TX FIFO: %d\n", ret); + goto err_suspend; + } + ret = tegra30_ahub_set_rx_cif_source(i2s->playback_i2s_cif, + i2s->playback_fifo_cif); + if (ret) { + dev_err(&pdev->dev, "Could not route TX FIFO: %d\n", ret); + goto err_free_tx_fifo;
- } + + i2s->capture_dma_data.addr_width =
DMA_SLAVE_BUSWIDTH_4_BYTES; + i2s->capture_dma_data.maxburst = 4; + ret = tegra30_ahub_allocate_rx_fifo(&i2s->capture_fifo_cif,
&i2s->capture_dma_data.addr, +&i2s->capture_dma_data.slave_id); + if (ret) { + dev_err(&pdev->dev, "Could not alloc RX FIFO: %d\n", ret); + goto err_unroute_tx_fifo; + } + ret = tegra30_ahub_set_rx_cif_source(i2s->capture_fifo_cif, + i2s->capture_i2s_cif); + if (ret) { + dev_err(&pdev->dev, "Could not route TX FIFO: %d\n", ret); + goto err_free_rx_fifo; + } +
It could be useful to have these in a separate function so as not to make the .probe() any larger. It's already pretty big as it is.
May I ignore this; I personally prefer larger linear functions; it's much easier to follow the code-flow without having to jump back/forth between a bunch of single-use functions.
On Tue, Dec 03, 2013 at 12:55:55PM -0700, Stephen Warren wrote:
On 11/29/2013 07:40 AM, Thierry Reding wrote:
On Fri, Nov 15, 2013 at 01:54:13PM -0700, Stephen Warren wrote:
Teh Tegra30 I2S driver currently allocates DMA FIFOs from the AHUB only when an audio stream starts playback. This is theoretically nice for resource sharing, but makes no practical difference for any configuration the drivers currently support. However, this deferral prevents conversion to the standard DMA DT bindings, since conversion requires knowledge of the specific DMA channel to be allocated, which in turn depends on which specific FIFO was allocated.
For this reason, move the FIFO allocate into probe() to allow later
...
- i2s->playback_dma_data.addr_width =
DMA_SLAVE_BUSWIDTH_4_BYTES; + i2s->playback_dma_data.maxburst = 4; + ret = tegra30_ahub_allocate_tx_fifo(&i2s->playback_fifo_cif, + &i2s->playback_dma_data.addr, + &i2s->playback_dma_data.slave_id); + if (ret) { + dev_err(&pdev->dev, "Could not alloc TX FIFO: %d\n", ret); + goto err_suspend; + } + ret = tegra30_ahub_set_rx_cif_source(i2s->playback_i2s_cif, + i2s->playback_fifo_cif); + if (ret) { + dev_err(&pdev->dev, "Could not route TX FIFO: %d\n", ret); + goto err_free_tx_fifo;
- } + + i2s->capture_dma_data.addr_width =
DMA_SLAVE_BUSWIDTH_4_BYTES; + i2s->capture_dma_data.maxburst = 4; + ret = tegra30_ahub_allocate_rx_fifo(&i2s->capture_fifo_cif,
&i2s->capture_dma_data.addr, +&i2s->capture_dma_data.slave_id); + if (ret) { + dev_err(&pdev->dev, "Could not alloc RX FIFO: %d\n", ret); + goto err_unroute_tx_fifo; + } + ret = tegra30_ahub_set_rx_cif_source(i2s->capture_fifo_cif, + i2s->capture_i2s_cif); + if (ret) { + dev_err(&pdev->dev, "Could not route TX FIFO: %d\n", ret); + goto err_free_rx_fifo; + } +
It could be useful to have these in a separate function so as not to make the .probe() any larger. It's already pretty big as it is.
May I ignore this; I personally prefer larger linear functions; it's much easier to follow the code-flow without having to jump back/forth between a bunch of single-use functions.
Alright, I won't lose any sleep over it.
Thierry
From: Stephen Warren swarren@nvidia.com
By passing no flags when calling snd_dmaengine_pcm_register() from tegra_pcm.c, we end up using dma_request_slave_channel() rather than dmaengine_pcm_compat_request_channel(), and hence rely on the standard DMA DT bindings and stashing the DMA slave ID away during channel allocation. This means there's no need to use a custom DT property to store the slave ID. So, remove all the code that parsed it.
Cc: treding@nvidia.com Cc: linux-tegra@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: Liam Girdwood lgirdwood@gmail.com Cc: Mark Brown broonie@kernel.org Cc: alsa-devel@alsa-project.org Signed-off-by: Stephen Warren swarren@nvidia.com --- This patch is part of a series with strong internal depdendencies. I'm looking for an ack so that I can take the entire series through the Tegra and arm-soc trees. The series will be part of a stable branch that can be merged into other subsystems if needed to avoid/resolve dependencies. --- sound/soc/tegra/tegra20_ac97.c | 11 ----------- sound/soc/tegra/tegra20_i2s.c | 20 +------------------- sound/soc/tegra/tegra30_ahub.c | 23 ++++++----------------- sound/soc/tegra/tegra30_ahub.h | 9 ++++----- sound/soc/tegra/tegra30_i2s.c | 14 +++++++++----- sound/soc/tegra/tegra30_i2s.h | 3 +++ sound/soc/tegra/tegra_pcm.c | 17 ++++++++++++++--- sound/soc/tegra/tegra_pcm.h | 5 +++++ 8 files changed, 42 insertions(+), 60 deletions(-)
diff --git a/sound/soc/tegra/tegra20_ac97.c b/sound/soc/tegra/tegra20_ac97.c index ae27bcd586d2..d8b98d70ff41 100644 --- a/sound/soc/tegra/tegra20_ac97.c +++ b/sound/soc/tegra/tegra20_ac97.c @@ -313,7 +313,6 @@ static int tegra20_ac97_platform_probe(struct platform_device *pdev) { struct tegra20_ac97 *ac97; struct resource *mem; - u32 of_dma[2]; void __iomem *regs; int ret = 0;
@@ -348,14 +347,6 @@ static int tegra20_ac97_platform_probe(struct platform_device *pdev) goto err_clk_put; }
- if (of_property_read_u32_array(pdev->dev.of_node, - "nvidia,dma-request-selector", - of_dma, 2) < 0) { - dev_err(&pdev->dev, "No DMA resource\n"); - ret = -ENODEV; - goto err_clk_put; - } - ac97->reset_gpio = of_get_named_gpio(pdev->dev.of_node, "nvidia,codec-reset-gpio", 0); if (gpio_is_valid(ac97->reset_gpio)) { @@ -380,12 +371,10 @@ static int tegra20_ac97_platform_probe(struct platform_device *pdev) ac97->capture_dma_data.addr = mem->start + TEGRA20_AC97_FIFO_RX1; ac97->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; ac97->capture_dma_data.maxburst = 4; - ac97->capture_dma_data.slave_id = of_dma[1];
ac97->playback_dma_data.addr = mem->start + TEGRA20_AC97_FIFO_TX1; ac97->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; ac97->playback_dma_data.maxburst = 4; - ac97->playback_dma_data.slave_id = of_dma[1];
ret = tegra_asoc_utils_init(&ac97->util_data, &pdev->dev); if (ret) diff --git a/sound/soc/tegra/tegra20_i2s.c b/sound/soc/tegra/tegra20_i2s.c index 364bf6a907e1..1dc869c475e7 100644 --- a/sound/soc/tegra/tegra20_i2s.c +++ b/sound/soc/tegra/tegra20_i2s.c @@ -339,9 +339,7 @@ static const struct regmap_config tegra20_i2s_regmap_config = { static int tegra20_i2s_platform_probe(struct platform_device *pdev) { struct tegra20_i2s *i2s; - struct resource *mem, *memregion, *dmareq; - u32 of_dma[2]; - u32 dma_ch; + struct resource *mem, *memregion; void __iomem *regs; int ret;
@@ -370,20 +368,6 @@ static int tegra20_i2s_platform_probe(struct platform_device *pdev) goto err_clk_put; }
- dmareq = platform_get_resource(pdev, IORESOURCE_DMA, 0); - if (!dmareq) { - if (of_property_read_u32_array(pdev->dev.of_node, - "nvidia,dma-request-selector", - of_dma, 2) < 0) { - dev_err(&pdev->dev, "No DMA resource\n"); - ret = -ENODEV; - goto err_clk_put; - } - dma_ch = of_dma[1]; - } else { - dma_ch = dmareq->start; - } - memregion = devm_request_mem_region(&pdev->dev, mem->start, resource_size(mem), DRV_NAME); if (!memregion) { @@ -410,12 +394,10 @@ static int tegra20_i2s_platform_probe(struct platform_device *pdev) i2s->capture_dma_data.addr = mem->start + TEGRA20_I2S_FIFO2; i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; i2s->capture_dma_data.maxburst = 4; - i2s->capture_dma_data.slave_id = dma_ch;
i2s->playback_dma_data.addr = mem->start + TEGRA20_I2S_FIFO1; i2s->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; i2s->playback_dma_data.maxburst = 4; - i2s->playback_dma_data.slave_id = dma_ch;
pm_runtime_enable(&pdev->dev); if (!pm_runtime_enabled(&pdev->dev)) { diff --git a/sound/soc/tegra/tegra30_ahub.c b/sound/soc/tegra/tegra30_ahub.c index 2f1a566dd9cc..572fab23e75f 100644 --- a/sound/soc/tegra/tegra30_ahub.c +++ b/sound/soc/tegra/tegra30_ahub.c @@ -96,8 +96,8 @@ static int tegra30_ahub_runtime_resume(struct device *dev) }
int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif, - dma_addr_t *fiforeg, - unsigned int *reqsel) + char *dmachan, int dmachan_len, + dma_addr_t *fiforeg) { int channel; u32 reg, val; @@ -111,9 +111,9 @@ int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif, __set_bit(channel, ahub->rx_usage);
*rxcif = TEGRA30_AHUB_RXCIF_APBIF_RX0 + channel; + snprintf(dmachan, dmachan_len, "rx%d", channel); *fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_RXFIFO + (channel * TEGRA30_AHUB_CHANNEL_RXFIFO_STRIDE); - *reqsel = ahub->dma_sel + channel;
pm_runtime_get_sync(ahub->dev);
@@ -198,8 +198,8 @@ int tegra30_ahub_free_rx_fifo(enum tegra30_ahub_rxcif rxcif) EXPORT_SYMBOL_GPL(tegra30_ahub_free_rx_fifo);
int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif, - dma_addr_t *fiforeg, - unsigned int *reqsel) + char *dmachan, int dmachan_len, + dma_addr_t *fiforeg) { int channel; u32 reg, val; @@ -213,9 +213,9 @@ int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif, __set_bit(channel, ahub->tx_usage);
*txcif = TEGRA30_AHUB_TXCIF_APBIF_TX0 + channel; + snprintf(dmachan, dmachan_len, "tx%d", channel); *fiforeg = ahub->apbif_addr + TEGRA30_AHUB_CHANNEL_TXFIFO + (channel * TEGRA30_AHUB_CHANNEL_TXFIFO_STRIDE); - *reqsel = ahub->dma_sel + channel;
pm_runtime_get_sync(ahub->dev);
@@ -511,7 +511,6 @@ static int tegra30_ahub_probe(struct platform_device *pdev) struct reset_control *rst; int i; struct resource *res0, *res1, *region; - u32 of_dma[2]; void __iomem *regs_apbif, *regs_ahub; int ret = 0;
@@ -574,16 +573,6 @@ static int tegra30_ahub_probe(struct platform_device *pdev) goto err_clk_put_d_audio; }
- if (of_property_read_u32_array(pdev->dev.of_node, - "nvidia,dma-request-selector", - of_dma, 2) < 0) { - dev_err(&pdev->dev, - "Missing property nvidia,dma-request-selector\n"); - ret = -ENODEV; - goto err_clk_put_d_audio; - } - ahub->dma_sel = of_dma[1]; - res0 = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res0) { dev_err(&pdev->dev, "No apbif memory resource\n"); diff --git a/sound/soc/tegra/tegra30_ahub.h b/sound/soc/tegra/tegra30_ahub.h index 1383f8cd3572..fd7ba75ed814 100644 --- a/sound/soc/tegra/tegra30_ahub.h +++ b/sound/soc/tegra/tegra30_ahub.h @@ -465,15 +465,15 @@ enum tegra30_ahub_rxcif { };
extern int tegra30_ahub_allocate_rx_fifo(enum tegra30_ahub_rxcif *rxcif, - dma_addr_t *fiforeg, - unsigned int *reqsel); + char *dmachan, int dmachan_len, + dma_addr_t *fiforeg); extern int tegra30_ahub_enable_rx_fifo(enum tegra30_ahub_rxcif rxcif); extern int tegra30_ahub_disable_rx_fifo(enum tegra30_ahub_rxcif rxcif); extern int tegra30_ahub_free_rx_fifo(enum tegra30_ahub_rxcif rxcif);
extern int tegra30_ahub_allocate_tx_fifo(enum tegra30_ahub_txcif *txcif, - dma_addr_t *fiforeg, - unsigned int *reqsel); + char *dmachan, int dmachan_len, + dma_addr_t *fiforeg); extern int tegra30_ahub_enable_tx_fifo(enum tegra30_ahub_txcif txcif); extern int tegra30_ahub_disable_tx_fifo(enum tegra30_ahub_txcif txcif); extern int tegra30_ahub_free_tx_fifo(enum tegra30_ahub_txcif txcif); @@ -524,7 +524,6 @@ struct tegra30_ahub { struct device *dev; struct clk *clk_d_audio; struct clk *clk_apbif; - int dma_sel; resource_size_t apbif_addr; struct regmap *regmap_apbif; struct regmap *regmap_ahub; diff --git a/sound/soc/tegra/tegra30_i2s.c b/sound/soc/tegra/tegra30_i2s.c index 531a1ff2101d..362e8f728ddf 100644 --- a/sound/soc/tegra/tegra30_i2s.c +++ b/sound/soc/tegra/tegra30_i2s.c @@ -459,8 +459,9 @@ static int tegra30_i2s_platform_probe(struct platform_device *pdev) i2s->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; i2s->playback_dma_data.maxburst = 4; ret = tegra30_ahub_allocate_tx_fifo(&i2s->playback_fifo_cif, - &i2s->playback_dma_data.addr, - &i2s->playback_dma_data.slave_id); + i2s->playback_dma_chan, + sizeof(i2s->playback_dma_chan), + &i2s->playback_dma_data.addr); if (ret) { dev_err(&pdev->dev, "Could not alloc TX FIFO: %d\n", ret); goto err_suspend; @@ -475,8 +476,9 @@ static int tegra30_i2s_platform_probe(struct platform_device *pdev) i2s->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES; i2s->capture_dma_data.maxburst = 4; ret = tegra30_ahub_allocate_rx_fifo(&i2s->capture_fifo_cif, - &i2s->capture_dma_data.addr, - &i2s->capture_dma_data.slave_id); + i2s->capture_dma_chan, + sizeof(i2s->capture_dma_chan), + &i2s->capture_dma_data.addr); if (ret) { dev_err(&pdev->dev, "Could not alloc RX FIFO: %d\n", ret); goto err_unroute_tx_fifo; @@ -496,7 +498,9 @@ static int tegra30_i2s_platform_probe(struct platform_device *pdev) goto err_unroute_rx_fifo; }
- ret = tegra_pcm_platform_register(&pdev->dev); + ret = tegra_pcm_platform_register_with_chan_names(&pdev->dev, + &i2s->dma_config, i2s->playback_dma_chan, + i2s->capture_dma_chan); if (ret) { dev_err(&pdev->dev, "Could not register PCM: %d\n", ret); goto err_unregister_component; diff --git a/sound/soc/tegra/tegra30_i2s.h b/sound/soc/tegra/tegra30_i2s.h index 4d0b0a30dbfb..774fc6ad2026 100644 --- a/sound/soc/tegra/tegra30_i2s.h +++ b/sound/soc/tegra/tegra30_i2s.h @@ -238,11 +238,14 @@ struct tegra30_i2s { struct clk *clk_i2s; enum tegra30_ahub_txcif capture_i2s_cif; enum tegra30_ahub_rxcif capture_fifo_cif; + char capture_dma_chan[8]; struct snd_dmaengine_dai_dma_data capture_dma_data; enum tegra30_ahub_rxcif playback_i2s_cif; enum tegra30_ahub_txcif playback_fifo_cif; + char playback_dma_chan[8]; struct snd_dmaengine_dai_dma_data playback_dma_data; struct regmap *regmap; + struct snd_dmaengine_pcm_config dma_config; };
#endif diff --git a/sound/soc/tegra/tegra_pcm.c b/sound/soc/tegra/tegra_pcm.c index 7b2d23ba69b3..7ce5c334a660 100644 --- a/sound/soc/tegra/tegra_pcm.c +++ b/sound/soc/tegra/tegra_pcm.c @@ -61,12 +61,23 @@ static const struct snd_dmaengine_pcm_config tegra_dmaengine_pcm_config = {
int tegra_pcm_platform_register(struct device *dev) { - return snd_dmaengine_pcm_register(dev, &tegra_dmaengine_pcm_config, - SND_DMAENGINE_PCM_FLAG_NO_DT | - SND_DMAENGINE_PCM_FLAG_COMPAT); + return snd_dmaengine_pcm_register(dev, &tegra_dmaengine_pcm_config, 0); } EXPORT_SYMBOL_GPL(tegra_pcm_platform_register);
+int tegra_pcm_platform_register_with_chan_names(struct device *dev, + struct snd_dmaengine_pcm_config *config, + char *txdmachan, char *rxdmachan) +{ + *config = tegra_dmaengine_pcm_config; + config->dma_dev = dev->parent; + config->chan_names[0] = txdmachan; + config->chan_names[1] = rxdmachan; + + return snd_dmaengine_pcm_register(dev, config, 0); +} +EXPORT_SYMBOL_GPL(tegra_pcm_platform_register_with_chan_names); + void tegra_pcm_platform_unregister(struct device *dev) { return snd_dmaengine_pcm_unregister(dev); diff --git a/sound/soc/tegra/tegra_pcm.h b/sound/soc/tegra/tegra_pcm.h index 68ad901714a9..7883dec748a3 100644 --- a/sound/soc/tegra/tegra_pcm.h +++ b/sound/soc/tegra/tegra_pcm.h @@ -31,7 +31,12 @@ #ifndef __TEGRA_PCM_H__ #define __TEGRA_PCM_H__
+struct snd_dmaengine_pcm_config; + int tegra_pcm_platform_register(struct device *dev); +int tegra_pcm_platform_register_with_chan_names(struct device *dev, + struct snd_dmaengine_pcm_config *config, + char *txdmachan, char *rxdmachan); void tegra_pcm_platform_unregister(struct device *dev);
#endif
On Fri, Nov 15, 2013 at 01:54:14PM -0700, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
By passing no flags when calling snd_dmaengine_pcm_register() from tegra_pcm.c, we end up using dma_request_slave_channel() rather than
Acked-by: Mark Brown broonie@linaro.org
On 15.11.2013 22:53, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
This series implements a common reset framework driver for Tegra, and updates all relevant Tegra drivers to use it. It also removes the custom DMA bindings and replaced them with the standard DMA DT bindings.
Historically, the Tegra clock driver has exported a custom API for module reset. This series removes that API, and transitions DT and drivers to the new reset framework.
The custom API used a "struct clk" to identify which module to reset, and consequently some DT bindings and drivers required clocks to be provided where they really needed just a reset identifier instead. Due to this known deficiency, I have always considered most Tegra bindings to be unstable. This series removes this excuse for instability, although I still consider some Tegra bindings unstable due to the need to convert to the common DMA bindings.
Historically, Tegra DMA channels have been represented in DT using a custom nvidia,dma-request-selector property. Now that standard DMA DT bindings exist, convert all Tegra bindings, DTs, and drivers to use the standard instead.
This series makes a DT-ABI-incompatible change to:
- Require reset specifiers in DT where relevant.
- Require standard DMA specifiers.
- Remove clock specifiers from DT where they were only needed for reset.
- Remove legacy DMA specifier properties.
I anticipate merging this whole series into the Tegra and arm-soc trees as its own branch, due to internal dependencies. This branch will be stable and can then be merged into any other subsystem trees should any conflicts arise.
This series depends on Peter's Tegra clock driver rework, available at git://nv-tegra.nvidia.com/user/pdeschrijver/linux tegra-clk-tegra124-0 (or whatever version of that gets included in 3.14)
Overall, a good change. For host1x part:
Acked-By: Terje Bergstrom tbergstrom@nvidia.com
This patch does not change the behavior, but we have in original code the problem that we don't flush the MC queue when resetting an engine. This can cause some memory writes to not hit memory. There was an earlier discussion on that, but we seem to have lost track of the issue.
Terje
On Friday 15 November 2013, Stephen Warren wrote:
This series implements a common reset framework driver for Tegra, and updates all relevant Tegra drivers to use it. It also removes the custom DMA bindings and replaced them with the standard DMA DT bindings.
The series is rather long, so I may have missed it, but I think you need one more patch to the apbdma binding to document the use of #dma-cells, what value it has, and what the format of the dma specifiers in slave drivers needs to be.
Arnd
On 11/20/2013 08:37 AM, Arnd Bergmann wrote:
On Friday 15 November 2013, Stephen Warren wrote:
This series implements a common reset framework driver for Tegra, and updates all relevant Tegra drivers to use it. It also removes the custom DMA bindings and replaced them with the standard DMA DT bindings.
The series is rather long, so I may have missed it, but I think you need one more patch to the apbdma binding to document the use of #dma-cells, what value it has, and what the format of the dma specifiers in slave drivers needs to be.
Yes, you're right. I will fold the following into "ARM: tegra: document use of standard DMA DT bindings":
diff --git a/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt b/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt index 0b1e577ab9d3..0b0f9498e265 100644 --- a/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt +++ b/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt @@ -11,6 +11,10 @@ Required properties: See ../reset/reset.txt for details.
- reset-names : Must include the following entries:
- dma
+- #iommu-cells : Must be <1>. This dictates the length of DMA specifiers in
- client nodes' dmas properties. The specifier represents the DMA request
- select value for the peripheral. For more details, consult the Tegra TRM's
- documentation of the APB DMA channel control register REQ_SEL field.
Examples:
@@ -36,4 +40,5 @@ apbdma: dma@6000a000 { clocks = <&tegra_car 34>; resets = <&tegra_car 34>; reset-names = "dma";
- #iommu-cells = <1>;
};
On Wednesday 20 November 2013, Stephen Warren wrote:
+- #iommu-cells : Must be <1>. This dictates the length of DMA specifiers in
- client nodes' dmas properties. The specifier represents the DMA request
- select value for the peripheral. For more details, consult the Tegra TRM's
- documentation of the APB DMA channel control register REQ_SEL field.
Examples:
@@ -36,4 +40,5 @@ apbdma: dma@6000a000 { clocks = <&tegra_car 34>; resets = <&tegra_car 34>; reset-names = "dma";
#iommu-cells = <1>;
s/iommu/dma/
Otherwise looks good. The dts files are correct, so I guess it's just a typo here.
Arnd
On 11/20/2013 10:03 AM, Arnd Bergmann wrote:
On Wednesday 20 November 2013, Stephen Warren wrote:
+- #iommu-cells : Must be <1>. This dictates the length of DMA specifiers in
- client nodes' dmas properties. The specifier represents the DMA request
- select value for the peripheral. For more details, consult the Tegra TRM's
- documentation of the APB DMA channel control register REQ_SEL field.
Examples:
@@ -36,4 +40,5 @@ apbdma: dma@6000a000 { clocks = <&tegra_car 34>; resets = <&tegra_car 34>; reset-names = "dma";
#iommu-cells = <1>;s/iommu/dma/
Otherwise looks good. The dts files are correct, so I guess it's just a typo here.
Thanks, fixed locally.
Dear all, My ac100 screen is flickering so much. I realized I'm not using it anymore. So if anyone wants to have it for free would be for me a huge pleasure to give it away. I'm based in milan and I'll be in London for the next week. Maybe someone needs it.
Martino
2013/11/20 Stephen Warren swarren@wwwdotorg.org
On 11/20/2013 08:37 AM, Arnd Bergmann wrote:
On Friday 15 November 2013, Stephen Warren wrote:
This series implements a common reset framework driver for Tegra, and updates all relevant Tegra drivers to use it. It also removes the custom DMA bindings and replaced them with the standard DMA DT bindings.
The series is rather long, so I may have missed it, but I think you need
one
more patch to the apbdma binding to document the use of #dma-cells, what value it has, and what the format of the dma specifiers in slave drivers needs to be.
Yes, you're right. I will fold the following into "ARM: tegra: document use of standard DMA DT bindings":
diff --git a/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt
b/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt
index 0b1e577ab9d3..0b0f9498e265 100644 --- a/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt +++ b/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt @@ -11,6 +11,10 @@ Required properties: See ../reset/reset.txt for details.
- reset-names : Must include the following entries:
- dma
+- #iommu-cells : Must be <1>. This dictates the length of DMA
specifiers in
- client nodes' dmas properties. The specifier represents the DMA
request
- select value for the peripheral. For more details, consult the Tegra
TRM's
- documentation of the APB DMA channel control register REQ_SEL field.
Examples:
@@ -36,4 +40,5 @@ apbdma: dma@6000a000 { clocks = <&tegra_car 34>; resets = <&tegra_car 34>; reset-names = "dma";
#iommu-cells = <1>;};
Mailing list: https://launchpad.net/~ac100 Post to : ac100@lists.launchpad.net Unsubscribe : https://launchpad.net/~ac100 More help : https://help.launchpad.net/ListHelp
On 11/15/2013 01:53 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
This series implements a common reset framework driver for Tegra, and updates all relevant Tegra drivers to use it. It also removes the custom DMA bindings and replaced them with the standard DMA DT bindings.
Historically, the Tegra clock driver has exported a custom API for module reset. This series removes that API, and transitions DT and drivers to the new reset framework.
The custom API used a "struct clk" to identify which module to reset, and consequently some DT bindings and drivers required clocks to be provided where they really needed just a reset identifier instead. Due to this known deficiency, I have always considered most Tegra bindings to be unstable. This series removes this excuse for instability, although I still consider some Tegra bindings unstable due to the need to convert to the common DMA bindings.
Historically, Tegra DMA channels have been represented in DT using a custom nvidia,dma-request-selector property. Now that standard DMA DT bindings exist, convert all Tegra bindings, DTs, and drivers to use the standard instead.
This series makes a DT-ABI-incompatible change to:
- Require reset specifiers in DT where relevant.
- Require standard DMA specifiers.
- Remove clock specifiers from DT where they were only needed for reset.
- Remove legacy DMA specifier properties.
I anticipate merging this whole series into the Tegra and arm-soc trees as its own branch, due to internal dependencies. This branch will be stable and can then be merged into any other subsystem trees should any conflicts arise.
This series depends on Peter's Tegra clock driver rework, available at git://nv-tegra.nvidia.com/user/pdeschrijver/linux tegra-clk-tegra124-0 (or whatever version of that gets included in 3.14)
I've applied this series (and pulled in the DMA/ASoC/clk dependencies required) to Tegra's for-3.14/dmas-resets-rework branch.
participants (7)
-
Arnd Bergmann -
Lars-Peter Clausen -
Mark Brown -
Martino Brandolini -
Stephen Warren -
Terje Bergström -
Thierry Reding