ARM: tegra: move to generic DMA DT binding

All Tegra device drivers will soon move to generic DMA device tree bindings. Add the required properties to the Tegra DT files to support that. The legacy property nvidia,dma-request-selector will be removed after all drivers have been converted, in order to maintain bisectability.

Changes:
- Add '#dma-cells' for apbdma nodes
- And properties 'dmas' and 'dma-names' for apbdma client nodes
- update apbdma DT binding doc

Signed-off-by Richard Zhao <[email protected]>

---
 .../devicetree/bindings/dma/tegra20-apbdma.txt     |  1 +
 arch/arm/boot/dts/tegra114.dtsi                    | 27 ++++++++++++++++++++++
 arch/arm/boot/dts/tegra20.dtsi                     | 27 ++++++++++++++++++++++
 arch/arm/boot/dts/tegra30.dtsi                     | 25 ++++++++++++++++++++

(Stripping the Cc list a lot, but adding in the DT binding maintainers)

Quoted 10 lines · Richard Zhao

All Tegra device drivers will soon move to generic DMA device tree bindings. Add the required properties to the Tegra DT files to support that. The legacy property nvidia,dma-request-selector will be removed after all drivers have been converted, in order to maintain bisectability.

Changes:
- Add '#dma-cells' for apbdma nodes
- And properties 'dmas' and 'dma-names' for apbdma client nodes
- update apbdma DT binding doc

Quoted 6 lines

Signed-off-by Richard Zhao <[email protected]>

---
 .../devicetree/bindings/dma/tegra20-apbdma.txt     |  1 +
 arch/arm/boot/dts/tegra114.dtsi                    | 27 ++++++++++++++++++++++
 arch/arm/boot/dts/tegra20.dtsi                     | 27 ++++++++++++++++++++++
 arch/arm/boot/dts/tegra30.dtsi                     | 25 ++++++++++++++++++++

This patch should be split differently.

Patch 1 in the series should include the binding document change and the associated DMA driver change, which will support both the old and new bindings.

Patch 2 should contain the *.dtsi changes.

Quoted 26 lines · Stephen Warren

(Stripping the Cc list a lot, but adding in the DT binding maintainers)

Quoted 10 lines · Richard Zhao

All Tegra device drivers will soon move to generic DMA device tree bindings. Add the required properties to the Tegra DT files to support that. The legacy property nvidia,dma-request-selector will be removed after all drivers have been converted, in order to maintain bisectability.

Changes:
- Add '#dma-cells' for apbdma nodes
- And properties 'dmas' and 'dma-names' for apbdma client nodes
- update apbdma DT binding doc

Quoted 6 lines

Signed-off-by Richard Zhao <[email protected]>

---
 .../devicetree/bindings/dma/tegra20-apbdma.txt     |  1 +
 arch/arm/boot/dts/tegra114.dtsi                    | 27 ++++++++++++++++++++++
 arch/arm/boot/dts/tegra20.dtsi                     | 27 ++++++++++++++++++++++
 arch/arm/boot/dts/tegra30.dtsi                     | 25 ++++++++++++++++++++

This patch should be split differently.

Patch 1 in the series should include the binding document change and the associated DMA driver change, which will support both the old and new bindings.

Quoted 1 line

Patch 2 should contain the *.dtsi changes.

The reason I update apbdma binding doc here is to make it consistent with the mass properties added in dtsi files.
apbdma has less places of changes than dtsi files.

Quoted 21 lines · Richard Zhao

Quoted 20 lines · Stephen Warren

(Stripping the Cc list a lot, but adding in the DT binding maintainers)

Quoted 16 lines · Richard Zhao

All Tegra device drivers will soon move to generic DMA device tree bindings. Add the required properties to the Tegra DT files to support that. The legacy property nvidia,dma-request-selector will be removed after all drivers have been converted, in order to maintain bisectability.

Changes:
- Add '#dma-cells' for apbdma nodes
- And properties 'dmas' and 'dma-names' for apbdma client nodes
- update apbdma DT binding doc

Signed-off-by Richard Zhao <[email protected]>

