Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Date:   Tue, 17 Oct 2023 08:48:27 +0100
From:   Conor Dooley <conor@kernel.org>
To:     Ante Knezic <ante.knezic@helmholz.de>
Cc:     olteanv@gmail.com, andrew@lunn.ch, conor+dt@kernel.org,
        davem@davemloft.net, devicetree@vger.kernel.org,
        edumazet@google.com, f.fainelli@gmail.com,
        krzysztof.kozlowski+dt@linaro.org, kuba@kernel.org,
        linux-kernel@vger.kernel.org, marex@denx.de,
        netdev@vger.kernel.org, pabeni@redhat.com, robh+dt@kernel.org,
        woojung.huh@microchip.com
Subject: Re: [PATCH net-next v2 2/2] dt-bindings: net: microchip,ksz:
 document microchip,rmii-clk-internal
Message-ID: <20231017-fading-retrial-7faebda79ebb@spud>
References: <20231016103708.6ka5vxfkdatrjvdk@skbuf>
 <20231017073548.15050-1-ante.knezic@helmholz.de>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="wIO3iCZ/QV7V4j5D"
Content-Disposition: inline
In-Reply-To: <20231017073548.15050-1-ante.knezic@helmholz.de>
Precedence: bulk


--wIO3iCZ/QV7V4j5D
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Oct 17, 2023 at 09:35:48AM +0200, Ante Knezic wrote:
> > > +  microchip,rmii-clk-internal:
> > > +    $ref: /schemas/types.yaml#/definitions/flag
> > > +    description:
> > > +      Set if the RMII reference clock is provided internally. Otherw=
ise
> > > +      reference clock should be provided externally.
> > > +
> > > +if:
> > > +  not:
> > > +    properties:
> > > +      compatible:
> > > +        enum:
> > > +          - microchip,ksz8863
> > > +          - microchip,ksz8873
> > > +then:
> > > +  not:
> > > +    required:
> > > +      - microchip,rmii-clk-internal

I think this bit can become the slightly simpler
	then:
	  properties:
	     microchip,rmii-clk-internal: false

> > I think that what you want to express is that microchip,rmii-clk-intern=
al
> > is only defined for microchip,ksz8863 and microchip,ksz8873.
> > Can't you describe that as "if: properties: compatible: (...) then:
> > properties: microchip,rmii-clk-internal"?
>=20
> If I understood you correctly you are refering to a solution like
> if:
>   properties:
>     compatible:
>       enum:
>         - microchip,ksz8863
>         - microchip,ksz8873
> then:
>   properties:
>     microchip,rmii-clk-internal:
>       $ref: /schemas/types.yaml#/definitions/flag
>       description:
>         Set if the RMII reference clock is provided internally. Otherwise
>         reference clock should be provided externally.
>=20
> This was already suggested in v1, but was not a satisfactory solution
> according to Mr. Conor Dooley:

Yeah, we prefer not to have the property definitions inside the
conditionals, but rather constrain or allow/disallow them there.

Cheers,
Conor.

>=20
> >> On Tue, 10 Oct 2023 16:25:55 +0100, Conor Dooley wrote:
> >> > On Tue, Oct 10, 2023 at 03:18:54PM +0200, Ante Knezic wrote:
> >> > > Add documentation for selecting reference rmii clock on KSZ88X3 de=
vices
> >> > >=20
> >> > > Signed-off-by: Ante Knezic <ante.knezic@helmholz.de>
> >> > > ---
> >> > >  Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml | 6 =
++++++
> >> > >  1 file changed, 6 insertions(+)
> >> > >=20
> >> > > diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,k=
sz.yaml b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> >> > > index e51be1ac0362..3df5d2e72dba 100644
> >> > > --- a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> >> > > +++ b/Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml
> >> > > @@ -49,6 +49,12 @@ properties:
> >> > >        Set if the output SYNCLKO clock should be disabled. Do not =
mix with
> >> > >        microchip,synclko-125.
> >> > > =20
> >> > > +  microchip,rmii-clk-internal:
> >> > > +    $ref: /schemas/types.yaml#/definitions/flag
> >> > > +    description:
> >> > > +      Set if the RMII reference clock should be provided internal=
ly.
> >> >=20
> >> > > Applies only
> >> > > +      to KSZ88X3 devices.
> >> >=20
> >> > This should be enforced by the schema, the example schema in the docs
> >> > should show you how to do this.
> >>=20
> >> I am guessing you are refering to limiting the property to ksz88x3 dev=
ices?
> >> Something like:
> >>=20
> >> if:
> >>   properties:
> >>     compatible:
> >>       enum:
> >>         - microchip,ksz8863
> >>         - microchip,ksz8873
> >> then:
> >>   properties:
> >>     microchip,rmii-clk-internal:
> >>       $ref: /schemas/types.yaml#/definitions/flag
> >>       description:
> >>         Set if the RMII reference clock is provided internally. Otherw=
ise
> >>         reference clock should be provided externally.
> >
> >Not quite. The definition of the property should be outside the if/then,
> >but one should be used to allow/disallow the property.
>=20

--wIO3iCZ/QV7V4j5D
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZS48SwAKCRB4tDGHoIJi
0velAQClJ9EQ6MEhRqxPuOI8Aa3rxhaXsnRNccr4O2vn8y6vLwD/ffOYqRRz1OQ+
vAcG8dVYyfmc7/0OfyJOWaW4keCGvQM=
=/J8x
-----END PGP SIGNATURE-----

--wIO3iCZ/QV7V4j5D--