Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Tue, 17 Oct 2023 12:31:37 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Conor Dooley <conor@kernel.org>
Cc:     Ante Knezic <ante.knezic@helmholz.de>, 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: <20231017093137.qag7xcjuklekn3ef@skbuf>
References: <20231016103708.6ka5vxfkdatrjvdk@skbuf>
 <20231017073548.15050-1-ante.knezic@helmholz.de>
 <20231017-fading-retrial-7faebda79ebb@spud>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231017-fading-retrial-7faebda79ebb@spud>
Precedence: bulk

On Tue, Oct 17, 2023 at 08:48:27AM +0100, Conor Dooley wrote:
> 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. Otherwise
> > > > +      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

This looks better. I don't understand how the original formulation worked
("not: required:" when the property was never "required" in the first
place - does that do anything?), but I understand how this one does.

> > > I think that what you want to express is that microchip,rmii-clk-internal
> > > is only defined for microchip,ksz8863 and microchip,ksz8873.
> > > Can't you describe that as "if: properties: compatible: (...) then:
> > > properties: microchip,rmii-clk-internal"?
> > 
> > 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.
> > 
> > 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.

Ok, now you know I didn't open the discussion on v1 :)