diff mbox series

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

Message ID 20221206043554.1521522-2-vigneshr@ti.com
State New
Headers show
Series dmaengine: Add support for AM62A SoC DMAs | expand

Commit Message

Vignesh Raghavendra Dec. 6, 2022, 4:35 a.m. UTC
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(-)

Comments

Vignesh Raghavendra Dec. 7, 2022, 6:09 a.m. UTC | #1
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!
Krzysztof Kozlowski Dec. 7, 2022, 8:03 a.m. UTC | #2
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
diff mbox series

Patch

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: