diff mbox series

[v4,1/3] dt-bindings: dma: snps,dw-axi-dmac: constrain resets items for JH7110 dma

Message ID 20230306140430.28951-2-walker.chen@starfivetech.com (mailing list archive)
State Superseded
Headers show
Series Add DMA driver for StarFive JH7110 SoC | expand

Commit Message

Walker Chen March 6, 2023, 2:04 p.m. UTC
The DMA controller needs two reset items to work properly on JH7110 SoC,
so there is need to constrain the items' value to 2, other platforms
have 1 reset item at most.

Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
---
 .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

Comments

Krzysztof Kozlowski March 7, 2023, 9:03 a.m. UTC | #1
On 06/03/2023 15:04, Walker Chen wrote:
> The DMA controller needs two reset items to work properly on JH7110 SoC,
> so there is need to constrain the items' value to 2, other platforms
> have 1 reset item at most.
> 
> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
> ---
>  .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> index ad107a4d3b33..d8b5439f215c 100644
> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
> @@ -12,14 +12,12 @@ maintainers:
>  description:
>    Synopsys DesignWare AXI DMA Controller DT Binding
>  
> -allOf:
> -  - $ref: "dma-controller.yaml#"
> -
>  properties:
>    compatible:
>      enum:
>        - snps,axi-dma-1.01a
>        - intel,kmb-axi-dma
> +      - starfive,jh7110-axi-dma
>  
>    reg:
>      minItems: 1
> @@ -58,7 +56,8 @@ properties:
>      maximum: 8
>  
>    resets:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
>  
>    snps,dma-masters:
>      description: |
> @@ -109,6 +108,23 @@ required:
>    - snps,priority
>    - snps,block-size
>  
> +allOf:
> +  - $ref: "dma-controller.yaml#"

Rebase your patches on something recent... I would argue that it should
be based on maintainer's (or linux-next) tree, but that would be too
good to be true.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - starfive,jh7110-axi-dma
> +    then:
> +      properties:
> +        resets:
> +          minItems: 2

What are the items expected here?

> +    else:
> +      properties:
> +        resets:

Best regards,
Krzysztof
Walker Chen March 7, 2023, 10:30 a.m. UTC | #2
On 2023/3/7 17:03, Krzysztof Kozlowski wrote:
> On 06/03/2023 15:04, Walker Chen wrote:
>> The DMA controller needs two reset items to work properly on JH7110 SoC,
>> so there is need to constrain the items' value to 2, other platforms
>> have 1 reset item at most.
>> 
>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
>> ---
>>  .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
>>  1 file changed, 20 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>> index ad107a4d3b33..d8b5439f215c 100644
>> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>> @@ -12,14 +12,12 @@ maintainers:
>>  description:
>>    Synopsys DesignWare AXI DMA Controller DT Binding
>>  
>> -allOf:
>> -  - $ref: "dma-controller.yaml#"
>> -
>>  properties:
>>    compatible:
>>      enum:
>>        - snps,axi-dma-1.01a
>>        - intel,kmb-axi-dma
>> +      - starfive,jh7110-axi-dma
>>  
>>    reg:
>>      minItems: 1
>> @@ -58,7 +56,8 @@ properties:
>>      maximum: 8
>>  
>>    resets:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 2
>>  
>>    snps,dma-masters:
>>      description: |
>> @@ -109,6 +108,23 @@ required:
>>    - snps,priority
>>    - snps,block-size
>>  
>> +allOf:
>> +  - $ref: "dma-controller.yaml#"
> 
> Rebase your patches on something recent... I would argue that it should
> be based on maintainer's (or linux-next) tree, but that would be too
> good to be true.

This was written by referring to the syntax of other dt-binding, but your suggestion 
is a good one, the next version of patches will be rebased on kernel 6.3.

> 
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - starfive,jh7110-axi-dma
>> +    then:
>> +      properties:
>> +        resets:
>> +          minItems: 2
> 
> What are the items expected here?

Do you mean to add descriptions for items here ?

Thanks!

Best regards,
Walker
Krzysztof Kozlowski March 7, 2023, 3:51 p.m. UTC | #3
On 07/03/2023 11:30, Walker Chen wrote:
> 
> 
> On 2023/3/7 17:03, Krzysztof Kozlowski wrote:
>> On 06/03/2023 15:04, Walker Chen wrote:
>>> The DMA controller needs two reset items to work properly on JH7110 SoC,
>>> so there is need to constrain the items' value to 2, other platforms
>>> have 1 reset item at most.
>>>
>>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
>>> ---
>>>  .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
>>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>> index ad107a4d3b33..d8b5439f215c 100644
>>> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>> @@ -12,14 +12,12 @@ maintainers:
>>>  description:
>>>    Synopsys DesignWare AXI DMA Controller DT Binding
>>>  
>>> -allOf:
>>> -  - $ref: "dma-controller.yaml#"
>>> -
>>>  properties:
>>>    compatible:
>>>      enum:
>>>        - snps,axi-dma-1.01a
>>>        - intel,kmb-axi-dma
>>> +      - starfive,jh7110-axi-dma
>>>  
>>>    reg:
>>>      minItems: 1
>>> @@ -58,7 +56,8 @@ properties:
>>>      maximum: 8
>>>  
>>>    resets:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    maxItems: 2
>>>  
>>>    snps,dma-masters:
>>>      description: |
>>> @@ -109,6 +108,23 @@ required:
>>>    - snps,priority
>>>    - snps,block-size
>>>  
>>> +allOf:
>>> +  - $ref: "dma-controller.yaml#"
>>
>> Rebase your patches on something recent... I would argue that it should
>> be based on maintainer's (or linux-next) tree, but that would be too
>> good to be true.
> 
> This was written by referring to the syntax of other dt-binding, but your suggestion 
> is a good one, the next version of patches will be rebased on kernel 6.3.

