Re: [PATCH v3 01/12] dt-bindings: net: add STM32MP13 compatible in documentation for stm32

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Conor Dooley <conor@kernel.org>,
	Christophe Roullier <christophe.roullier@foss.st.com>
Cc: "David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Richard Cochran <richardcochran@gmail.com>,
	Jose Abreu <joabreu@synopsys.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	netdev@vger.kernel.org, devicetree@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 01/12] dt-bindings: net: add STM32MP13 compatible in documentation for stm32
Date: Sat, 30 Sep 2023 17:46:03 +0200	[thread overview]
Message-ID: <46cf84f1-060b-4f9f-9c9c-edf504bf03ca@linaro.org> (raw)
In-Reply-To: <20230928-footwork-padlock-524173c3e205@spud>

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@foss.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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel