[PATCH net-next 0/2] net: dsa: microchip: enable setting rmii reference

[PATCH net-next 0/2] net: dsa: microchip: enable setting rmii reference

[Ante Knezic]

KSZ88X3 devices can select between internal and external RMII reference clock.
This patch series introduces new device tree property for setting reference
clock to internal.

Ante Knezic (2):
  net:dsa:microchip add property to select internal RMII reference clock
  dt-bindings: net: microchip,ksz: document microchip,rmii-clk-internal

 Documentation/devicetree/bindings/net/dsa/microchip,ksz.yaml | 6 ++++++
 drivers/net/dsa/microchip/ksz8795.c                          | 4 ++++
 drivers/net/dsa/microchip/ksz8795_reg.h                      | 3 +++
 drivers/net/dsa/microchip/ksz_common.c                       | 3 +++
 drivers/net/dsa/microchip/ksz_common.h                       | 1 +
 5 files changed, 17 insertions(+)

-- 
2.11.0

[PATCH net-next 1/2] net:dsa:microchip: add property to select internal RMII reference clock

[Ante Knezic]

Microchip KSZ8863/KSZ8873 have the ability to select between internal
and external RMII reference clock. By default, reference clock
needs to be provided via REFCLKI_3 pin. If required, device can be
setup to provide RMII clock internally so that REFCLKI_3 pin can be
left unconnected.
Add a new "microchip,rmii-clk-internal" property which will set
RMII clock reference to internal.

Signed-off-by: Ante Knezic <ante.knezic@helmholz.de>
---
 drivers/net/dsa/microchip/ksz8795.c     | 4 ++++
 drivers/net/dsa/microchip/ksz8795_reg.h | 3 +++
 drivers/net/dsa/microchip/ksz_common.c  | 3 +++
 drivers/net/dsa/microchip/ksz_common.h  | 1 +
 4 files changed, 11 insertions(+)

diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 91aba470fb2f..c5df571153ab 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -1434,6 +1434,10 @@ int ksz8_setup(struct dsa_switch *ds)
 	for (i = 0; i < (dev->info->num_vlans / 4); i++)
 		ksz8_r_vlan_entries(dev, i);
 
+	if (ksz_is_ksz88x3(dev) && dev->rmii_clk_internal)
+		ksz_cfg(dev, KSZ8863_REG_FVID_AND_HOST_MODE,
+			KSZ8863_PORT3_RMII_CLK_INTERNAL, true);
+
 	return ksz8_handle_global_errata(ds);
 }
 
diff --git a/drivers/net/dsa/microchip/ksz8795_reg.h b/drivers/net/dsa/microchip/ksz8795_reg.h
index 7a57c6088f80..0a3fff7d6d32 100644
--- a/drivers/net/dsa/microchip/ksz8795_reg.h
+++ b/drivers/net/dsa/microchip/ksz8795_reg.h
@@ -22,6 +22,9 @@
 #define KSZ8863_GLOBAL_SOFTWARE_RESET	BIT(4)
 #define KSZ8863_PCS_RESET		BIT(0)
 
+#define KSZ8863_REG_FVID_AND_HOST_MODE  0xC6
+#define KSZ8863_PORT3_RMII_CLK_INTERNAL BIT(3)
+
 #define REG_SW_CTRL_0			0x02
 
 #define SW_NEW_BACKOFF			BIT(7)
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 6673122266b7..dc8cf59c6dd9 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -3631,6 +3631,9 @@ int ksz_switch_register(struct ksz_device *dev)
 		}
 	}
 
+	dev->rmii_clk_internal = of_property_read_bool(dev->dev->of_node,
+						       "microchip,rmii-clk-internal");
+
 	ret = dsa_register_switch(dev->ds);
 	if (ret) {
 		dev->dev_ops->exit(dev);
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index a4de58847dea..54589736398a 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -157,6 +157,7 @@ struct ksz_device {
 	phy_interface_t compat_interface;
 	bool synclko_125;
 	bool synclko_disable;
+	bool rmii_clk_internal;
 
 	struct vlan_table *vlan_cache;
 
-- 
2.11.0

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

[Ante Knezic]

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

[Andrew Lunn]

> +  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

[Ante Knezic]

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

[Andrew Lunn]

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

[Ante Knezic]

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

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]

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
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Ante Knezic]

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

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1983 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]