Rebasing on old kernel was referring to syntax of other binding? I don't
understand this. How old code which you copied is related anyhow to
other binding? You are expected to send patches always based on recent
one, not something old.

> 
>>
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - starfive,jh7110-axi-dma
>>> +    then:
>>> +      properties:
>>> +        resets:
>>> +          minItems: 2
>>
>> What are the items expected here?
> 
> Do you mean to add descriptions for items here ?

Yes, because order of the items is fixed.



Best regards,
Krzysztof
Walker Chen March 8, 2023, 1:08 a.m. UTC | #4
On 2023/3/7 23:51, Krzysztof Kozlowski wrote:
> On 07/03/2023 11:30, Walker Chen wrote:
>> 
>> 
>> On 2023/3/7 17:03, Krzysztof Kozlowski wrote:
>>> On 06/03/2023 15:04, Walker Chen wrote:
>>>> The DMA controller needs two reset items to work properly on JH7110 SoC,
>>>> so there is need to constrain the items' value to 2, other platforms
>>>> have 1 reset item at most.
>>>>
>>>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
>>>> ---
>>>>  .../bindings/dma/snps,dw-axi-dmac.yaml        | 24 +++++++++++++++----
>>>>  1 file changed, 20 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>>> index ad107a4d3b33..d8b5439f215c 100644
>>>> --- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>>> +++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
>>>> @@ -12,14 +12,12 @@ maintainers:
>>>>  description:
>>>>    Synopsys DesignWare AXI DMA Controller DT Binding
>>>>  
>>>> -allOf:
>>>> -  - $ref: "dma-controller.yaml#"
>>>> -
>>>>  properties:
>>>>    compatible:
>>>>      enum:
>>>>        - snps,axi-dma-1.01a
>>>>        - intel,kmb-axi-dma
>>>> +      - starfive,jh7110-axi-dma
>>>>  
>>>>    reg:
>>>>      minItems: 1
>>>> @@ -58,7 +56,8 @@ properties:
>>>>      maximum: 8
>>>>  
>>>>    resets:
>>>> -    maxItems: 1
>>>> +    minItems: 1
>>>> +    maxItems: 2
>>>>  
>>>>    snps,dma-masters:
>>>>      description: |
>>>> @@ -109,6 +108,23 @@ required:
>>>>    - snps,priority
>>>>    - snps,block-size
>>>>  
>>>> +allOf:
>>>> +  - $ref: "dma-controller.yaml#"
>>>
>>> Rebase your patches on something recent... I would argue that it should
>>> be based on maintainer's (or linux-next) tree, but that would be too
>>> good to be true.
>> 
>> This was written by referring to the syntax of other dt-binding, but your suggestion 
>> is a good one, the next version of patches will be rebased on kernel 6.3.
> 
> Rebasing on old kernel was referring to syntax of other binding? I don't
> understand this. How old code which you copied is related anyhow to
> other binding? You are expected to send patches always based on recent
> one, not something old.

Okay, I understand what you mean.

> 
>> 
>>>
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - starfive,jh7110-axi-dma
>>>> +    then:
>>>> +      properties:
>>>> +        resets:
>>>> +          minItems: 2
>>>
>>> What are the items expected here?
>> 
>> Do you mean to add descriptions for items here ?
> 
> Yes, because order of the items is fixed.

Thanks!

Best regards,
Walker
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
index ad107a4d3b33..d8b5439f215c 100644
--- a/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
+++ b/Documentation/devicetree/bindings/dma/snps,dw-axi-dmac.yaml
@@ -12,14 +12,12 @@  maintainers:
 description:
   Synopsys DesignWare AXI DMA Controller DT Binding
 
-allOf:
-  - $ref: "dma-controller.yaml#"
-
 properties:
   compatible:
     enum:
       - snps,axi-dma-1.01a
       - intel,kmb-axi-dma
+      - starfive,jh7110-axi-dma
 
   reg:
     minItems: 1
@@ -58,7 +56,8 @@  properties:
     maximum: 8
 
   resets:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   snps,dma-masters:
     description: |
@@ -109,6 +108,23 @@  required:
   - snps,priority
   - snps,block-size
 
+allOf:
+  - $ref: "dma-controller.yaml#"
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - starfive,jh7110-axi-dma
+    then:
+      properties:
+        resets:
+          minItems: 2
+    else:
+      properties:
+        resets:
+          maxItems: 1
+
 additionalProperties: false
 
 examples: