diff mbox

[v3] dmaengine: sirf: enable generic dt binding for dma channels

Message ID 1389007012-10887-1-git-send-email-21cnbao@gmail.com (mailing list archive)
State Superseded
Delegated to: Vinod Koul
Headers show

Commit Message

Barry Song Jan. 6, 2014, 11:16 a.m. UTC
From: Barry Song <Baohua.Song@csr.com>

move to support of_dma_request_slave_channel() and dma_request_slave_channel.
we add a xlate() to let dma clients be able to find right dma_chan by generic
"dmas" properties in dts.

Cc: Lars-Peter Clausen <lars@metafoo.de>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 -v3:
 drop unused variant cap pointed out by Lars-Peter;
 add dt-binding document

 .../devicetree/bindings/dma/sirfsoc-dma.txt        |   42 ++++++++++++++++++++
 arch/arm/boot/dts/atlas6.dtsi                      |    4 ++
 arch/arm/boot/dts/prima2.dtsi                      |    4 ++
 drivers/dma/sirf-dma.c                             |   23 +++++++++++
 4 files changed, 73 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/dma/sirfsoc-dma.txt

Comments

Mark Rutland Jan. 6, 2014, 3:09 p.m. UTC | #1
On Mon, Jan 06, 2014 at 11:16:52AM +0000, Barry Song wrote:
> From: Barry Song <Baohua.Song@csr.com>
> 
> move to support of_dma_request_slave_channel() and dma_request_slave_channel.
> we add a xlate() to let dma clients be able to find right dma_chan by generic
> "dmas" properties in dts.
> 
> Cc: Lars-Peter Clausen <lars@metafoo.de>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
>  -v3:
>  drop unused variant cap pointed out by Lars-Peter;
>  add dt-binding document
> 
>  .../devicetree/bindings/dma/sirfsoc-dma.txt        |   42 ++++++++++++++++++++
>  arch/arm/boot/dts/atlas6.dtsi                      |    4 ++
>  arch/arm/boot/dts/prima2.dtsi                      |    4 ++
>  drivers/dma/sirf-dma.c                             |   23 +++++++++++
>  4 files changed, 73 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/dma/sirfsoc-dma.txt
> 
> diff --git a/Documentation/devicetree/bindings/dma/sirfsoc-dma.txt b/Documentation/devicetree/bindings/dma/sirfsoc-dma.txt
> new file mode 100644
> index 0000000..262cf8c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/sirfsoc-dma.txt
> @@ -0,0 +1,42 @@
> +* CSR SiRFSoC DMA controller
> +
> +See dma.txt first
> +
> +Required properties:
> +- compatible: Should be "sirf,prima2-dmac" or "sirf,marco-dmac"
> +- reg: Should contain DMA registers location and length.
> +- interrupts: Should contain one interrupt shared by all channel
> +- #dma-cells: see dma.txt, should be 1, para number

para number?

> +- dma-channels: physical channels supported
> +- clocks: clock required
> +
> +Example:
> +
> +Controller:
> +dmac0: dma-controller@b00b0000 {
> +	compatible = "sirf,prima2-dmac";
> +	reg = <0xb00b0000 0x10000>;
> +	interrupts = <12>;
> +	clocks = <&clks 24>;
> +};

#dma-cells and dma-channels seem to be missing.

> +
> +
> +Client:
> +Use specific request line passing from dmax
> +For example, spi0 read channel request line is 9 of the 2nd dma controller, while
> +write channel uses 4 of the 2nd dma controller; spi1 read channel request line is
> +12 of the 1st dma controller, while write channel uses 13 of the 1st dma controller.

I think this should be described in the #dma-cells description, as this
described the meaning of the single dma cell.

