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. If property is not set, reference
clock needs to be provided externally.
Signed-off-by: Ante Knezic <ante.knezic@helmholz.de>
---
drivers/net/dsa/microchip/ksz8795.c | 5 +++++
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, 12 insertions(+)
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 91aba470fb2f..78f3a668aa99 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -1434,6 +1434,11 @@ 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))
+ ksz_cfg(dev, KSZ88X3_REG_FVID_AND_HOST_MODE,
+ KSZ88X3_PORT3_RMII_CLK_INTERNAL,
+ dev->rmii_clk_internal);
+
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 3c9dae53e4d8..beca974e0171 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 KSZ88X3_REG_FVID_AND_HOST_MODE 0xC6
+#define KSZ88X3_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 b800ace40ce1..0a0a53ce5b1b 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -4160,6 +4160,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 8842efca0871..e5b0445fe2ca 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -163,6 +163,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.0Commit message: "net: dsa: microchip: ".
On Thu, Oct 12, 2023 at 12:55:55PM +0200, Ante Knezic wrote:
> 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. If property is not set, reference
> clock needs to be provided externally.
>
> Signed-off-by: Ante Knezic <ante.knezic@helmholz.de>
> ---
> drivers/net/dsa/microchip/ksz8795.c | 5 +++++
> 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, 12 insertions(+)
>
> diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> index 91aba470fb2f..78f3a668aa99 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -1434,6 +1434,11 @@ 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))
> + ksz_cfg(dev, KSZ88X3_REG_FVID_AND_HOST_MODE,
> + KSZ88X3_PORT3_RMII_CLK_INTERNAL,
> + dev->rmii_clk_internal);
> +
Can this be done in dev->dev_ops->phylink_mac_config() (which so far has no implementation)
for port 3 of ksz88x3?
> 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 3c9dae53e4d8..beca974e0171 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 KSZ88X3_REG_FVID_AND_HOST_MODE 0xC6
> +#define KSZ88X3_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 b800ace40ce1..0a0a53ce5b1b 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -4160,6 +4160,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");
Port property.
> +
> 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 8842efca0871..e5b0445fe2ca 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -163,6 +163,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
On Mon, 16 Oct 2023 13:37:08 +0300, Vladimir Oltean wrote:
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
> > index 91aba470fb2f..78f3a668aa99 100644
> > --- a/drivers/net/dsa/microchip/ksz8795.c
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -1434,6 +1434,11 @@ 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))
> > + ksz_cfg(dev, KSZ88X3_REG_FVID_AND_HOST_MODE,
> > + KSZ88X3_PORT3_RMII_CLK_INTERNAL,
> > + dev->rmii_clk_internal);
> > +
>
> Can this be done in dev->dev_ops->phylink_mac_config() (which so far has no implementation)
> for port 3 of ksz88x3?
> > 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 3c9dae53e4d8..beca974e0171 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 KSZ88X3_REG_FVID_AND_HOST_MODE 0xC6
> > +#define KSZ88X3_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 b800ace40ce1..0a0a53ce5b1b 100644
> > --- a/drivers/net/dsa/microchip/ksz_common.c
> > +++ b/drivers/net/dsa/microchip/ksz_common.c
> > @@ -4160,6 +4160,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");
>
> Port property.
Yes, I guess we can do it in phylink_mac_config. Or perhaps it would be better
to put everything in ksz8_port_setup if you suggest this is a port specific
property and not global?
Something like:
@@ -1312,8 +1314,15 @@ void ksz8_port_setup(struct ksz_device *dev, int port, bool cpu_port)
ksz_port_cfg(dev, port, P_PRIO_CTRL, PORT_802_1P_ENABLE, true);
if (cpu_port) {
- if (!ksz_is_ksz88x3(dev))
+ if (!ksz_is_ksz88x3(dev)) {
ksz8795_cpu_interface_select(dev, port);
+ } else {
+ dev->rmii_clk_internal = of_property_read_bool(dev->dev->of_node,
+ "microchip,rmii-clk-internal");
+ ksz_cfg(dev, KSZ88X3_REG_FVID_AND_HOST_MODE,
+ KSZ88X3_PORT3_RMII_CLK_INTERNAL,
+ dev->rmii_clk_internal);
+ }
> > + 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 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:
>> 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.
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. >
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 :)
© 2016 - 2025 Red Hat, Inc.