diff mbox

[PATCH_V3,1/3] dt-bindings: dma: Add binding for jz4780-dma

Message ID 1424954614-12589-2-git-send-email-Zubair.Kakakhel@imgtec.com
State Accepted, archived
Commit c8307106f5fa53b8fe8763b488d629e3cce9fae3
Headers show

Commit Message

Zubair Lutfullah Kakakhel Feb. 26, 2015, 12:43 p.m. UTC
From: Alex Smith <alex.smith@imgtec.com>

Add device tree bindings for the DMA controller on JZ4780 SoCs, used by
the dma-jz4780 driver.

Signed-off-by: Alex Smith <alex.smith@imgtec.com>
Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>

---
V3 -> V2
Changed binding.
Used to be 3 DMA cells required. <&dma TX_type RX_type Reserved>
Now 2 DMA cells are required. <&dma Transfer_type Reserved>

This is more common in DMA bindings.
And I couldn't figure any reason that 3 cells were used.

V1 -> V2 None
---
 .../devicetree/bindings/dma/jz4780-dma.txt         | 56 ++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/jz4780-dma.txt

Comments

Alex Smith Feb. 26, 2015, 8:04 p.m. UTC | #1
Hi Zubair,

On 26 February 2015 at 12:43, Zubair Lutfullah Kakakhel
<Zubair.Kakakhel@imgtec.com> wrote:
> From: Alex Smith <alex.smith@imgtec.com>
>
> Add device tree bindings for the DMA controller on JZ4780 SoCs, used by
> the dma-jz4780 driver.
>
> Signed-off-by: Alex Smith <alex.smith@imgtec.com>
> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
>
> ---
> V3 -> V2
> Changed binding.
> Used to be 3 DMA cells required. <&dma TX_type RX_type Reserved>
> Now 2 DMA cells are required. <&dma Transfer_type Reserved>
>
> This is more common in DMA bindings.
> And I couldn't figure any reason that 3 cells were used.

There are different request type numbers for transfers to/from the
same device (see the JZ4780 programmers manual, page 505). While only
having the option to specify one transfer type is OK when the driver
is using separate channels for read/writes, I recall seeing/writing
some other drivers which use a single channel for both reads and
writes. This would not be possible if we can only specify one transfer
type, you'd have to have them separate.

Thanks,
Alex

>
> V1 -> V2 None
> ---
>  .../devicetree/bindings/dma/jz4780-dma.txt         | 56 ++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/jz4780-dma.txt
>
> diff --git a/Documentation/devicetree/bindings/dma/jz4780-dma.txt b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
> new file mode 100644
> index 0000000..f25feee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
> @@ -0,0 +1,56 @@
> +* Ingenic JZ4780 DMA Controller
> +
> +Required properties:
> +
> +- compatible: Should be "ingenic,jz4780-dma"
> +- reg: Should contain the DMA controller registers location and length.
> +- interrupts: Should contain the interrupt specifier of the DMA controller.
> +- interrupt-parent: Should be the phandle of the interrupt controller that
> +- clocks: Should contain a clock specifier for the JZ4780 PDMA clock.
> +- #dma-cells: Must be <2>. Number of integer cells in the dmas property of
> +  DMA clients (see below).
> +
> +Optional properties:
> +
> +- ingenic,reserved-channels: Bitmask of channels to reserve for devices that
> +  need a specific channel. These channels will only be assigned when explicitly
> +  requested by a client. The primary use for this is channels 0 and 1, which
> +  can be configured to have special behaviour for NAND/BCH when using
> +  programmable firmware.
> +
> +Example:
> +
> +dma: dma@13420000 {
> +       compatible = "ingenic,jz4780-dma";
> +       reg = <0x13420000 0x10000>;
> +
> +       interrupt-parent = <&intc>;
> +       interrupts = <10>;
> +
> +       clocks = <&cgu JZ4780_CLK_PDMA>;
> +
> +       #dma-cells = <2>;
> +
> +       ingenic,reserved-channels = <0x3>;
> +};
> +
> +DMA clients must use the format described in dma.txt, giving a phandle to the
> +DMA controller plus the following 2 integer cells:
> +
> +1. Request type: The DMA request type for transfers to/from the device on
> +   the allocated channel, as defined in the SoC documentation.
> +
> +2. Channel: If set to 0xffffffff, any available channel will be allocated for
> +   the client. Otherwise, the exact channel specified will be used. The channel
> +   should be reserved on the DMA controller using the ingenic,reserved-channels
> +   property.
> +
> +Example:
> +
> +uart0: serial@10030000 {
> +       ...
> +       dmas = <&dma 0x14 0xffffffff
> +               &dma 0x15 0xffffffff>;
> +       dma-names = "tx", "rx";
> +       ...
> +};
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zubair Lutfullah Kakakhel Feb. 27, 2015, 9:20 a.m. UTC | #2
Hi Alex,

