linux-kernel - Re: [PATCH v3 01/12] 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: <46cf84f1-060b-4f9f-9c9c-edf504bf03ca@linaro.org>
Date:   Sat, 30 Sep 2023 17:46:03 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Conor Dooley <conor@...nel.org>,
        Christophe Roullier <christophe.roullier@...s.st.com>
Cc:     "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>, 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 v3 01/12] dt-bindings: net: add STM32MP13 compatible in
 documentation for stm32

On 28/09/2023 22:39, Conor Dooley wrote:
> Hey,
> 
> On Thu, Sep 28, 2023 at 05:15:01PM +0200, 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  | 78 +++++++++++++++++--
>>  1 file changed, 70 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/stm32-dwmac.yaml b/Documentation/devicetree/bindings/net/stm32-dwmac.yaml
>> index fc8c96b08d7dc..ca976281bfc22 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:
>> @@ -74,13 +73,10 @@ properties:
>>  
>>    st,syscon:
>>      $ref: /schemas/types.yaml#/definitions/phandle-array
>> -    items:
>> -      - items:
>> -          - description: phandle to the syscon node which encompases the glue register
>> -          - description: offset of the control 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,eth-clk-sel:
>>      description:
>> @@ -101,6 +97,38 @@ required:
>>  
>>  unevaluatedProperties: false
>>  
>> +allOf:
>> +  - $ref: snps,dwmac.yaml#
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - st,stm32mp1-dwmac
>> +              - st,stm32-dwmac
>> +    then:
>> +      properties:
>> +        st,syscon:
>> +          items:
>> +            items:
>> +              - description: phandle to the syscon node which encompases the glue register
>> +              - description: offset of the control register
> 
> These descriptions should, IMO, be moved back out to the st,syscon
> definition. If you put the 3 descriptions there, with "minItems: 2" &
> put "maxItems: 2" and "minItems: 3" in each of the if/then clauses.
> Also, it should be sufficient to simplify to if/then/else.

What's more, same property should not have different types. This syscon
property was way too generic, thus now you have trouble of conflicting
types.

Best regards,
Krzysztof