Messages in this thread | | | Date | Tue, 17 Oct 2023 08:48:27 +0100 | From | Conor Dooley <> | Subject | Re: [PATCH net-next v2 2/2] dt-bindings: net: microchip,ksz: document microchip,rmii-clk-internal |
| |
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
> > 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.
> > >> 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 devices > >> > > > >> > > Signed-off-by: Ante Knezic <ante.knezic@helmholz.de> > >> > > --- > >> > > Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml | 6 ++++++ > >> > > 1 file changed, 6 insertions(+) > >> > > > >> > > diff --git a/Documentation/devicetree/bindings/net/dsa/microchip,ksz.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. > >> > > > >> > > + microchip,rmii-clk-internal: > >> > > + $ref: /schemas/types.yaml#/definitions/flag > >> > > + description: > >> > > + Set if the RMII reference clock should be provided internally. > >> > > >> > > Applies only > >> > > + to KSZ88X3 devices. > >> > > >> > This should be enforced by the schema, the example schema in the docs > >> > should show you how to do this. > >> > >> I am guessing you are refering to limiting the property to ksz88x3 devices? > >> Something 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. > > > >Not quite. The definition of the property should be outside the if/then, > >but one should be used to allow/disallow the property. > [unhandled content-type:application/pgp-signature] | |