[PATCH 1/5] dt-bindings: dma: ti: k3-bcdma: Add bindings for BCDMA CSI RX

Vignesh Raghavendra posted 5 patches 2 years, 9 months ago
There is a newer version of this series
[PATCH 1/5] dt-bindings: dma: ti: k3-bcdma: Add bindings for BCDMA CSI RX
Posted by Vignesh Raghavendra 2 years, 9 months ago
AM62A SoC has a dedicated BCDMA that serves Camera Serial Interface
(CSI) IP. Add new compatible for the same. Unlike system
BCDMA, this instance only has RX DMA channels and lack TX or block copy
channel. Thus make those properties optional. Additionally CSI RX has
independent power domain, add the binding for the same.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---
 .../devicetree/bindings/dma/ti/k3-bcdma.yaml  | 87 ++++++++++++++-----
 1 file changed, 63 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
index 08627d91e607..d7b5adbb9b2e 100644
--- a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
+++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
@@ -32,9 +32,66 @@ allOf:
   - $ref: /schemas/dma/dma-controller.yaml#
   - $ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: ti,am62a-dmss-bcdma-csirx
+    then:
+      properties:
+        ti,sci-rm-range-bchan: false
+        ti,sci-rm-range-tchan: false
+
+        reg:
+          maxItems: 3
+
+        reg-names:
+          items:
+            - const: gcfg
+            - const: rchanrt
+            - const: ringrt
+
+      required:
+        - compatible
+        - "#dma-cells"
+        - reg
+        - reg-names
+        - msi-parent
+        - ti,sci
+        - ti,sci-dev-id
+        - ti,sci-rm-range-rchan
+        - power-domains
+
+    else:
+      properties:
+        reg:
+          maxItems: 5
+
+        reg-names:
+          items:
+            - const: gcfg
+            - const: bchanrt
+            - const: rchanrt
+            - const: tchanrt
+            - const: ringrt
+
+      required:
+        - compatible
+        - "#dma-cells"
+        - reg
+        - reg-names
+        - msi-parent
+        - ti,sci
+        - ti,sci-dev-id
+        - ti,sci-rm-range-bchan
+        - ti,sci-rm-range-tchan
+        - ti,sci-rm-range-rchan
+
 properties:
   compatible:
-    const: ti,am64-dmss-bcdma
+    enum:
+      - ti,am64-dmss-bcdma
+      - ti,am62a-dmss-bcdma-csirx
 
   "#dma-cells":
     const: 3
@@ -65,19 +122,13 @@ properties:
 
       cell 3: ASEL value for the channel
 
-  reg:
-    maxItems: 5
-
-  reg-names:
-    items:
-      - const: gcfg
-      - const: bchanrt
-      - const: rchanrt
-      - const: tchanrt
-      - const: ringrt
-
   msi-parent: true
 
+  power-domains:
+    description:
+      Power domain if available
+    maxItems: 1
+
   ti,asel:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: ASEL value for non slave channels
@@ -115,18 +166,6 @@ properties:
     items:
       maximum: 0x3f
 
-required:
-  - compatible
-  - "#dma-cells"
-  - reg
-  - reg-names
-  - msi-parent
-  - ti,sci
-  - ti,sci-dev-id
-  - ti,sci-rm-range-bchan
-  - ti,sci-rm-range-tchan
-  - ti,sci-rm-range-rchan
-
 unevaluatedProperties: false
 
 examples:
-- 
2.38.1
Re: [PATCH 1/5] dt-bindings: dma: ti: k3-bcdma: Add bindings for BCDMA CSI RX
Posted by Krzysztof Kozlowski 2 years, 9 months ago
On 06/12/2022 05:35, Vignesh Raghavendra wrote:
> AM62A SoC has a dedicated BCDMA that serves Camera Serial Interface
> (CSI) IP. Add new compatible for the same. Unlike system
> BCDMA, this instance only has RX DMA channels and lack TX or block copy
> channel. Thus make those properties optional. Additionally CSI RX has
> independent power domain, add the binding for the same.
> 
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
>  .../devicetree/bindings/dma/ti/k3-bcdma.yaml  | 87 ++++++++++++++-----
>  1 file changed, 63 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> index 08627d91e607..d7b5adbb9b2e 100644
> --- a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
> @@ -32,9 +32,66 @@ allOf:
>    - $ref: /schemas/dma/dma-controller.yaml#
>    - $ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
>  

