2023-03-06 14:05:20

by Walker Chen

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

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 <[email protected]>
---
.../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#"
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - starfive,jh7110-axi-dma
+ then:
+ properties:
+ resets:
+ minItems: 2
+ else:
+ properties:
+ resets:
+ maxItems: 1
+
additionalProperties: false

examples:
--
2.17.1



2023-03-07 09:04:04

by Krzysztof Kozlowski

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

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 <[email protected]>
> ---
> .../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


2023-03-07 10:31:24

by Walker Chen

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



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 <[email protected]>
>> ---
>> .../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

2023-03-07 15:52:02

by Krzysztof Kozlowski

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

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 <[email protected]>
>>> ---
>>> .../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


2023-03-08 01:08:53

by Walker Chen

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



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 <[email protected]>
>>>> ---
>>>> .../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