| Message ID | df8490e3a39a6daa66c5a0dd266d9f4a388dfe7b.1693482665.git.ante.knezic@helmholz.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: dsa: microchip: enable setting rmii reference | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/apply | fail | Patch does not apply to net-next |
> + 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. It would be good to define what happens when microchip,rmii-clk-internal is not present. Looking at the code, you leave it unchanged. Is that what we want, or do we want to force it to external? Andrew
On Tue, 10 Oct 2023 15:25:44 +0200, Andrew Lunn wrote: >> + 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. > >It would be good to define what happens when >microchip,rmii-clk-internal is not present. Looking at the code, you >leave it unchanged. Is that what we want, or do we want to force it to >external? > > Andrew Default register setting is to use external RMII clock (which is btw only available option for other KSZ devices - as far as I am aware) so I guess theres no need to force it to external clock?
On Tue, Oct 10, 2023 at 03:41:39PM +0200, Ante Knezic wrote: > On Tue, 10 Oct 2023 15:25:44 +0200, Andrew Lunn wrote: > >> + 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. > > > >It would be good to define what happens when > >microchip,rmii-clk-internal is not present. Looking at the code, you > >leave it unchanged. Is that what we want, or do we want to force it to > >external? > > > > Andrew > > Default register setting is to use external RMII clock (which is btw only > available option for other KSZ devices - as far as I am aware) so I guess > theres no need to force it to external clock? We just need to watch out for a bootloader setting it. Or is it really guaranteed to be false, because the DSA driver always does a device reset, removing all existing configuration? I prefer it is unambiguously documented what not having the property means. Andrew
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. Thanks, Conor. > + > required: > - compatible > - reg > -- > 2.11.0 > >
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. Thanks, Ante
On Tue, 10 Oct 2023 15:57:34 +0200 Anrew Lunn wrote: >On Tue, Oct 10, 2023 at 03:41:39PM +0200, Ante Knezic wrote: >> On Tue, 10 Oct 2023 15:25:44 +0200, Andrew Lunn wrote: >> >> + 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. >> > >> >It would be good to define what happens when >> >microchip,rmii-clk-internal is not present. Looking at the code, you >> >leave it unchanged. Is that what we want, or do we want to force it to >> >external? >> > >> > Andrew >> >> Default register setting is to use external RMII clock (which is btw only >> available option for other KSZ devices - as far as I am aware) so I guess >> theres no need to force it to external clock? > >We just need to watch out for a bootloader setting it. Or is it really >guaranteed to be false, because the DSA driver always does a device reset, >removing all existing configuration? > >I prefer it is unambiguously documented what not having the property >means. > > Andrew The bootloader case might be a issue if the reset-gpio property is not defined for the switch. In this case we should probably enforce the value either way. I will do the changes and repost. Thanks for feedback, Ante
On Wed, Oct 11, 2023 at 03:26:00PM +0200, Ante Knezic wrote: > 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.
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. + required: - compatible - reg
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(+)