diff mbox series

[v5,12/12] dt-bindings: fsl-dma: fsl-edma: add edma3 compatible string

Message ID 20230613213149.2076358-13-Frank.Li@nxp.com
State New
Headers
Series dmaengine: edma: add freescale edma v3 support |

Commit Message

Frank Li June 13, 2023, 9:31 p.m. UTC 
  Extend Freescale eDMA driver bindings to support eDMA3 IP blocks in
i.MX8QM and i.MX8QXP SoCs. In i.MX93, both eDMA3 and eDMA4 are now.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 .../devicetree/bindings/dma/fsl,edma.yaml     | 118 +++++++++++++++---
 1 file changed, 100 insertions(+), 18 deletions(-)

Comments

Krzysztof Kozlowski June 13, 2023, 9:43 p.m. UTC | #1 
On 13/06/2023 23:31, Frank Li wrote:
> Extend Freescale eDMA driver bindings to support eDMA3 IP blocks in
> i.MX8QM and i.MX8QXP SoCs. In i.MX93, both eDMA3 and eDMA4 are now.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  .../devicetree/bindings/dma/fsl,edma.yaml     | 118 +++++++++++++++---
>  1 file changed, 100 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/fsl,edma.yaml b/Documentation/devicetree/bindings/dma/fsl,edma.yaml
> index 5fd8fc604261..2f79492fb332 100644
> --- a/Documentation/devicetree/bindings/dma/fsl,edma.yaml
> +++ b/Documentation/devicetree/bindings/dma/fsl,edma.yaml
> @@ -21,32 +21,20 @@ properties:
>        - enum:
>            - fsl,vf610-edma
>            - fsl,imx7ulp-edma
> +          - fsl,imx8qm-adma
> +          - fsl,imx8qm-edma
> +          - fsl,imx93-edma3
> +          - fsl,imx93-edma4
>        - items:
>            - const: fsl,ls1028a-edma
>            - const: fsl,vf610-edma
>  
> -  reg:
> -    minItems: 2
> -    maxItems: 3
> -
> -  interrupts:
> -    minItems: 2
> -    maxItems: 17

What is happening here?

> -
> -  interrupt-names:
> -    minItems: 2
> -    maxItems: 17
> -
> -  "#dma-cells":
> -    const: 2
> -
> -  dma-channels:
> -    const: 32

No, why all these are being removed?

> -
>    clocks:
> +    minItems: 1
>      maxItems: 2
>  
>    clock-names:
> +    minItems: 1
>      maxItems: 2
>  
>    big-endian:
> @@ -55,6 +43,43 @@ properties:
>        eDMA are implemented in big endian mode, otherwise in little mode.
>      type: boolean
>  
> +if:

This should not be outside of your allOf. This patch looks entirely
different than your v4 and I don't really understand why.


> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - fsl,imx8qm-adma
> +          - fsl,imx8qm-edma
> +          - fsl,imx93-edma3
> +          - fsl,imx93-edma4
> +then:
> +  properties:
> +    reg:
> +      maxItems: 1
> +    interrupts:
> +      minItems: 1
> +      maxItems: 64

What's more, you don't have these properties defined in top-level.
Sorry, they should not be moved. I did not ask for this.

Best regards,
Krzysztof
Frank Li June 13, 2023, 10:01 p.m. UTC | #2 
On Tue, Jun 13, 2023 at 11:43:31PM +0200, Krzysztof Kozlowski wrote:
> On 13/06/2023 23:31, Frank Li wrote:
> > Extend Freescale eDMA driver bindings to support eDMA3 IP blocks in
> > i.MX8QM and i.MX8QXP SoCs. In i.MX93, both eDMA3 and eDMA4 are now.
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  .../devicetree/bindings/dma/fsl,edma.yaml     | 118 +++++++++++++++---
> >  1 file changed, 100 insertions(+), 18 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/dma/fsl,edma.yaml b/Documentation/devicetree/bindings/dma/fsl,edma.yaml
> > index 5fd8fc604261..2f79492fb332 100644
> > --- a/Documentation/devicetree/bindings/dma/fsl,edma.yaml
> > +++ b/Documentation/devicetree/bindings/dma/fsl,edma.yaml
> > @@ -21,32 +21,20 @@ properties:
> >        - enum:
> >            - fsl,vf610-edma
> >            - fsl,imx7ulp-edma
> > +          - fsl,imx8qm-adma
> > +          - fsl,imx8qm-edma
> > +          - fsl,imx93-edma3
> > +          - fsl,imx93-edma4
> >        - items:
> >            - const: fsl,ls1028a-edma
> >            - const: fsl,vf610-edma
> >  
> > -  reg:
> > -    minItems: 2
> > -    maxItems: 3
> > -
> > -  interrupts:
> > -    minItems: 2
> > -    maxItems: 17
> 
> What is happening here?

