linux-kernel - Re: [PATCH v4 01/11] dt-bindings: net: add STM32MP13 compatible in documentation for stm32

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c6ff5778-f928-4a65-8a32-a3582d9d8f94@linaro.org>
Date: Wed, 5 Jun 2024 13:46:33 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Christophe ROULLIER <christophe.roullier@...s.st.com>,
 "David S . Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Richard Cochran <richardcochran@...il.com>, Jose Abreu
 <joabreu@...opsys.com>, Liam Girdwood <lgirdwood@...il.com>,
 Mark Brown <broonie@...nel.org>, Marek Vasut <marex@...x.de>
Cc: netdev@...r.kernel.org, devicetree@...r.kernel.org,
 linux-stm32@...md-mailman.stormreply.com,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 01/11] dt-bindings: net: add STM32MP13 compatible in
 documentation for stm32

On 05/06/2024 11:55, Christophe ROULLIER wrote:
> 
> On 6/5/24 10:14, Krzysztof Kozlowski wrote:
>> On 04/06/2024 16:34, Christophe Roullier wrote:
>>> New STM32 SOC have 2 GMACs instances.
>>> GMAC IP version is SNPS 4.20.
>>>
>>> Signed-off-by: Christophe Roullier <christophe.roullier@...s.st.com>
>>> ---
>>>   .../devicetree/bindings/net/stm32-dwmac.yaml  | 41 +++++++++++++++----
>>>   1 file changed, 34 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>>> index 7ccf75676b6d5..ecbed9a7aaf6d 100644
>>> --- a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>>> +++ b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>>> @@ -22,18 +22,17 @@ select:
>>>           enum:
>>>             - st,stm32-dwmac
>>>             - st,stm32mp1-dwmac
>>> +          - st,stm32mp13-dwmac
>>>     required:
>>>       - compatible
>>>   
>>> -allOf:
>>> -  - $ref: snps,dwmac.yaml#
>>> -
>>>   properties:
>>>     compatible:
>>>       oneOf:
>>>         - items:
>>>             - enum:
>>>                 - st,stm32mp1-dwmac
>>> +              - st,stm32mp13-dwmac
>>>             - const: snps,dwmac-4.20a
>>>         - items:
>>>             - enum:
>>> @@ -75,12 +74,15 @@ properties:
>>>     st,syscon:
>>>       $ref: /schemas/types.yaml#/definitions/phandle-array
>>>       items:
>>> -      - items:
>>> +      - minItems: 2
>>> +        items:
>>>             - description: phandle to the syscon node which encompases the glue register
>>>             - description: offset of the control register
>>> +          - description: field to set mask in register
>>>       description:
>>>         Should be phandle/offset pair. The phandle to the syscon node which
>>> -      encompases the glue register, and the offset of the control register
>>> +      encompases the glue register, the offset of the control register and
>>> +      the mask to set bitfield in control register
>>>   
>>>     st,ext-phyclk:
>>>       description:
>>> @@ -112,12 +114,37 @@ required:
>>>   
>>>   unevaluatedProperties: false
>>>   
>>> +allOf:
>>> +  - $ref: snps,dwmac.yaml#
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - st,stm32mp1-dwmac
>>> +              - st,stm32-dwmac
>>> +    then:
>>> +      properties:
>>> +        st,syscon:
>>> +          items:
>>> +            maxItems: 2
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - st,stm32mp13-dwmac
>>> +    then:
>>> +      properties:
>>> +        st,syscon:
>>> +          items:
>>> +            minItems: 3
>> I don't think this works. You now constrain the first dimension which
>> had only one item before.
>>
>> Make your example complete and test it.
>>
>> Best regards,
>> Krzysztof
> 
> Hi Krzysztof,
> 
> "Official" bindings for MP15: st,syscon = <&syscfg 0x4>;
> "Official" bindings for MP13: st,syscon = <&syscfg 0x4 0xff0000>; or 
> st,syscon = <&syscfg 0x4 0xff000000>;
> 
> If I execute make dt_binding_check 
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/stm32-dwmac.yaml with:
> 
>     For MP15: st,syscon = <&syscfg>; 
> =>bindings/net/stm32-dwmac.example.dtb: ethernet@...27000: st,syscon:0: 
> [4294967295] is too short
> 
>     For MP15: st,syscon = <&syscfg 0x4 0xff0000>; 
> =>devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@...27000: 
> st,syscon:0: [4294967295, 4, 16711680] is too long
> 
>     For MP13: st,syscon = <&syscfg 0x4>; => 
> devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@...0a000: 
> st,syscon:0: [4294967295, 4] is too short
> 
>     For MP13: st,syscon = <&syscfg 0x4 0xff0000 0xff>; => 
> devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@...0a000: 
> st,syscon:0: [4294967295, 4, 16711680, 255] is too long
> 
> So it is seems good :-)

Code is still incorrect, although will work because of how schema parses
matrix. But even by looking it is not symmetrical between allOf:if:then
and properties:. Make it symmetric - apply the number of items on the
second dimension.

Best regards,
Krzysztof