> +
> +spi0: spi@b00d0000 {
> +	compatible = "sirf,prima2-spi";
> +	dmas = <&dmac1 9>,
> +		<&dmac1 4>;
> +	dma-names = "rx", "tx";
> +};
> +
> +spi1: spi@b0170000 {
> +	compatible = "sirf,prima2-spi";
> +	dmas = <&dmac0 12>,
> +		<&dmac0 13>;
> +	dma-names = "rx", "tx";
> +};
> diff --git a/arch/arm/boot/dts/atlas6.dtsi b/arch/arm/boot/dts/atlas6.dtsi
> index b63cfef..aa3ff43 100644
> --- a/arch/arm/boot/dts/atlas6.dtsi
> +++ b/arch/arm/boot/dts/atlas6.dtsi
> @@ -260,6 +260,8 @@
>  				reg = <0xb00b0000 0x10000>;
>  				interrupts = <12>;
>  				clocks = <&clks 24>;
> +				#dma-cells = <1>;
> +				#dma-channels = <16>;

is it dma-channels or #dma-channels? The document and examples differ.

[...]

> +static struct dma_chan *of_dma_sirfsoc_xlate(struct of_phandle_args *dma_spec,
> +	struct of_dma *ofdma)
> +{
> +	struct sirfsoc_dma *sdma = ofdma->of_dma_data;
> +	unsigned int request = dma_spec->args[0];
> +
> +	if (request > SIRFSOC_DMA_CHANNELS)
> +		return NULL;
> +
> +	return dma_get_slave_channel(&(sdma->channels[request].chan));

Are the internal brackets necessary? "&" is lower precendence than "->",
"[]", and ".".

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Barry Song Jan. 7, 2014, 2:19 a.m. UTC | #2
Hi Mark,
thanks!

2014/1/6 Mark Rutland <mark.rutland@arm.com>:
> On Mon, Jan 06, 2014 at 11:16:52AM +0000, Barry Song wrote:
>> From: Barry Song <Baohua.Song@csr.com>
>>
>> move to support of_dma_request_slave_channel() and dma_request_slave_channel.
>> we add a xlate() to let dma clients be able to find right dma_chan by generic
>> "dmas" properties in dts.
>>
>> Cc: Lars-Peter Clausen <lars@metafoo.de>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>>  -v3:
>>  drop unused variant cap pointed out by Lars-Peter;
>>  add dt-binding document
>>
>>  .../devicetree/bindings/dma/sirfsoc-dma.txt        |   42 ++++++++++++++++++++
>>  arch/arm/boot/dts/atlas6.dtsi                      |    4 ++
>>  arch/arm/boot/dts/prima2.dtsi                      |    4 ++
>>  drivers/dma/sirf-dma.c                             |   23 +++++++++++
>>  4 files changed, 73 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/dma/sirfsoc-dma.txt
>>
>> diff --git a/Documentation/devicetree/bindings/dma/sirfsoc-dma.txt b/Documentation/devicetree/bindings/dma/sirfsoc-dma.txt
>> new file mode 100644
>> index 0000000..262cf8c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/dma/sirfsoc-dma.txt
>> @@ -0,0 +1,42 @@
>> +* CSR SiRFSoC DMA controller
>> +
>> +See dma.txt first
>> +
>> +Required properties:
>> +- compatible: Should be "sirf,prima2-dmac" or "sirf,marco-dmac"
>> +- reg: Should contain DMA registers location and length.
>> +- interrupts: Should contain one interrupt shared by all channel
>> +- #dma-cells: see dma.txt, should be 1, para number
>
> para number?

i copied the document from k3dma.txt and changed for sirfsoc. so
missed this issue as well:

# grep "para number" *
k3dma.txt:- #dma-cells: see dma.txt, should be 1, para number

>
>> +- dma-channels: physical channels supported
>> +- clocks: clock required
>> +
>> +Example:
>> +
>> +Controller:
>> +dmac0: dma-controller@b00b0000 {
>> +     compatible = "sirf,prima2-dmac";
>> +     reg = <0xb00b0000 0x10000>;
>> +     interrupts = <12>;
>> +     clocks = <&clks 24>;
>> +};
>
> #dma-cells and dma-channels seem to be missing.

real. #dma-cells is missed.

>
>> +
>> +
>> +Client:
>> +Use specific request line passing from dmax
>> +For example, spi0 read channel request line is 9 of the 2nd dma controller, while
>> +write channel uses 4 of the 2nd dma controller; spi1 read channel request line is
>> +12 of the 1st dma controller, while write channel uses 13 of the 1st dma controller.
>
> I think this should be described in the #dma-cells description, as this
> described the meaning of the single dma cell.

here it is explaining how to fill the client even though it is mapping
with the #dma-cells of host.

>
>> +
>> +spi0: spi@b00d0000 {
>> +     compatible = "sirf,prima2-spi";
>> +     dmas = <&dmac1 9>,
>> +             <&dmac1 4>;
>> +     dma-names = "rx", "tx";
>> +};
>> +
>> +spi1: spi@b0170000 {
>> +     compatible = "sirf,prima2-spi";
>> +     dmas = <&dmac0 12>,
>> +             <&dmac0 13>;
>> +     dma-names = "rx", "tx";
>> +};
>> diff --git a/arch/arm/boot/dts/atlas6.dtsi b/arch/arm/boot/dts/atlas6.dtsi
>> index b63cfef..aa3ff43 100644
>> --- a/arch/arm/boot/dts/atlas6.dtsi
>> +++ b/arch/arm/boot/dts/atlas6.dtsi
>> @@ -260,6 +260,8 @@
>>                               reg = <0xb00b0000 0x10000>;
>>                               interrupts = <12>;
>>                               clocks = <&clks 24>;
>> +                             #dma-cells = <1>;
>> +                             #dma-channels = <16>;
>
> is it dma-channels or #dma-channels? The document and examples differ.

i realized i don't need dma-channels or #dma-channels at all as in
sirfsoc, the number of channels of every dma controller is dedicated
to SIRFSOC_DMA_CHANNELS(16). so i will drop the stuff.

>
> [...]
>
>> +static struct dma_chan *of_dma_sirfsoc_xlate(struct of_phandle_args *dma_spec,
>> +     struct of_dma *ofdma)
>> +{
>> +     struct sirfsoc_dma *sdma = ofdma->of_dma_data;
>> +     unsigned int request = dma_spec->args[0];
>> +
>> +     if (request > SIRFSOC_DMA_CHANNELS)
>> +             return NULL;
>> +
>> +     return dma_get_slave_channel(&(sdma->channels[request].chan));
>
> Are the internal brackets necessary? "&" is lower precendence than "->",
> "[]", and ".".

it is not a big issue. it seems more readable with this bracket.

>
> Thanks,
> Mark.

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

Patch

diff --git a/Documentation/devicetree/bindings/dma/sirfsoc-dma.txt b/Documentation/devicetree/bindings/dma/sirfsoc-dma.txt
new file mode 100644
index 0000000..262cf8c
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/sirfsoc-dma.txt
@@ -0,0 +1,42 @@ 
+* CSR SiRFSoC DMA controller
+
+See dma.txt first
+
+Required properties:
+- compatible: Should be "sirf,prima2-dmac" or "sirf,marco-dmac"
+- reg: Should contain DMA registers location and length.
+- interrupts: Should contain one interrupt shared by all channel
+- #dma-cells: see dma.txt, should be 1, para number
+- dma-channels: physical channels supported
+- clocks: clock required
+
+Example:
+
+Controller:
+dmac0: dma-controller@b00b0000 {
+	compatible = "sirf,prima2-dmac";
+	reg = <0xb00b0000 0x10000>;
+	interrupts = <12>;
+	clocks = <&clks 24>;
+};
+
+
+Client:
+Use specific request line passing from dmax
+For example, spi0 read channel request line is 9 of the 2nd dma controller, while
+write channel uses 4 of the 2nd dma controller; spi1 read channel request line is
+12 of the 1st dma controller, while write channel uses 13 of the 1st dma controller.
+
+spi0: spi@b00d0000 {
+	compatible = "sirf,prima2-spi";
+	dmas = <&dmac1 9>,
+		<&dmac1 4>;
+	dma-names = "rx", "tx";
+};
+
+spi1: spi@b0170000 {
+	compatible = "sirf,prima2-spi";
+	dmas = <&dmac0 12>,
+		<&dmac0 13>;
+	dma-names = "rx", "tx";
+};
diff --git a/arch/arm/boot/dts/atlas6.dtsi b/arch/arm/boot/dts/atlas6.dtsi
index b63cfef..aa3ff43 100644
--- a/arch/arm/boot/dts/atlas6.dtsi
+++ b/arch/arm/boot/dts/atlas6.dtsi
@@ -260,6 +260,8 @@ 
 				reg = <0xb00b0000 0x10000>;
 				interrupts = <12>;
 				clocks = <&clks 24>;
+				#dma-cells = <1>;
+				#dma-channels = <16>;
 			};
 
 			dmac1: dma-controller@b0160000 {
@@ -268,6 +270,8 @@ 
 				reg = <0xb0160000 0x10000>;
 				interrupts = <13>;
 				clocks = <&clks 25>;
+				#dma-cells = <1>;
+				#dma-channels = <16>;
 			};
 
 			vip@b00C0000 {
diff --git a/arch/arm/boot/dts/prima2.dtsi b/arch/arm/boot/dts/prima2.dtsi
index b292a5c..e543aba 100644
--- a/arch/arm/boot/dts/prima2.dtsi
+++ b/arch/arm/boot/dts/prima2.dtsi
@@ -277,6 +277,8 @@ 
 				reg = <0xb00b0000 0x10000>;
 				interrupts = <12>;
 				clocks = <&clks 24>;
+				#dma-cells = <1>;
+				#dma-channels = <16>;
 			};
 
 			dmac1: dma-controller@b0160000 {
@@ -285,6 +287,8 @@ 
 				reg = <0xb0160000 0x10000>;
 				interrupts = <13>;
 				clocks = <&clks 25>;
+				#dma-cells = <1>;
+				#dma-channels = <16>;
 			};
 
 			vip@b00C0000 {
diff --git a/drivers/dma/sirf-dma.c b/drivers/dma/sirf-dma.c
index 6aec3ad..ab32bbb 100644
--- a/drivers/dma/sirf-dma.c
+++ b/drivers/dma/sirf-dma.c
@@ -18,6 +18,7 @@ 
 #include <linux/of_device.h>
 #include <linux/of_platform.h>
 #include <linux/clk.h>
+#include <linux/of_dma.h>
 #include <linux/sirfsoc_dma.h>
 
 #include "dmaengine.h"
@@ -640,6 +641,18 @@  bool sirfsoc_dma_filter_id(struct dma_chan *chan, void *chan_id)
 }
 EXPORT_SYMBOL(sirfsoc_dma_filter_id);
 
+static struct dma_chan *of_dma_sirfsoc_xlate(struct of_phandle_args *dma_spec,
+	struct of_dma *ofdma)
+{
+	struct sirfsoc_dma *sdma = ofdma->of_dma_data;
+	unsigned int request = dma_spec->args[0];
+
+	if (request > SIRFSOC_DMA_CHANNELS)
+		return NULL;
+
+	return dma_get_slave_channel(&(sdma->channels[request].chan));
+}
+
 static int sirfsoc_dma_probe(struct platform_device *op)
 {
 	struct device_node *dn = op->dev.of_node;
@@ -744,11 +757,20 @@  static int sirfsoc_dma_probe(struct platform_device *op)
 	if (ret)
 		goto free_irq;
 
+	/* Device-tree DMA controller registration */
+	ret = of_dma_controller_register(dn, of_dma_sirfsoc_xlate, sdma);
+	if (ret) {
+		dev_err(dev, "failed to register DMA controller\n");
+		goto unreg_dma_dev;
+	}
+
 	pm_runtime_enable(&op->dev);
 	dev_info(dev, "initialized SIRFSOC DMAC driver\n");
 
 	return 0;
 
+unreg_dma_dev:
+	dma_async_device_unregister(dma);
 free_irq:
 	free_irq(sdma->irq, sdma);
 irq_dispose:
@@ -761,6 +783,7 @@  static int sirfsoc_dma_remove(struct platform_device *op)
 	struct device *dev = &op->dev;
 	struct sirfsoc_dma *sdma = dev_get_drvdata(dev);
 
+	of_dma_controller_free(op->dev.of_node);
 	dma_async_device_unregister(&sdma->dma);
 	free_irq(sdma->irq, sdma);
 	irq_dispose_mapping(sdma->irq);