---
 .../devicetree/bindings/dma/tegra20-apbdma.txt     |  1 +
 arch/arm/boot/dts/tegra114.dtsi                    | 27 ++++++++++++++++++++++
 arch/arm/boot/dts/tegra20.dtsi                     | 27 ++++++++++++++++++++++
 arch/arm/boot/dts/tegra30.dtsi                     | 25 ++++++++++++++++++++

Quoted 5 lines

Quoted 5 lines

This patch should be split differently.

Patch 1 in the series should include the binding document change and the associated DMA driver change, which will support both the old and new bindings.

Quoted 6 lines

Quoted 2 lines

Patch 2 should contain the *.dtsi changes.

The reason I update apbdma binding doc here is to make it consistent with the mass properties added in dtsi files.
apbdma has less places of changes than dtsi files.

It'll still be consistent if you update the binding documentation first, then update the *.dtsi to conform to the new binding docs. It won't affect the number of changes.

Update tegra20-apbdma driver to adopt generic DMA device tree bindings. It calls of_dma_controller_register() with of_dma_simple_xlate to get the generic DMA device tree helper support. The #dma-cells for apbdma must be 1, which is slave ID.

The existing nvidia,dma-request-selector still works there, and the support will be removed after all clients get converted to generic DMA device tree helper.

drivers/dma/tegra20-apb-dma.c +6 -0

 
+static bool tegra_dma_filter_fn(struct dma_chan *dc, void *param)
+{
+	if (dc->device->dev->driver == &tegra_dmac_driver.driver) {
+		struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
+		unsigned req = *(unsigned *)param;
+

(Stripping the Cc list a lot)

Quoted 5 lines · Richard Zhao

Update tegra20-apbdma driver to adopt generic DMA device tree bindings. It calls of_dma_controller_register() with of_dma_simple_xlate to get the generic DMA device tree helper support. The #dma-cells for apbdma must be 1, which is slave ID.

Quoted 3 lines

The existing nvidia,dma-request-selector still works there, and the support will be removed after all clients get converted to generic DMA device tree helper.

(BTW, I would like to take this series through the Tegra tree to simplify dependencies. So, I'm looking for ack's on the drivers rather than for them to be applied. I had hoped Richard would point this out when posting the patches)

Quoted 17 lines

drivers/dma/tegra20-apb-dma.c +14 -0

 
+static bool tegra_dma_filter_fn(struct dma_chan *dc, void *param)
+{
+	if (dc->device->dev->driver == &tegra_dmac_driver.driver) {
+		struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
+		unsigned req = *(unsigned *)param;
+
+		tdc->slave_id = req;
+
+		return true;
+	}
+	return false;
+}
+
+static struct of_dma_filter_info tegra_dma_info = {

drivers/dma/tegra20-apb-dma.c static void tegra_dma_free_chan_resources(struct dma_chan *dc) · line 1239

Quoted 2 lines

drivers/dma/tegra20-apb-dma.c +2 -0

+	.filter_fn = tegra_dma_filter_fn,
+};

Why does this driver need to define a filter function? I thought the dmaengine's DT support would already match DMA clients/engines based on the phandle in DT? Isn't the need to pass the slave channel ID into drivers something pretty fundamental to dmaengine, and hence something it already supports?

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 28 lines · Stephen Warren

(Stripping the Cc list a lot)

Quoted 25 lines · Richard Zhao

Update tegra20-apbdma driver to adopt generic DMA device tree bindings. It calls of_dma_controller_register() with of_dma_simple_xlate to get the generic DMA device tree helper support. The #dma-cells for apbdma must be 1, which is slave ID.

The existing nvidia,dma-request-selector still works there, and the support will be removed after all clients get converted to generic DMA device tree helper.

drivers/dma/tegra20-apb-dma.c +14 -0

 
+static bool tegra_dma_filter_fn(struct dma_chan *dc, void *param)
+{
+	if (dc->device->dev->driver == &tegra_dmac_driver.driver) {
+		struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
+		unsigned req = *(unsigned *)param;
+
+		tdc->slave_id = req;
+
+		return true;
+	}
+	return false;
+}
+
+static struct of_dma_filter_info tegra_dma_info = {

Quoted 8 lines

drivers/dma/tegra20-apb-dma.c static void tegra_dma_free_chan_resources(struct dma_chan *dc) · line 1239

Quoted 2 lines

drivers/dma/tegra20-apb-dma.c +2 -0

+	.filter_fn = tegra_dma_filter_fn,
+};

Why does this driver need to define a filter function? I thought the dmaengine's DT support would already match DMA clients/engines based on the phandle in DT? Isn't the need to pass the slave channel ID into drivers something pretty fundamental to dmaengine, and hence something it already supports?

of_dma_simple_xlate only pass slave id to filter function. It needs change to pass dt node too. I didn't start work on the common code change yet. Do you think we need to hold it until common code change?

Quoted 38 lines · Richard Zhao

Quoted 36 lines · Stephen Warren

(Stripping the Cc list a lot)

Quoted 17 lines · Richard Zhao

Update tegra20-apbdma driver to adopt generic DMA device tree bindings. It calls of_dma_controller_register() with of_dma_simple_xlate to get the generic DMA device tree helper support. The #dma-cells for apbdma must be 1, which is slave ID.

The existing nvidia,dma-request-selector still works there, and the support will be removed after all clients get converted to generic DMA device tree helper.

drivers/dma/tegra20-apb-dma.c +6 -0

 
+static bool tegra_dma_filter_fn(struct dma_chan *dc, void *param)
+{
+	if (dc->device->dev->driver == &tegra_dmac_driver.driver) {
+		struct tegra_dma_channel *tdc = to_tegra_dma_chan(dc);
+		unsigned req = *(unsigned *)param;
+

drivers/dma/tegra20-apb-dma.c static void tegra_dma_free_chan_resources(struct dma_chan *dc) · line 1239

Quoted 10 lines

drivers/dma/tegra20-apb-dma.c +10 -0

+		tdc->slave_id = req;
+
+		return true;
+	}
+	return false;
+}
+
+static struct of_dma_filter_info tegra_dma_info = {
+	.filter_fn = tegra_dma_filter_fn,
+};

Why does this driver need to define a filter function? I thought the dmaengine's DT support would already match DMA clients/engines based on the phandle in DT? Isn't the need to pass the slave channel ID into drivers something pretty fundamental to dmaengine, and hence something it already supports?

Quoted 3 lines

of_dma_simple_xlate only pass slave id to filter function. It needs change to pass dt node too. I didn't start work on the common code change yet. Do you think we need to hold it until common code change?

Oh dear, I can't see how the core DT DMA support can work for anyone without the ability to limit DMA clients to the specific DMA controller that's specified in DT. If that functionality truly is missing, then it certainly needs to be added first.

Quoted 9 lines · Stephen Warren

Quoted 8 lines · Richard Zhao

Quoted 7 lines · Stephen Warren

(Stripping the Cc list a lot)

Quoted 4 lines · Richard Zhao

Update tegra20-apbdma driver to adopt generic DMA device tree bindings. It calls of_dma_controller_register() with of_dma_simple_xlate to get the generic DMA device tree helper support. The #dma-cells for apbdma must be 1, which is slave ID.

Quoted 10 lines

Quoted 9 lines

Quoted 5 lines

Why does this driver need to define a filter function? I thought the dmaengine's DT support would already match DMA clients/engines based on the phandle in DT? Isn't the need to pass the slave channel ID into drivers something pretty fundamental to dmaengine, and hence something it already supports?

of_dma_simple_xlate only pass slave id to filter function. It needs change to pass dt node too. I didn't start work on the common code change yet. Do you think we need to hold it until common code change?

Quoted 4 lines

Oh dear, I can't see how the core DT DMA support can work for anyone without the ability to limit DMA clients to the specific DMA controller that's specified in DT. If that functionality truly is missing, then it certainly needs to be added first.

omap-dma uses the same way.

- driver: remove use of nvidia,dma-request-selector
use dma_request_slave_channel to request channel
- if dmas/dma-names are missing, it still supports cpu based transfer
- update binding doc and specify dmas/dma-names properties as optional

(Stripping CC list a bit)

Quoted 4 lines · Richard Zhao

- driver: remove use of nvidia,dma-request-selector
use dma_request_slave_channel to request channel
- if dmas/dma-names are missing, it still supports cpu based transfer
- update binding doc and specify dmas/dma-names properties as optional

I'll simply repeat the comments I already made downstream:

Most commit messages aren't written as bullet points.

This might be
better written as:

----------
Convert the Tegra114 SPI driver to support the standard DMA device-tree bindings.

Note that DMA mode is optional; if the DT properties required to support DMA are missing, then the driver will fall back to pure CPU operation.
----------

Documentation/devicetree/bindings/spi/nvidia,tegra114-spi.txt Required properties: · line 13

Quoted 6 lines

Documentation/devicetree/bindings/spi/nvidia,tegra114-spi.txt +4 -0

 
+Optional properties:
+- dmas : The Tegra DMA controller's phandle and request selector for
+  this SPI controller.
+- dma-names : Should be "rx-tx".

This should reference dma.txt which defines the DMA DT bindings. How about:

----------
Optional properties:
- dmas, dma-names : The DMA channel to use for DMA-mode operation. See dma.txt for the definition of these properties. A single entry named "rx-tx" must be specified.
----------

Quoted 17 lines · Stephen Warren

(Stripping CC list a bit)

Documentation/devicetree/bindings/spi/nvidia,tegra114-spi.txt Required properties: · line 13

Quoted 11 lines · Richard Zhao

- driver: remove use of nvidia,dma-request-selector
use dma_request_slave_channel to request channel
- if dmas/dma-names are missing, it still supports cpu based transfer
- update binding doc and specify dmas/dma-names properties as optional

Documentation/devicetree/bindings/spi/nvidia,tegra114-spi.txt +4 -0

 
+Optional properties:
+- dmas : The Tegra DMA controller's phandle and request selector for
+  this SPI controller.
+- dma-names : Should be "rx-tx".

This should reference dma.txt which defines the DMA DT bindings. How about:

Quoted 6 lines

----------
Optional properties:
- dmas, dma-names : The DMA channel to use for DMA-mode operation. See dma.txt for the definition of these properties. A single entry named "rx-tx" must be specified.
----------

Ok. but how about replace "DMA channel" with "DMA request"?

update binding doc.

Quoted 1 line · Richard Zhao

update binding doc.

It may be worth mentioning here that the driver isn't updated since it doesn't support DMA operation (yet?). A more complete patch description would also be useful.

- driver: remove use of nvidia,dma-request-selector use dma_request_slave_channel to request channel
- update binding doc

Quoted 3 lines · Richard Zhao

- driver: remove use of nvidia,dma-request-selector use dma_request_slave_channel to request channel
- update binding doc

This patch needs to be amended so that the DMA channel is looked up during probe() rather than open(), to guarantee that deferred probe works. It's possible to hold off probe, but not possible AFAIK to hold off opening the port.

Quoted 3 lines · Richard Zhao

- driver: remove use of nvidia,dma-request-selector use dma_request_slave_channel to request channel
- update binding doc

This patch needs to be amended so that the DMA channel is looked up during probe() rather than open(), to guarantee that deferred probe works. It's possible to hold off probe, but not possible AFAIK to hold off opening the port.

Quoted 5 lines · Stephen Warren

Quoted 3 lines · Richard Zhao

- driver: remove use of nvidia,dma-request-selector use dma_request_slave_channel to request channel
- update binding doc

Quoted 4 lines

This patch needs to be amended so that the DMA channel is looked up during probe() rather than open(), to guarantee that deferred probe works. It's possible to hold off probe, but not possible AFAIK to hold off opening the port.

How about avoid return -EPROBE_DEFER at open() time? It can be in another patch.

Quoted 11 lines · Richard Zhao

Quoted 9 lines · Stephen Warren

Quoted 3 lines · Richard Zhao

- driver: remove use of nvidia,dma-request-selector use dma_request_slave_channel to request channel
- update binding doc

This patch needs to be amended so that the DMA channel is looked up during probe() rather than open(), to guarantee that deferred probe works. It's possible to hold off probe, but not possible AFAIK to hold off opening the port.

Quoted 2 lines

How about avoid return -EPROBE_DEFER at open() time? It can be in another patch.

I don't really understand what you mean. -EPROBE_DEFER is something that only makes sense for probe() to return (or functions used by probe()). It doesn't make sense to return it anywhere else, since probe deferral is something that can only happen for probe().

Quoted 15 lines · Stephen Warren

Quoted 13 lines · Richard Zhao

Quoted 9 lines · Stephen Warren

Quoted 3 lines · Richard Zhao

- driver: remove use of nvidia,dma-request-selector use dma_request_slave_channel to request channel
- update binding doc

This patch needs to be amended so that the DMA channel is looked up during probe() rather than open(), to guarantee that deferred probe works. It's possible to hold off probe, but not possible AFAIK to hold off opening the port.

How about avoid return -EPROBE_DEFER at open() time? It can be in another patch.

Quoted 4 lines

I don't really understand what you mean. -EPROBE_DEFER is something that only makes sense for probe() to return (or functions used by probe()). It doesn't make sense to return it anywhere else, since probe deferral is something that can only happen for probe().

What I meant is, it might not worth to move request channel to probe only because we need to check error and return -EPROBE_DEFER.

dma_request_slave_channel is better to be called as late as possible to make better dynamic use of dma channels.

Actually, I searched the kernel code, most drivers don't return EPROBE_DEFER when dma_request_slave_channel failed. dma_request_slave_channel can fail in many cases, and dma device not probed is only one case.

Quoted 21 lines · Richard Zhao

Quoted 19 lines · Stephen Warren

Quoted 6 lines · Richard Zhao

Quoted 5 lines · Stephen Warren

Quoted 3 lines · Richard Zhao

- driver: remove use of nvidia,dma-request-selector use dma_request_slave_channel to request channel
- update binding doc

Quoted 7 lines

Quoted 4 lines

This patch needs to be amended so that the DMA channel is looked up during probe() rather than open(), to guarantee that deferred probe works. It's possible to hold off probe, but not possible AFAIK to hold off opening the port.

How about avoid return -EPROBE_DEFER at open() time? It can be in another patch.

I don't really understand what you mean. -EPROBE_DEFER is something that only makes sense for probe() to return (or functions used by probe()). It doesn't make sense to return it anywhere else, since probe deferral is something that can only happen for probe().

Quoted 2 lines

What I meant is, it might not worth to move request channel to probe only because we need to check error and return -EPROBE_DEFER.

If probe() doesn't attempt to validate that the/a DMA channel provider exists, then there's no guarantee that the DMA provider has probed before the DMA client driver either probes or executes any later code.

So, you could quite easily end up with a situation where the serial driver's open fails because the DMA provider has not yet probed. This is exactly what deferred probe was intended to avoid. This reason alone is enough to make it worth fixing this.

Quoted 2 lines

dma_request_slave_channel is better to be called as late as possible to make better dynamic use of dma channels.

probe() could validate that the provider exists in a way that doesn't permanently hold ownership of the DMA channel. For example, request it then immediately release it, in the absence of a dedicated "validate my DMA providers exist" API.

Quoted 4 lines

Actually, I searched the kernel code, most drivers don't return EPROBE_DEFER when dma_request_slave_channel failed. dma_request_slave_channel can fail in many cases, and dma device not probed is only one case.

That sounds like a bug. If a DMA channel /is/ specified, it should be possible to acquire it. If no DMA channel is specified, and if the device can do PIO, then it's reasonable to continue.

Quoted 22 lines · Stephen Warren

Quoted 21 lines · Richard Zhao

Quoted 19 lines · Stephen Warren

Quoted 6 lines · Richard Zhao

Quoted 5 lines · Stephen Warren

Quoted 3 lines · Richard Zhao

- driver: remove use of nvidia,dma-request-selector use dma_request_slave_channel to request channel
- update binding doc

Quoted 7 lines

Quoted 4 lines

This patch needs to be amended so that the DMA channel is looked up during probe() rather than open(), to guarantee that deferred probe works. It's possible to hold off probe, but not possible AFAIK to hold off opening the port.

How about avoid return -EPROBE_DEFER at open() time? It can be in another patch.

I don't really understand what you mean. -EPROBE_DEFER is something that only makes sense for probe() to return (or functions used by probe()). It doesn't make sense to return it anywhere else, since probe deferral is something that can only happen for probe().

Quoted 6 lines

Quoted 2 lines

What I meant is, it might not worth to move request channel to probe only because we need to check error and return -EPROBE_DEFER.

If probe() doesn't attempt to validate that the/a DMA channel provider exists, then there's no guarantee that the DMA provider has probed before the DMA client driver either probes or executes any later code.

I'm not against defer probe, but dma_request_slave_channel might not be the right function. Ideally dma channels are allocated at open() time and dma controller loaded checking is at probe() time. So there's better some function called check_dma_controller_loaded(), or we could also use initcalls to let dma driver probe earlier.

This patch intend to be simple to only change the binding.

Quoted 8 lines

So, you could quite easily end up with a situation where the serial driver's open fails because the DMA provider has not yet probed. This is exactly what deferred probe was intended to avoid. This reason alone is enough to make it worth fixing this.

Quoted 2 lines

dma_request_slave_channel is better to be called as late as possible to make better dynamic use of dma channels.

Quoted 4 lines

probe() could validate that the provider exists in a way that doesn't permanently hold ownership of the DMA channel. For example, request it then immediately release it, in the absence of a dedicated "validate my DMA providers exist" API.

It's ugly but may work. The current driver didn't check dma controller loaded at probe time. Let's leave the task to another patch?

- add tegra_dma_filter_data to specify dma info
DMA DT binding needs the device that raise dma request and dma name to request a dma channel. tegra30_i2s is a special case. It should be ahub device and it also has dma name that cannot handled by ASoC dmaengine code. So we pass the info using filter data in snd_dmaengine_dai_dma_data.

sound/soc/tegra/tegra_pcm.h +2 -0

 
+#define TEGRA_DMA_NAME_MAX_LEN	10
+

Quoted 7 lines · Richard Zhao

- add tegra_dma_filter_data to specify dma info
DMA DT binding needs the device that raise dma request and dma name to request a dma channel. tegra30_i2s is a special case. It should be ahub device and it also has dma name that cannot handled by ASoC dmaengine code. So we pass the info using filter data in snd_dmaengine_dai_dma_data.
- change i2s/ac97 drivers to use generic DT binding
- tegra30_i2s: alloc ahub tx/rx fifo at driver probe time

sound/soc/tegra/tegra30_i2s.c

Quoted 1 line

sound/soc/tegra/tegra30_i2s.c +0 -0

I don't understand why the FIFO alloc/free had to move from startup()/shutdown() to probe()/remove().

This will prevent later
changes from making the connectivity between the AHUB FIFOs and I2S modules more dynamic, which is kind of the whole point of the AHUB HW module.

Quoted 5 lines

sound/soc/tegra/tegra_pcm.h +2 -0

 
+#define TEGRA_DMA_NAME_MAX_LEN	10
+

sound/soc/tegra/tegra_pcm.h line 45

Quoted 10 lines

sound/soc/tegra/tegra_pcm.h +10 -0

+/*
+ * Generic DMA DT binding needs the device that raise dma request and dma name
+ * to request a dma channel. tegra30_i2s is a special case. It should be ahub
+ * device and it also has dma name that cannot handled by ASoC dmaengine code.
+ * So we pass the info using filter data in snd_dmaengine_dai_dma_data.
+ */
+struct tegra_dma_filter_data {
+	struct device *dma_dev;
+	char dma_name[TEGRA_DMA_NAME_MAX_LEN];
+};

I agree with Lars-Peter's comment that it'd be better to enhance the ASoC dmaengine support to allow the channel name to be specified so we don't need to much custom code.

And that should use "char *" not a
fixed-length array.

Quoted 9 lines · Stephen Warren

Quoted 7 lines · Richard Zhao

- add tegra_dma_filter_data to specify dma info
DMA DT binding needs the device that raise dma request and dma name to request a dma channel. tegra30_i2s is a special case. It should be ahub device and it also has dma name that cannot handled by ASoC dmaengine code. So we pass the info using filter data in snd_dmaengine_dai_dma_data.
- change i2s/ac97 drivers to use generic DT binding
- tegra30_i2s: alloc ahub tx/rx fifo at driver probe time

Quoted 4 lines

sound/soc/tegra/tegra30_i2s.c

Quoted 1 line

sound/soc/tegra/tegra30_i2s.c +0 -0

I don't understand why the FIFO alloc/free had to move from startup()/shutdown() to probe()/remove().

The call path is:
[<c0520e38>] (dump_stack+0x78/0xbc) from [<c03bb854>] (tegra_pcm_request_chan+0x10/0x34) [<c03bb854>] (tegra_pcm_request_chan+0x10/0x34) from [<c03b6c2c>] (dmaengine_pcm_new+0xe0/0x134) [<c03b6c2c>] (dmaengine_pcm_new+0xe0/0x134) from [<c03b5824>] (soc_new_pcm+0x2f0/0x3cc) [<c03b5824>] (soc_new_pcm+0x2f0/0x3cc) from [<c03aa1a0>] (soc_probe_link_dais+0x358/0x39c) [<c03aa1a0>] (soc_probe_link_dais+0x358/0x39c) from [<c03ab334>] (snd_soc_instantiate_card+0x35c/0x7a0) [<c03ab334>] (snd_soc_instantiate_card+0x35c/0x7a0) from [<c03ab9dc>] (snd_soc_register_card+0x264/0x330) [<c03ab9dc>] (snd_soc_register_card+0x264/0x330) from [<c03be350>] (tegra_rt5640_probe+0xf4/0x17c) [<c03be350>] (tegra_rt5640_probe+0xf4/0x17c) from [<c0294290>] (platform_drv_probe+0x18/0x1c)

So I need to request dma channel at probe time.

Quoted 4 lines

This will prevent later
changes from making the connectivity between the AHUB FIFOs and I2S modules more dynamic, which is kind of the whole point of the AHUB HW module.

That's because it's i2s driver to register pcm device rather not ahub. Ideally dma channel is only bound to ahub.

Quoted 18 lines

Quoted 4 lines

sound/soc/tegra/tegra_pcm.h +2 -0

 
+#define TEGRA_DMA_NAME_MAX_LEN	10
+

sound/soc/tegra/tegra_pcm.h line 45

Quoted 10 lines

sound/soc/tegra/tegra_pcm.h +10 -0

+/*
+ * Generic DMA DT binding needs the device that raise dma request and dma name
+ * to request a dma channel. tegra30_i2s is a special case. It should be ahub
+ * device and it also has dma name that cannot handled by ASoC dmaengine code.
+ * So we pass the info using filter data in snd_dmaengine_dai_dma_data.
+ */
+struct tegra_dma_filter_data {
+	struct device *dma_dev;
+	char dma_name[TEGRA_DMA_NAME_MAX_LEN];
+};

I agree with Lars-Peter's comment that it'd be better to enhance the ASoC dmaengine support to allow the channel name to be specified so we don't need to much custom code.

Not only dma-name but also dma client device for this case.

Quoted 2 lines

And that should use "char *" not a
fixed-length array.

Ok, then there' two solutions.

struct tegra30_i2s {
...

        struct snd_dmaengine_dai_dma_data capture_dma_data;
        char dma_rx_name[10];
        struct tegra_dma_filter_data filter_data_rx;
...
        struct snd_dmaengine_dai_dma_data playback_dma_data;
        char dma_tx_name[10];
        struct tegra_dma_filter_data filter_data_tx;
};

probe: i2s->filter_data_rx.dma_name = i2s.dma_rx_name

or

probe: i2s->filter_data_rx.dma_name = kasprintf(GFP_KERNEL, "channel%d" ...);
remove: kfree(i2s->filter_data_rx.dma.name)

Which one do you like?

All tegra dma client drivers have moved to generic dma binding.

Quoted 1 line · Richard Zhao

All tegra dma client drivers have moved to generic dma binding.

Nit: Tegra not tegra, DMA not dma.

The patch series aim to move apbdma to generic DT binding.

Changes:
- It add dmas/dma-names properties while leave nvidia,dma-request-selector to be compatible.
- update apbdma driver and dma client drivers
- ASoC tegra adds a new struct to pass device and dma-name.
- at last, remove legacy nvidia,dma-request-selector