When adding if:then:, please move entire allOf after "required:" part.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ti,am62a-dmss-bcdma-csirx
> +    then:
> +      properties:
> +        ti,sci-rm-range-bchan: false
> +        ti,sci-rm-range-tchan: false
> +
> +        reg:
> +          maxItems: 3
> +
> +        reg-names:
> +          items:
> +            - const: gcfg
> +            - const: rchanrt
> +            - const: ringrt

With my changes further this can be only "maxItems: 3"

> +
> +      required:
> +        - compatible
> +        - "#dma-cells"
> +        - reg
> +        - reg-names
> +        - msi-parent
> +        - ti,sci
> +        - ti,sci-dev-id
> +        - ti,sci-rm-range-rchan
> +        - power-domains
> +
> +    else:
> +      properties:
> +        reg:
> +          maxItems: 5
> +
> +        reg-names:
> +          items:
> +            - const: gcfg
> +            - const: bchanrt
> +            - const: rchanrt
> +            - const: tchanrt
> +            - const: ringrt

With my changes further this can be only "minItems: 5"

> +
> +      required:
> +        - compatible
> +        - "#dma-cells"
> +        - reg
> +        - reg-names
> +        - msi-parent
> +        - ti,sci
> +        - ti,sci-dev-id
> +        - ti,sci-rm-range-bchan
> +        - ti,sci-rm-range-tchan
> +        - ti,sci-rm-range-rchan
> +
>  properties:
>    compatible:
> -    const: ti,am64-dmss-bcdma
> +    enum:
> +      - ti,am64-dmss-bcdma
> +      - ti,am62a-dmss-bcdma-csirx

Keep some order, e.g. alphabetical. This reduces later conflicts on
simultaneous edits.

>  
>    "#dma-cells":
>      const: 3
> @@ -65,19 +122,13 @@ properties:
>  
>        cell 3: ASEL value for the channel
>  
> -  reg:
> -    maxItems: 5

Keep it here with widest constrains - minItems: 3, maxItems: 5

> -
> -  reg-names:
> -    items:
> -      - const: gcfg
> -      - const: bchanrt
> -      - const: rchanrt
> -      - const: tchanrt
> -      - const: ringrt

Keep the list here with minItems: 3

> -
>    msi-parent: true
>  
> +  power-domains:
> +    description:
> +      Power domain if available
> +    maxItems: 1
> +
>    ti,asel:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      description: ASEL value for non slave channels
> @@ -115,18 +166,6 @@ properties:
>      items:
>        maximum: 0x3f
>  
> -required:
> -  - compatible
> -  - "#dma-cells"
> -  - reg
> -  - reg-names
> -  - msi-parent
> -  - ti,sci
> -  - ti,sci-dev-id
> -  - ti,sci-rm-range-bchan
> -  - ti,sci-rm-range-tchan
> -  - ti,sci-rm-range-rchan

Keep required here. Customize it if needed in if:then:else.

> -
>  unevaluatedProperties: false
>  
>  examples:

Best regards,
Krzysztof
Re: [PATCH 1/5] dt-bindings: dma: ti: k3-bcdma: Add bindings for BCDMA CSI RX
Posted by Vignesh Raghavendra 2 years, 9 months ago
Hi Krzysztof,

On 06/12/22 14:02, Krzysztof Kozlowski wrote:
> On 06/12/2022 05:35, Vignesh Raghavendra wrote:
>> AM62A SoC has a dedicated BCDMA that serves Camera Serial Interface
>> (CSI) IP. Add new compatible for the same. Unlike system
>> BCDMA, this instance only has RX DMA channels and lack TX or block copy
>> channel. Thus make those properties optional. Additionally CSI RX has
>> independent power domain, add the binding for the same.
>>
>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>> ---
>>  .../devicetree/bindings/dma/ti/k3-bcdma.yaml  | 87 ++++++++++++++-----
>>  1 file changed, 63 insertions(+), 24 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
>> index 08627d91e607..d7b5adbb9b2e 100644
>> --- a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
>> +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
>> @@ -32,9 +32,66 @@ allOf:
>>    - $ref: /schemas/dma/dma-controller.yaml#
>>    - $ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
>>  
> 
> When adding if:then:, please move entire allOf after "required:" part.

Sure, will do

> 
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: ti,am62a-dmss-bcdma-csirx
>> +    then:
>> +      properties:
>> +        ti,sci-rm-range-bchan: false
>> +        ti,sci-rm-range-tchan: false
>> +
>> +        reg:
>> +          maxItems: 3
>> +
>> +        reg-names:
>> +          items:
>> +            - const: gcfg
>> +            - const: rchanrt
>> +            - const: ringrt
> 
> With my changes further this can be only "maxItems: 3"

Yes, but wont that mean any of the 3 reg-names out of the 5? Would it
not be better to further restrict specifically to above 3 reg-names (as
thats how the IP is)

> 
>> +
>> +      required:
>> +        - compatible
>> +        - "#dma-cells"
>> +        - reg
>> +        - reg-names
>> +        - msi-parent
>> +        - ti,sci
>> +        - ti,sci-dev-id
>> +        - ti,sci-rm-range-rchan
>> +        - power-domains
>> +
>> +    else:
>> +      properties:
>> +        reg:
>> +          maxItems: 5
>> +
>> +        reg-names:
>> +          items:
>> +            - const: gcfg
>> +            - const: bchanrt
>> +            - const: rchanrt
>> +            - const: tchanrt
>> +            - const: ringrt
> 
> With my changes further this can be only "minItems: 5"

Ok.

> 
>> +
>> +      required:
>> +        - compatible
>> +        - "#dma-cells"
>> +        - reg
>> +        - reg-names
>> +        - msi-parent
>> +        - ti,sci
>> +        - ti,sci-dev-id
>> +        - ti,sci-rm-range-bchan
>> +        - ti,sci-rm-range-tchan
>> +        - ti,sci-rm-range-rchan
>> +
>>  properties:
>>    compatible:
>> -    const: ti,am64-dmss-bcdma
>> +    enum:
>> +      - ti,am64-dmss-bcdma
>> +      - ti,am62a-dmss-bcdma-csirx
> 
> Keep some order, e.g. alphabetical. This reduces later conflicts on
> simultaneous edits.

Will fix!

> 
>>  
>>    "#dma-cells":
>>      const: 3
>> @@ -65,19 +122,13 @@ properties:
>>  
>>        cell 3: ASEL value for the channel
>>  
>> -  reg:
>> -    maxItems: 5
> 
> Keep it here with widest constrains - minItems: 3, maxItems: 5
> 
>> -
>> -  reg-names:
>> -    items:
>> -      - const: gcfg
>> -      - const: bchanrt
>> -      - const: rchanrt
>> -      - const: tchanrt
>> -      - const: ringrt
> 
> Keep the list here with minItems: 3
> 
>> -
>>    msi-parent: true
>>  
>> +  power-domains:
>> +    description:
>> +      Power domain if available
>> +    maxItems: 1
>> +
>>    ti,asel:
>>      $ref: /schemas/types.yaml#/definitions/uint32
>>      description: ASEL value for non slave channels
>> @@ -115,18 +166,6 @@ properties:
>>      items:
>>        maximum: 0x3f
>>  
>> -required:
>> -  - compatible
>> -  - "#dma-cells"
>> -  - reg
>> -  - reg-names
>> -  - msi-parent
>> -  - ti,sci
>> -  - ti,sci-dev-id
>> -  - ti,sci-rm-range-bchan
>> -  - ti,sci-rm-range-tchan
>> -  - ti,sci-rm-range-rchan
> 
> Keep required here. Customize it if needed in if:then:else.

Got it, will fix accordingly...

> 
>> -
>>  unevaluatedProperties: false
>>  
>>  examples:
> 
> Best regards,
> Krzysztof
> 

Thanks for the review!


-- 
Regards
Vignesh
Re: [PATCH 1/5] dt-bindings: dma: ti: k3-bcdma: Add bindings for BCDMA CSI RX
Posted by Krzysztof Kozlowski 2 years, 9 months ago
On 07/12/2022 07:09, Vignesh Raghavendra wrote:
> Hi Krzysztof,
> 
> On 06/12/22 14:02, Krzysztof Kozlowski wrote:
>> On 06/12/2022 05:35, Vignesh Raghavendra wrote:
>>> AM62A SoC has a dedicated BCDMA that serves Camera Serial Interface
>>> (CSI) IP. Add new compatible for the same. Unlike system
>>> BCDMA, this instance only has RX DMA channels and lack TX or block copy
>>> channel. Thus make those properties optional. Additionally CSI RX has
>>> independent power domain, add the binding for the same.
>>>
>>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>>> ---
>>>  .../devicetree/bindings/dma/ti/k3-bcdma.yaml  | 87 ++++++++++++++-----
>>>  1 file changed, 63 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
>>> index 08627d91e607..d7b5adbb9b2e 100644
>>> --- a/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
>>> +++ b/Documentation/devicetree/bindings/dma/ti/k3-bcdma.yaml
>>> @@ -32,9 +32,66 @@ allOf:
>>>    - $ref: /schemas/dma/dma-controller.yaml#
>>>    - $ref: /schemas/arm/keystone/ti,k3-sci-common.yaml#
>>>  
>>
>> When adding if:then:, please move entire allOf after "required:" part.
> 
> Sure, will do
> 
>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: ti,am62a-dmss-bcdma-csirx
>>> +    then:
>>> +      properties:
>>> +        ti,sci-rm-range-bchan: false
>>> +        ti,sci-rm-range-tchan: false
>>> +
>>> +        reg:
>>> +          maxItems: 3
>>> +
>>> +        reg-names:
>>> +          items:
>>> +            - const: gcfg
>>> +            - const: rchanrt
>>> +            - const: ringrt
>>
>> With my changes further this can be only "maxItems: 3"
> 
> Yes, but wont that mean any of the 3 reg-names out of the 5? Would it
> not be better to further restrict specifically to above 3 reg-names (as
> thats how the IP is)

I thought that first three entries are the same, but they are not, so
indeed keep it like you have it now.

> 
>>
>>> +
>>> +      required:
>>> +        - compatible
>>> +        - "#dma-cells"
>>> +        - reg
>>> +        - reg-names
>>> +        - msi-parent
>>> +        - ti,sci
>>> +        - ti,sci-dev-id
>>> +        - ti,sci-rm-range-rchan
>>> +        - power-domains
>>> +
>>> +    else:
>>> +      properties:
>>> +        reg:
>>> +          maxItems: 5
>>> +
>>> +        reg-names:
>>> +          items:
>>> +            - const: gcfg
>>> +            - const: bchanrt
>>> +            - const: rchanrt
>>> +            - const: tchanrt
>>> +            - const: ringrt
>>
>> With my changes further this can be only "minItems: 5"
> 
> Ok.

I was wrong, keep it.

> 
>>
>>> +
>>> +      required:
>>> +        - compatible
>>> +        - "#dma-cells"
>>> +        - reg
>>> +        - reg-names
>>> +        - msi-parent
>>> +        - ti,sci
>>> +        - ti,sci-dev-id
>>> +        - ti,sci-rm-range-bchan
>>> +        - ti,sci-rm-range-tchan
>>> +        - ti,sci-rm-range-rchan
>>> +
>>>  properties:
>>>    compatible:
>>> -    const: ti,am64-dmss-bcdma
>>> +    enum:
>>> +      - ti,am64-dmss-bcdma
>>> +      - ti,am62a-dmss-bcdma-csirx
>>
>> Keep some order, e.g. alphabetical. This reduces later conflicts on
>> simultaneous edits.
> 
> Will fix!
> 
>>
>>>  
>>>    "#dma-cells":
>>>      const: 3
>>> @@ -65,19 +122,13 @@ properties:
>>>  
>>>        cell 3: ASEL value for the channel
>>>  
>>> -  reg:
>>> -    maxItems: 5
>>
>> Keep it here with widest constrains - minItems: 3, maxItems: 5
>>
>>> -
>>> -  reg-names:
>>> -    items:
>>> -      - const: gcfg
>>> -      - const: bchanrt
>>> -      - const: rchanrt
>>> -      - const: tchanrt
>>> -      - const: ringrt
>>
>> Keep the list here with minItems: 3

So here minItems:3, maxItems:5


Best regards,
Krzysztof