diff mbox series

[v4,2/7] dt-bindings: net: snps,dwmac: Update the maxitems number of resets and reset-names

Message ID 20230118061701.30047-3-yanhong.wang@starfivetech.com (mailing list archive)
State Not Applicable
Delegated to: Conor Dooley
Headers show
Series Add Ethernet driver for StarFive JH7110 SoC | expand

Checks

Context Check Description
conchuod/tree_selection fail Failed to apply to next/pending-fixes or riscv/for-next

Commit Message

yanhong wang Jan. 18, 2023, 6:16 a.m. UTC
Some boards(such as StarFive VisionFive v2) require more than one value
which defined by resets property, so the original definition can not
meet the requirements. In order to adapt to different requirements,
adjust the maxitems number definition.

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

Comments

Rob Herring (Arm) Jan. 18, 2023, 1:10 p.m. UTC | #1
On Wed, 18 Jan 2023 14:16:56 +0800, Yanhong Wang wrote:
> Some boards(such as StarFive VisionFive v2) require more than one value
> which defined by resets property, so the original definition can not
> meet the requirements. In order to adapt to different requirements,
> adjust the maxitems number definition.
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.example.dtb: ethernet@1c0b000: Unevaluated properties are not allowed ('reset-names' was unexpected)
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.example.dtb: ethernet@1c0b000: Unevaluated properties are not allowed ('reset-names' was unexpected)
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.example.dtb: ethernet@1c0b000: Unevaluated properties are not allowed ('reset-names' was unexpected)
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230118061701.30047-3-yanhong.wang@starfivetech.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Krzysztof Kozlowski Jan. 18, 2023, 3:46 p.m. UTC | #2
On 18/01/2023 07:16, Yanhong Wang wrote:
> Some boards(such as StarFive VisionFive v2) require more than one value
> which defined by resets property, so the original definition can not
> meet the requirements. In order to adapt to different requirements,
> adjust the maxitems number definition.
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index e26c3e76ebb7..baf2c5b9e92d 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -133,12 +133,9 @@ properties:
>          - ptp_ref
>  
>    resets:
> -    maxItems: 1
> -    description:
> -      MAC Reset signal.
> -
> -  reset-names:
> -    const: stmmaceth
> +    minItems: 1
> +    maxItems: 3
> +    additionalItems: true

NAK, because I told you twice to drop this one. So this is third time -
drop this line...

Best regards,
Krzysztof
Krzysztof Kozlowski Jan. 18, 2023, 3:47 p.m. UTC | #3
On 18/01/2023 07:16, Yanhong Wang wrote:
> Some boards(such as StarFive VisionFive v2) require more than one value
> which defined by resets property, so the original definition can not
> meet the requirements. In order to adapt to different requirements,
> adjust the maxitems number definition.
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index e26c3e76ebb7..baf2c5b9e92d 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -133,12 +133,9 @@ properties:
>          - ptp_ref
>  
>    resets:
> -    maxItems: 1

Also, this does not make sense on its own and messes constraints for all
other users. So another no for entire patch.

Best regards,
Krzysztof
yanhong wang Feb. 7, 2023, 2:43 a.m. UTC | #4
On 2023/1/18 23:47, Krzysztof Kozlowski wrote:
> On 18/01/2023 07:16, Yanhong Wang wrote:
>> Some boards(such as StarFive VisionFive v2) require more than one value
>> which defined by resets property, so the original definition can not
>> meet the requirements. In order to adapt to different requirements,
>> adjust the maxitems number definition.
>> 
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> ---
>>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 9 +++------
>>  1 file changed, 3 insertions(+), 6 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> index e26c3e76ebb7..baf2c5b9e92d 100644
>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> @@ -133,12 +133,9 @@ properties:
>>          - ptp_ref
>>  
>>    resets:
>> -    maxItems: 1
> 
> Also, this does not make sense on its own and messes constraints for all
> other users. So another no for entire patch.
> 

Thanks. Change the properties of 'resets' and reset-names like this:

  resets:
    minItems: 1
    maxItems: 2

  reset-names:
    minItems: 1
    maxItems: 2

Is it right?  Do you have any other better suggestions?

> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Feb. 7, 2023, 7:59 a.m. UTC | #5
On 07/02/2023 03:43, yanhong wang wrote:
> 
> 
> On 2023/1/18 23:47, Krzysztof Kozlowski wrote:
>> On 18/01/2023 07:16, Yanhong Wang wrote:
>>> Some boards(such as StarFive VisionFive v2) require more than one value
>>> which defined by resets property, so the original definition can not
>>> meet the requirements. In order to adapt to different requirements,
>>> adjust the maxitems number definition.
>>>
>>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>>> ---
>>>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 9 +++------
>>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>> index e26c3e76ebb7..baf2c5b9e92d 100644
>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>> @@ -133,12 +133,9 @@ properties:
>>>          - ptp_ref
>>>  
>>>    resets:
>>> -    maxItems: 1
>>
>> Also, this does not make sense on its own and messes constraints for all
>> other users. So another no for entire patch.
>>
> 
> Thanks. Change the properties of 'resets' and reset-names like this:
> 
>   resets:
>     minItems: 1
>     maxItems: 2
> 
>   reset-names:
>     minItems: 1
>     maxItems: 2
> 
> Is it right?  Do you have any other better suggestions?

Isn't this allowing two reset items for every variant of snps,dwmac?

Best regards,
Krzysztof
yanhong wang Feb. 15, 2023, 7:46 a.m. UTC | #6
On 2023/2/7 15:59, Krzysztof Kozlowski wrote:
> On 07/02/2023 03:43, yanhong wang wrote:
>> 
>> 
>> On 2023/1/18 23:47, Krzysztof Kozlowski wrote:
>>> On 18/01/2023 07:16, Yanhong Wang wrote:
>>>> Some boards(such as StarFive VisionFive v2) require more than one value
>>>> which defined by resets property, so the original definition can not
>>>> meet the requirements. In order to adapt to different requirements,
>>>> adjust the maxitems number definition.
>>>>
>>>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 9 +++------
>>>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>> index e26c3e76ebb7..baf2c5b9e92d 100644
>>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>> @@ -133,12 +133,9 @@ properties:
>>>>          - ptp_ref
>>>>  
>>>>    resets:
>>>> -    maxItems: 1
>>>
>>> Also, this does not make sense on its own and messes constraints for all
>>> other users. So another no for entire patch.
>>>
>> 
>> Thanks. Change the properties of 'resets' and reset-names like this:
>> 
>>   resets:
>>     minItems: 1
>>     maxItems: 2
>> 
>>   reset-names:
>>     minItems: 1
>>     maxItems: 2
>> 
>> Is it right?  Do you have any other better suggestions?
> 
> Isn't this allowing two reset items for every variant of snps,dwmac?
> 

Sorry for not getting back to you faster.
After referring to the above modification, i used the command 'make DT_CHECKER_FLAGS=-m dt_binding_check'
to check all the bindings(including 'starfive,jh7110-dwmac.yaml'), no errors are reported,
and the errors reported by Rob Herring are gone.

> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Feb. 16, 2023, 8:13 a.m. UTC | #7
On 15/02/2023 08:46, yanhong wang wrote:
> 
> 
> On 2023/2/7 15:59, Krzysztof Kozlowski wrote:
>> On 07/02/2023 03:43, yanhong wang wrote:
>>>
>>>
>>> On 2023/1/18 23:47, Krzysztof Kozlowski wrote:
>>>> On 18/01/2023 07:16, Yanhong Wang wrote:
>>>>> Some boards(such as StarFive VisionFive v2) require more than one value
>>>>> which defined by resets property, so the original definition can not
>>>>> meet the requirements. In order to adapt to different requirements,
>>>>> adjust the maxitems number definition.
>>>>>
>>>>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/net/snps,dwmac.yaml | 9 +++------
>>>>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>>> index e26c3e76ebb7..baf2c5b9e92d 100644
>>>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>>> @@ -133,12 +133,9 @@ properties:
>>>>>          - ptp_ref
>>>>>  
>>>>>    resets:
>>>>> -    maxItems: 1
>>>>
>>>> Also, this does not make sense on its own and messes constraints for all
>>>> other users. So another no for entire patch.
>>>>
>>>
>>> Thanks. Change the properties of 'resets' and reset-names like this:
>>>
>>>   resets:
>>>     minItems: 1
>>>     maxItems: 2
>>>
>>>   reset-names:
>>>     minItems: 1
>>>     maxItems: 2
>>>
>>> Is it right?  Do you have any other better suggestions?
>>
>> Isn't this allowing two reset items for every variant of snps,dwmac?
>>
> 
> Sorry for not getting back to you faster.
> After referring to the above modification, i used the command 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> to check all the bindings(including 'starfive,jh7110-dwmac.yaml'), no errors are reported,
> and the errors reported by Rob Herring are gone.

I don't see how does it answer my question. I claim you loosen the
constraints and allow now two resets for everyone. You say you don't see
errors. I never claimed there will be errors. I claimed what I said -
you allow now to reset everywhere, which might not be correct
description of every hardware.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index e26c3e76ebb7..baf2c5b9e92d 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -133,12 +133,9 @@  properties:
         - ptp_ref
 
   resets:
-    maxItems: 1
-    description:
-      MAC Reset signal.
-
-  reset-names:
-    const: stmmaceth
+    minItems: 1
+    maxItems: 3
+    additionalItems: true
 
   power-domains:
     maxItems: 1