On 26/02/15 20:04, Alex Smith wrote:
> Hi Zubair,
> 
> On 26 February 2015 at 12:43, Zubair Lutfullah Kakakhel
> <Zubair.Kakakhel@imgtec.com> wrote:
>> From: Alex Smith <alex.smith@imgtec.com>
>>
>> Add device tree bindings for the DMA controller on JZ4780 SoCs, used by
>> the dma-jz4780 driver.
>>
>> Signed-off-by: Alex Smith <alex.smith@imgtec.com>
>> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
>>
>> ---
>> V3 -> V2
>> Changed binding.
>> Used to be 3 DMA cells required. <&dma TX_type RX_type Reserved>
>> Now 2 DMA cells are required. <&dma Transfer_type Reserved>
>>
>> This is more common in DMA bindings.
>> And I couldn't figure any reason that 3 cells were used.
> 
> There are different request type numbers for transfers to/from the
> same device (see the JZ4780 programmers manual, page 505). While only
> having the option to specify one transfer type is OK when the driver
> is using separate channels for read/writes, I recall seeing/writing
> some other drivers which use a single channel for both reads and
> writes. This would not be possible if we can only specify one transfer
> type, you'd have to have them separate.
> 

I know. I looked at the drivers and did this on purpose.

We'd like to keep the same bindings/code for jz4740/jz4780 peripheral drivers and dma code.

Our jz47xx-mmc driver we had was the main culprit I found. As well as the jz4740-i2s one.

However, jz4740-mmc upstream driver has improved already for dma and takes two dma channels.
And the jz4740-i2s also takes two channels. One for Tx and one for Rx.

If we move to 3 cells for jz4780-dma. Then 'ideally' jz4740-dma would need 3 cells too.
Or we'd have a binding nightmare everywhere.

But when we use jz4740-mmc and jz4740-i2s, we still have to change the driver to share one channel 
or simply pass them
<&dma Tx 0 Reserved>
<&dma 0 Rx Reserved>

Which makes the binding redundant.

There isn't any particular reason a driver would need to share one channel only.
The 'special' nand/nemc channels don't have a request type.

Thanks,
ZubairLK

