1. Patchew
  2. linux
  3. [PATCH net-next 0/2] net: dsa: microchip: enable setting rmii reference
  4. View patch

[PATCH net-next 2/2] dt-bindings: net: microchip,ksz: document microchip,rmii-clk-internal

Ante Knezic posted 2 patches 2 years, 2 months ago
There is a newer version of this series
[PATCH net-next 2/2] dt-bindings: net: microchip,ksz: document microchip,rmii-clk-internal
Posted by Ante Knezic 2 years, 2 months ago 
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.
+
 required:
   - compatible
   - reg
-- 
2.11.0
Re: [PATCH net-next 2/2] dt-bindings: net: microchip,ksz: document microchip,rmii-clk-internal
Posted by Conor Dooley 2 years, 2 months ago 
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
> 
>
[PATCH net-next 2/2] dt-bindings: net: microchip,ksz: document microchip,rmii-clk-internal
Posted by Ante Knezic 2 years, 2 months ago 
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
Re: [PATCH net-next 2/2] dt-bindings: net: microchip,ksz: document microchip,rmii-clk-internal
Posted by Conor Dooley 2 years, 2 months ago 
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.
Re: [PATCH net-next 2/2] dt-bindings: net: microchip,ksz: document microchip,rmii-clk-internal
Posted by Andrew Lunn 2 years, 2 months ago 
> +  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
[PATCH net-next 2/2] dt-bindings: net: microchip,ksz: document microchip,rmii-clk-internal
Posted by Ante Knezic 2 years, 2 months ago 
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?
Re: [PATCH net-next 2/2] dt-bindings: net: microchip,ksz: document microchip,rmii-clk-internal
Posted by Andrew Lunn 2 years, 2 months ago 
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
[PATCH net-next 2/2] dt-bindings: net: microchip,ksz: document microchip,rmii-clk-internal
Posted by Ante Knezic 2 years, 2 months ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.