diff mbox series

[v2,1/3] dt/bindings: da8xx-usb: Add binding for the CPPI 4.1 DMA controller

Message ID 20170117142016.11163-2-abailon@baylibre.com
State Accepted
Commit d567206e478f744f90b52f80b85debc3377ea5a0
Headers show
Series dmaengine: cppi41: Add dma support to da8xx | expand

Commit Message

Alexandre Bailon Jan. 17, 2017, 2:20 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          | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)

-- 
2.10.2

--
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

Comments

Sergei Shtylyov Jan. 17, 2017, 5:14 p.m. UTC | #1
On 01/17/2017 05:20 PM, Alexandre Bailon wrote:

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


    "CPPI 4.1 DMA" again.

>

> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

> ---

>  .../devicetree/bindings/usb/da8xx-usb.txt          | 42 ++++++++++++++++++++++

>  1 file changed, 42 insertions(+)

>

> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt

> index ccb844a..aed3169 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: CPPI DMA Controller,

> +  CPPI DMA Scheduler, Queue Manager

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

> +- #dma-cells: should be set to 2. The first number represents the

> +  channel 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.


    Shouldn't the # of channels be a part of the glue data?

[...]
> @@ -39,5 +58,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 =  <0x201000 0x1000

> +				0x202000 0x1000

> +				0x204000 0x4000>;

> +			reg-names = "controller", "scheduler", "queuemgr";

> +			interrupts = <58>;

> +			interrupt-names = "glue";

> +			#dma-cells = <2>;

> +			#dma-channels = <4>;

> +			#dma-requests = <256>;


    Not seeing this prop documented...

> +			status = "okay";


    Can be omitted.

> +		};

> +

>  	};

>


--
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
Sergei Shtylyov Jan. 17, 2017, 5:16 p.m. UTC | #2
On 01/17/2017 05:20 PM, Alexandre Bailon wrote:

> 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          | 42 ++++++++++++++++++++++

>  1 file changed, 42 insertions(+)

>

> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt

> index ccb844a..aed3169 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


    Almost missed this -- wildcards in this property are forbidden.
We should use "ti,da830-cppi41" as a least common denominator.

MBR, Sergei

--
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
Sergei Shtylyov Jan. 18, 2017, 9:30 a.m. UTC | #3
Hello!

On 1/18/2017 11:33 AM, Sekhar Nori wrote:

>>> 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          | 42

>>> ++++++++++++++++++++++

>>>  1 file changed, 42 insertions(+)

>>>

>>> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt

>>> b/Documentation/devicetree/bindings/usb/da8xx-usb.txt

>>> index ccb844a..aed3169 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

>>

>>    Almost missed this -- wildcards in this property are forbidden.

>> We should use "ti,da830-cppi41" as a least common denominator.

>

> Documentation/devicetree/bindings/submitting-patches.txt states:

>

> "

>   5) The wildcard "<chip>" may be used in compatible strings, as in

>      the following example:

>

>          - compatible: Must contain '"nvidia,<chip>-pcie",

>            "nvidia,tegra20-pcie"' where <chip> is tegra30, tegra132, ...

> "

>

> I take this to mean that using wildcards to denote an SoC family on

> which the same IP is present is okay to do.> With that understanding, I

> think using ti,da8xx-cppi41 is fine too.


    It doesn't really follow. I repeat, x's are not allowed.

[...]

> Thanks,

> Sekhar


MBR, Sergei

--
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
Sekhar Nori Jan. 18, 2017, 10:21 a.m. UTC | #4
Hi Rob,

On Wednesday 18 January 2017 03:00 PM, Sergei Shtylyov wrote:
> Hello!

> 

> On 1/18/2017 11:33 AM, Sekhar Nori wrote:

> 

>>>> 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          | 42

>>>> ++++++++++++++++++++++

>>>>  1 file changed, 42 insertions(+)

>>>>

>>>> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt

>>>> b/Documentation/devicetree/bindings/usb/da8xx-usb.txt

>>>> index ccb844a..aed3169 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

>>>

>>>    Almost missed this -- wildcards in this property are forbidden.

>>> We should use "ti,da830-cppi41" as a least common denominator.

>>

>> Documentation/devicetree/bindings/submitting-patches.txt states:

>>

>> "

>>   5) The wildcard "<chip>" may be used in compatible strings, as in

>>      the following example:

>>

>>          - compatible: Must contain '"nvidia,<chip>-pcie",

>>            "nvidia,tegra20-pcie"' where <chip> is tegra30, tegra132, ...

>> "

>>

>> I take this to mean that using wildcards to denote an SoC family on

>> which the same IP is present is okay to do.> With that understanding, I

>> think using ti,da8xx-cppi41 is fine too.

> 

>    It doesn't really follow. I repeat, x's are not allowed.


Can you clarify here? If x's are indeed not allowed, then I think the
document is being ambiguous about what constitutes "The wildcard "<chip>""

Thanks,
Sekhar
--
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
Alexandre Bailon Jan. 18, 2017, 1:04 p.m. UTC | #5
On 01/17/2017 06:14 PM, Sergei Shtylyov wrote:
> On 01/17/2017 05:20 PM, Alexandre Bailon wrote:

> 

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

> 

>    "CPPI 4.1 DMA" again.

> 

>>

>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

>> ---

>>  .../devicetree/bindings/usb/da8xx-usb.txt          | 42

>> ++++++++++++++++++++++

>>  1 file changed, 42 insertions(+)

>>

>> diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt

>> b/Documentation/devicetree/bindings/usb/da8xx-usb.txt

>> index ccb844a..aed3169 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: CPPI DMA

>> Controller,

>> +  CPPI DMA Scheduler, Queue Manager

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

>> +- #dma-cells: should be set to 2. The first number represents the

>> +  channel 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.

> 

>    Shouldn't the # of channels be a part of the glue data?

TBH I don't think know. For AM33xx, the number of channels is defined
so I did the same for DA850.
> 

> [...]

>> @@ -39,5 +58,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 =  <0x201000 0x1000

>> +                0x202000 0x1000

>> +                0x204000 0x4000>;

>> +            reg-names = "controller", "scheduler", "queuemgr";

>> +            interrupts = <58>;

>> +            interrupt-names = "glue";

>> +            #dma-cells = <2>;

>> +            #dma-channels = <4>;

>> +            #dma-requests = <256>;

> 

>    Not seeing this prop documented...

Right. Actually, this prop is not even used so I will just remove it.
> 

>> +            status = "okay";

> 

>    Can be omitted.

> 

>> +        };

>> +

>>      };

>>

> 

Best Regards,
Alexandre
--
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 series

Patch

diff --git a/Documentation/devicetree/bindings/usb/da8xx-usb.txt b/Documentation/devicetree/bindings/usb/da8xx-usb.txt
index ccb844a..aed3169 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: CPPI DMA Controller,
+  CPPI DMA Scheduler, Queue Manager
+- reg-names: "controller", "scheduler", "queuemgr"
+- #dma-cells: should be set to 2. The first number represents the
+  channel 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";
@@ -31,6 +47,9 @@  Example:
 	usb0: usb@200000 {
 		compatible = "ti,da830-musb";
 		reg =   <0x00200000 0x10000>;
+		ranges;
+		#address-cells = <1>;
+		#size-cells = <1>;
 		interrupts = <58>;
 		interrupt-names = "mc";
 
@@ -39,5 +58,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 =  <0x201000 0x1000
+				0x202000 0x1000
+				0x204000 0x4000>;
+			reg-names = "controller", "scheduler", "queuemgr";
+			interrupts = <58>;
+			interrupt-names = "glue";
+			#dma-cells = <2>;
+			#dma-channels = <4>;
+			#dma-requests = <256>;
+			status = "okay";
+		};
+
 	};