I found dt_check always check this part firstly, then check allOf.

> 
> > -
> > -  interrupt-names:
> > -    minItems: 2
> > -    maxItems: 17
> > -
> > -  "#dma-cells":
> > -    const: 2
> > -
> > -  dma-channels:
> > -    const: 32
> 
> No, why all these are being removed?

I move common part ahead of if-then-else branch to read early.

> 
> > -
> >    clocks:
> > +    minItems: 1
> >      maxItems: 2
> >  
> >    clock-names:
> > +    minItems: 1
> >      maxItems: 2
> >  
> >    big-endian:
> > @@ -55,6 +43,43 @@ properties:
> >        eDMA are implemented in big endian mode, otherwise in little mode.
> >      type: boolean
> >  
> > +if:
> 
> This should not be outside of your allOf. This patch looks entirely
> different than your v4 and I don't really understand why.
> 

allOf looks like addtional constraints addition to previous define.
for example: 
    if previous interrupts is 17, I can't overwrite to bigger value 64
in this sesson. 

previous version: dts check report two error, 
first: interrupt is too long. (look like check top one)
then: interrupt is too short. (look like check allOf part)

> 
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - fsl,imx8qm-adma
> > +          - fsl,imx8qm-edma
> > +          - fsl,imx93-edma3
> > +          - fsl,imx93-edma4
> > +then:
> > +  properties:
> > +    reg:
> > +      maxItems: 1
> > +    interrupts:
> > +      minItems: 1
> > +      maxItems: 64
> 
> What's more, you don't have these properties defined in top-level.
> Sorry, they should not be moved. I did not ask for this.

It is there. 
if-then-else before "required"

> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski June 14, 2023, 6:39 a.m. UTC | #3 
On 14/06/2023 00:01, Frank Li wrote:
> On Tue, Jun 13, 2023 at 11:43:31PM +0200, Krzysztof Kozlowski wrote:
>> On 13/06/2023 23:31, Frank Li wrote:
>>> Extend Freescale eDMA driver bindings to support eDMA3 IP blocks in
>>> i.MX8QM and i.MX8QXP SoCs. In i.MX93, both eDMA3 and eDMA4 are now.
>>>
>>> Signed-off-by: Frank Li <Frank.Li@nxp.com>
>>> ---
>>>  .../devicetree/bindings/dma/fsl,edma.yaml     | 118 +++++++++++++++---
>>>  1 file changed, 100 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/dma/fsl,edma.yaml b/Documentation/devicetree/bindings/dma/fsl,edma.yaml
>>> index 5fd8fc604261..2f79492fb332 100644
>>> --- a/Documentation/devicetree/bindings/dma/fsl,edma.yaml
>>> +++ b/Documentation/devicetree/bindings/dma/fsl,edma.yaml
>>> @@ -21,32 +21,20 @@ properties:
>>>        - enum:
>>>            - fsl,vf610-edma
>>>            - fsl,imx7ulp-edma
>>> +          - fsl,imx8qm-adma
>>> +          - fsl,imx8qm-edma
>>> +          - fsl,imx93-edma3
>>> +          - fsl,imx93-edma4
>>>        - items:
>>>            - const: fsl,ls1028a-edma
>>>            - const: fsl,vf610-edma
>>>  
>>> -  reg:
>>> -    minItems: 2
>>> -    maxItems: 3
>>> -
>>> -  interrupts:
>>> -    minItems: 2
>>> -    maxItems: 17
>>
>> What is happening here?
> 
> I found dt_check always check this part firstly, then check allOf.
> 
>>
>>> -
>>> -  interrupt-names:
>>> -    minItems: 2
>>> -    maxItems: 17
>>> -
>>> -  "#dma-cells":
>>> -    const: 2
>>> -
>>> -  dma-channels:
>>> -    const: 32
>>
>> No, why all these are being removed?
> 
> I move common part ahead of if-then-else branch to read early.
> 
>>
>>> -
>>>    clocks:
>>> +    minItems: 1
>>>      maxItems: 2
>>>  
>>>    clock-names:
>>> +    minItems: 1
>>>      maxItems: 2
>>>  
>>>    big-endian:
>>> @@ -55,6 +43,43 @@ properties:
>>>        eDMA are implemented in big endian mode, otherwise in little mode.
>>>      type: boolean
>>>  
>>> +if:
>>
>> This should not be outside of your allOf. This patch looks entirely
>> different than your v4 and I don't really understand why.
>>
> 
> allOf looks like addtional constraints addition to previous define.
> for example: 
>     if previous interrupts is 17, I can't overwrite to bigger value 64
> in this sesson. 

