Received-SPF: pass (google.com: domain of linux-kernel+bounces-203446-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Wed, 5 Jun 2024 18:21:26 -0600
From: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Christophe ROULLIER <christophe.roullier@foss.st.com>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	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>, Marek Vasut <marex@denx.de>,
	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 v4 01/11] dt-bindings: net: add STM32MP13 compatible in
 documentation for stm32
Message-ID: <20240606002126.GA3496044-robh@kernel.org>
References: <20240604143502.154463-1-christophe.roullier@foss.st.com>
 <20240604143502.154463-2-christophe.roullier@foss.st.com>
 <067d41e5-89cf-45eb-8cfa-b6c3cd434f76@linaro.org>
 <70b66190-2c55-4228-8c31-f58a05829d8b@foss.st.com>
 <c6ff5778-f928-4a65-8a32-a3582d9d8f94@linaro.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <c6ff5778-f928-4a65-8a32-a3582d9d8f94@linaro.org>

On Wed, Jun 05, 2024 at 01:46:33PM +0200, Krzysztof Kozlowski wrote:
> 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@foss.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@40027000: st,syscon:0: 
> > [4294967295] is too short
> > 
> >  ?? For MP15: st,syscon = <&syscfg 0x4 0xff0000>; 
> > =>devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@40027000: 
> > st,syscon:0: [4294967295, 4, 16711680] is too long
> > 
> >  ?? For MP13: st,syscon = <&syscfg 0x4>; => 
> > devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@5800a000: 
> > st,syscon:0: [4294967295, 4] is too short
> > 
> >  ?? For MP13: st,syscon = <&syscfg 0x4 0xff0000 0xff>; => 
> > devicetree/bindings/net/stm32-dwmac.example.dtb: ethernet@5800a000: 
> > 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.

It looks correct to me. But it could also be like this:

st,syscon:
  items:
    - minItems: 3

Either way works. Is that what you are asking for? I'm just happy when 
folks can write a working schema.

Rob