> Thanks,
> Alex
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alex Smith Feb. 28, 2015, 9:58 a.m. UTC | #3
On 27 February 2015 at 09:20, Zubair Lutfullah Kakakhel
<Zubair.Kakakhel@imgtec.com> wrote:
> Hi Alex,
>
> On 26/02/15 20:04, Alex Smith wrote:
>> Hi Zubair,
>>
>> On 26 February 2015 at 12:43, Zubair Lutfullah Kakakhel
>> <Zubair.Kakakhel@imgtec.com> wrote:
>>> From: Alex Smith <alex.smith@imgtec.com>
>>>
>>> Add device tree bindings for the DMA controller on JZ4780 SoCs, used by
>>> the dma-jz4780 driver.
>>>
>>> Signed-off-by: Alex Smith <alex.smith@imgtec.com>
>>> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@imgtec.com>
>>>
>>> ---
>>> V3 -> V2
>>> Changed binding.
>>> Used to be 3 DMA cells required. <&dma TX_type RX_type Reserved>
>>> Now 2 DMA cells are required. <&dma Transfer_type Reserved>
>>>
>>> This is more common in DMA bindings.
>>> And I couldn't figure any reason that 3 cells were used.
>>
>> There are different request type numbers for transfers to/from the
>> same device (see the JZ4780 programmers manual, page 505). While only
>> having the option to specify one transfer type is OK when the driver
>> is using separate channels for read/writes, I recall seeing/writing
>> some other drivers which use a single channel for both reads and
>> writes. This would not be possible if we can only specify one transfer
>> type, you'd have to have them separate.
>>
>
> I know. I looked at the drivers and did this on purpose.
>
> We'd like to keep the same bindings/code for jz4740/jz4780 peripheral drivers and dma code.
>
> Our jz47xx-mmc driver we had was the main culprit I found. As well as the jz4740-i2s one.
>
> However, jz4740-mmc upstream driver has improved already for dma and takes two dma channels.
> And the jz4740-i2s also takes two channels. One for Tx and one for Rx.
>
> If we move to 3 cells for jz4780-dma. Then 'ideally' jz4740-dma would need 3 cells too.
> Or we'd have a binding nightmare everywhere.
>
> But when we use jz4740-mmc and jz4740-i2s, we still have to change the driver to share one channel
> or simply pass them
> <&dma Tx 0 Reserved>
> <&dma 0 Rx Reserved>
>
> Which makes the binding redundant.
>
> There isn't any particular reason a driver would need to share one channel only.
> The 'special' nand/nemc channels don't have a request type.

Ok, fair enough. I was just thinking that the bindings shouldn't be
restrictive on the way that you can set things up, but if that's the
way that things are already being done for jz4740 then as you say it's
better to follow that.

Thanks,
Alex

>
> Thanks,
> ZubairLK
>
>> Thanks,
>> Alex
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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/jz4780-dma.txt b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
new file mode 100644
index 0000000..f25feee
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/jz4780-dma.txt
@@ -0,0 +1,56 @@ 
+* Ingenic JZ4780 DMA Controller
+
+Required properties:
+
+- compatible: Should be "ingenic,jz4780-dma"
+- reg: Should contain the DMA controller registers location and length.
+- interrupts: Should contain the interrupt specifier of the DMA controller.
+- interrupt-parent: Should be the phandle of the interrupt controller that
+- clocks: Should contain a clock specifier for the JZ4780 PDMA clock.
+- #dma-cells: Must be <2>. Number of integer cells in the dmas property of
+  DMA clients (see below).
+
+Optional properties:
+
+- ingenic,reserved-channels: Bitmask of channels to reserve for devices that
+  need a specific channel. These channels will only be assigned when explicitly
+  requested by a client. The primary use for this is channels 0 and 1, which
+  can be configured to have special behaviour for NAND/BCH when using
+  programmable firmware.
+
+Example:
+
+dma: dma@13420000 {
+	compatible = "ingenic,jz4780-dma";
+	reg = <0x13420000 0x10000>;
+
+	interrupt-parent = <&intc>;
+	interrupts = <10>;
+
+	clocks = <&cgu JZ4780_CLK_PDMA>;
+
+	#dma-cells = <2>;
+
+	ingenic,reserved-channels = <0x3>;
+};
+
+DMA clients must use the format described in dma.txt, giving a phandle to the
+DMA controller plus the following 2 integer cells:
+
+1. Request type: The DMA request type for transfers to/from the device on
+   the allocated channel, as defined in the SoC documentation.
+
+2. Channel: If set to 0xffffffff, any available channel will be allocated for
+   the client. Otherwise, the exact channel specified will be used. The channel
+   should be reserved on the DMA controller using the ingenic,reserved-channels
+   property.
+
+Example:
+
+uart0: serial@10030000 {
+	...
+	dmas = <&dma 0x14 0xffffffff
+		&dma 0x15 0xffffffff>;
+	dma-names = "tx", "rx";
+	...
+};