diff mbox

[07/11] dt/bindings: da8xx-usb: Add binding for the cppi41 dma controller

Message ID 20170109160656.3470-8-abailon@baylibre.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Alexandre Bailon Jan. 9, 2017, 4:06 p.m. UTC
DT binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx cppi41 dma controller.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 .../devicetree/bindings/usb/da8xx-usb.txt          | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)

Comments

Sergei Shtylyov Jan. 9, 2017, 6:41 p.m. UTC | #1
Hello!

On 01/09/2017 07:06 PM, Alexandre Bailon wrote:

> DT binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx cppi41 dma controller.

    It's called CPPI 4.1, not cppi41.
    Let me introduce myself: I was the author of the 1st CPPI 4.1 drivers 
(never merged).

> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  .../devicetree/bindings/usb/da8xx-usb.txt          | 39 ++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
> index ccb844a..2a4d737 100644
> --- a/Documentation/devicetree/bindings/usb/da8xx-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
> @@ -18,10 +18,26 @@ Required properties:
[...]
>  Optional properties:
>  ~~~~~~~~~~~~~~~~~~~~
>   - vbus-supply: Phandle to a regulator providing the USB bus power.
>
> +DMA
> +~~~
> +- compatible: ti,da8xx-cppi41
> +- reg: offset and length of the following register spaces: USBSS, USB

    I don't understand what you mean by USBSS and how it's related to CPPI 4.1.

> +  CPPI DMA Controller, USB CPPI DMA Scheduler, USB Queue Manager

    I'd drop "USB" everywhere, the DMAC scheduler and queue manager are not a 
part of any USB logic.

> +- reg-names: glue, controller, scheduler, queuemgr

    Quoted, maybe?

> +- #dma-cells: should be set to 2. The first number represents the
> +  endpoint number (0 … 3 for endpoints 1 … 4).

    Not sure why you need EPs here. Is that required by the DMA driver?
The DMA controller operates with channels...

[...]

MBR, Sergei

--
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
Alexandre Bailon Jan. 10, 2017, 10:37 a.m. UTC | #2
Hello,

On 01/09/2017 07:41 PM, Sergei Shtylyov wrote:
> Hello!
> 
> On 01/09/2017 07:06 PM, Alexandre Bailon wrote:
> 
>> DT binding for the TI DA8xx/OMAP-L1x/AM17xx/AM18xx cppi41 dma controller.
> 
>    It's called CPPI 4.1, not cppi41.
>    Let me introduce myself: I was the author of the 1st CPPI 4.1 drivers
> (never merged).
> 
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>> ---
>>  .../devicetree/bindings/usb/da8xx-usb.txt          | 39
>> ++++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>> b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>> index ccb844a..2a4d737 100644
>> --- a/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>> +++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
>> @@ -18,10 +18,26 @@ Required properties:
> [...]
>>  Optional properties:
>>  ~~~~~~~~~~~~~~~~~~~~
>>   - vbus-supply: Phandle to a regulator providing the USB bus power.
>>
>> +DMA
>> +~~~
>> +- compatible: ti,da8xx-cppi41
>> +- reg: offset and length of the following register spaces: USBSS, USB
> 
>    I don't understand what you mean by USBSS and how it's related to
> CPPI 4.1.
I have kept the terminology used in the CPPI 4.1 driver.
USBSS is how the MUSB glue is named in AM335x datasheet.
The CPPI 4.1 driver need to access to few of this registers.
> 
>> +  CPPI DMA Controller, USB CPPI DMA Scheduler, USB Queue Manager
> 
>    I'd drop "USB" everywhere, the DMAC scheduler and queue manager are
> not a part of any USB logic.
Will do it.
> 
>> +- reg-names: glue, controller, scheduler, queuemgr
> 
>    Quoted, maybe?
> 
>> +- #dma-cells: should be set to 2. The first number represents the
>> +  endpoint number (0 … 3 for endpoints 1 … 4).
> 
>    Not sure why you need EPs here. Is that required by the DMA driver?
> The DMA controller operates with channels...
I have taken this from am335x-usb bindings and I kept it as is because
I don't think that's wrong.
I agree we should use DMA terminology but actually the CPPI 4.1 is only
used by USB in DA8xx (share the same clock, the same irq, etc).
In the case of DA8xx, I'm not sure that would make things more clear.

Best Regards,
Alexandre
> 
> [...]
> 
> MBR, Sergei
> 

--
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/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
index ccb844a..2a4d737 100644
--- a/Documentation/devicetree/bindings/usb/da8xx-usb.txt
+++ b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
@@ -18,10 +18,26 @@  Required properties:
 
  - phy-names: Should be "usb-phy"
 
+ - dmas: specifies the dma channels
+
+ - dma-names: specifies the names of the channels. Use "rxN" for receive
+   and "txN" for transmit endpoints. N specifies the endpoint number.
+
 Optional properties:
 ~~~~~~~~~~~~~~~~~~~~
  - vbus-supply: Phandle to a regulator providing the USB bus power.
 
+DMA
+~~~
+- compatible: ti,da8xx-cppi41
+- reg: offset and length of the following register spaces: USBSS, USB
+  CPPI DMA Controller, USB CPPI DMA Scheduler, USB Queue Manager
+- reg-names: glue, controller, scheduler, queuemgr
+- #dma-cells: should be set to 2. The first number represents the
+  endpoint number (0 … 3 for endpoints 1 … 4).
+  The second number is 0 for RX and 1 for TX transfers.
+- #dma-channels: should be set to 4 representing the 4 endpoints.
+
 Example:
 	usb_phy: usb-phy {
 		compatible = "ti,da830-usb-phy";
@@ -39,5 +55,28 @@  Example:
 		phys = <&usb_phy 0>;
 		phy-names = "usb-phy";
 
+		dmas = <&cppi41dma 0 0 &cppi41dma 1 0
+			&cppi41dma 2 0 &cppi41dma 3 0
+			&cppi41dma 0 1 &cppi41dma 1 1
+			&cppi41dma 2 1 &cppi41dma 3 1>;
+		dma-names =
+			"rx1", "rx2", "rx3", "rx4",
+			"tx1", "tx2", "tx3", "tx4";
+
 		status = "okay";
 	};
+	cppi41dma: dma-controller@201000 {
+		compatible = "ti,da8xx-cppi41";
+		reg =  <0x200000 0x1000
+			0x201000 0x1000
+			0x202000 0x1000
+			0x204000 0x4000>;
+		reg-names = "glue", "controller", "scheduler", "queuemgr";
+		interrupts = <58>;
+		interrupt-names = "glue";
+		#dma-cells = <2>;
+		#dma-channels = <4>;
+		#dma-requests = <256>;
+		status = "disabled";
+
+	};