Yes, because the top-level had wrong constraint. Fix top-level, don't
remove it.

> 
> previous version: dts check report two error, 
> first: interrupt is too long. (look like check top one)
> then: interrupt is too short. (look like check allOf part)
> 
>>
>>> +  properties:
>>> +    compatible:
>>> +      contains:
>>> +        enum:
>>> +          - fsl,imx8qm-adma
>>> +          - fsl,imx8qm-edma
>>> +          - fsl,imx93-edma3
>>> +          - fsl,imx93-edma4
>>> +then:
>>> +  properties:
>>> +    reg:
>>> +      maxItems: 1
>>> +    interrupts:
>>> +      minItems: 1
>>> +      maxItems: 64
>>
>> What's more, you don't have these properties defined in top-level.
>> Sorry, they should not be moved. I did not ask for this.
> 
> It is there. 
> if-then-else before "required"

It's in if, not in top-level properties.


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/dma/fsl,edma.yaml b/Documentation/devicetree/bindings/dma/fsl,edma.yaml
index 5fd8fc604261..2f79492fb332 100644
--- a/Documentation/devicetree/bindings/dma/fsl,edma.yaml
+++ b/Documentation/devicetree/bindings/dma/fsl,edma.yaml
@@ -21,32 +21,20 @@  properties:
       - enum:
           - fsl,vf610-edma
           - fsl,imx7ulp-edma
+          - fsl,imx8qm-adma
+          - fsl,imx8qm-edma
+          - fsl,imx93-edma3
+          - fsl,imx93-edma4
       - items:
           - const: fsl,ls1028a-edma
           - const: fsl,vf610-edma
 
-  reg:
-    minItems: 2
-    maxItems: 3
-
-  interrupts:
-    minItems: 2
-    maxItems: 17
-
-  interrupt-names:
-    minItems: 2
-    maxItems: 17
-
-  "#dma-cells":
-    const: 2
-
-  dma-channels:
-    const: 32
-
   clocks:
+    minItems: 1
     maxItems: 2
 
   clock-names:
+    minItems: 1
     maxItems: 2
 
   big-endian:
@@ -55,6 +43,43 @@  properties:
       eDMA are implemented in big endian mode, otherwise in little mode.
     type: boolean
 
+if:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - fsl,imx8qm-adma
+          - fsl,imx8qm-edma
+          - fsl,imx93-edma3
+          - fsl,imx93-edma4
+then:
+  properties:
+    reg:
+      maxItems: 1
+    interrupts:
+      minItems: 1
+      maxItems: 64
+    "#dma-cells":
+      const: 3
+    dma-channels:
+      minItems: 1
+      maxItems: 64
+else:
+  properties:
+    reg:
+      minItems: 2
+      maxItems: 3
+    interrupts:
+      minItems: 2
+      maxItems: 17
+    interrupt-names:
+      minItems: 2
+      maxItems: 17
+    "#dma-cells":
+      const: 2
+    dma-channels:
+      const: 32
+
 required:
   - "#dma-cells"
   - compatible
@@ -101,6 +126,19 @@  allOf:
         reg:
           maxItems: 2
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - fsl,imx93-edma3
+              - fsl,imx93-edma4
+    then:
+      properties:
+        clock-names:
+          const: dma
+
+
 unevaluatedProperties: false
 
 examples:
@@ -153,3 +191,47 @@  examples:
        clock-names = "dma", "dmamux0";
        clocks = <&pcc2 IMX7ULP_CLK_DMA1>, <&pcc2 IMX7ULP_CLK_DMA_MUX1>;
     };
+
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/imx93-clock.h>
+
+    dma-controller@44000000 {
+      compatible = "fsl,imx93-edma3";
+      reg = <0x44000000 0x200000>;
+      #dma-cells = <3>;
+      dma-channels = <31>;
+      interrupts = <GIC_SPI 95 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 107 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&clk IMX93_CLK_EDMA1_GATE>;
+        clock-names = "dma";
+    };