Micrel Phy KSZ8031 clock select setting in dts

Micrel Phy KSZ8031 clock select setting in dts

[Oliver Graute]

Hello,

I try to enable a Micrel KSZ8031 in my imx6ul board device tree. But i'am
struggeling with the setting for KSZPHY_RMII_REF_CLK_SEL BIT(7). In my
revision of this Micrel KSZ8031 Phy the Bit(7) has to be true. The 0x1f
register must be 0x8180.

How can I configure this register setting into my DTS?

I already checked Documentation/devicetree/bindings/net/micrel.txt

but i'am not sure if this still up to date. There where some reworks
after git commit 86dc1342

some other commits related to this Phy clock setting I checked

commit 1fadee0c3
commit b838b4aced

my non working device tree blob for the phy is:

&fec1 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_enet1>;
	phy-mode = "rmii";
	rmmi-ref-clk-sel = <1>;
	phy-handle = <&ethphy0>;
	status = "okay";

	mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		ethphy0: ethernet-phy@0 {
			compatible = "micrel,ksz8031";
			reg = <0>;
		};
	};
};


some clue how to configure this phy register setting correctly?

Best regards,

Oliver

Re: Micrel Phy KSZ8031 clock select setting in dts

[Sergei Shtylyov]

On 06/17/2016 04:04 PM, Oliver Graute wrote:

> I try to enable a Micrel KSZ8031 in my imx6ul board device tree. But i'am
> struggeling with the setting for KSZPHY_RMII_REF_CLK_SEL BIT(7). In my
> revision of this Micrel KSZ8031 Phy the Bit(7) has to be true. The 0x1f
> register must be 0x8180.
>
> How can I configure this register setting into my DTS?
>
> I already checked Documentation/devicetree/bindings/net/micrel.txt
>
> but i'am not sure if this still up to date. There where some reworks
> after git commit 86dc1342
>
> some other commits related to this Phy clock setting I checked
>
> commit 1fadee0c3
> commit b838b4aced
>
> my non working device tree blob for the phy is:
>
> &fec1 {
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&pinctrl_enet1>;
> 	phy-mode = "rmii";
> 	rmmi-ref-clk-sel = <1>;
> 	phy-handle = <&ethphy0>;
> 	status = "okay";
>
> 	mdio {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
>
> 		ethphy0: ethernet-phy@0 {
> 			compatible = "micrel,ksz8031";
> 			reg = <0>;
> 		};
> 	};
> };
>
>
> some clue how to configure this phy register setting correctly?

    Tried specifying "micrel,rmii-reference-clock-select-25-mhz" property in 
the PHY node?

> Best regards,
>
> Oliver

MBR, Sergei

Re: Micrel Phy KSZ8031 clock select setting in dts

[Oliver Graute]

On 17/06/16, Sergei Shtylyov wrote:
> On 06/17/2016 04:04 PM, Oliver Graute wrote:
> 
> >I try to enable a Micrel KSZ8031 in my imx6ul board device tree. But i'am
> >struggeling with the setting for KSZPHY_RMII_REF_CLK_SEL BIT(7). In my
> >revision of this Micrel KSZ8031 Phy the Bit(7) has to be true. The 0x1f
> >register must be 0x8180.
> >
> >How can I configure this register setting into my DTS?
> >
> >I already checked Documentation/devicetree/bindings/net/micrel.txt
> >
> >but i'am not sure if this still up to date. There where some reworks
> >after git commit 86dc1342
> >
> >some other commits related to this Phy clock setting I checked
> >
> >commit 1fadee0c3
> >commit b838b4aced
> >
> >my non working device tree blob for the phy is:
> >
> >&fec1 {
> >	pinctrl-names = "default";
> >	pinctrl-0 = <&pinctrl_enet1>;
> >	phy-mode = "rmii";
> >	rmmi-ref-clk-sel = <1>;
> >	phy-handle = <&ethphy0>;
> >	status = "okay";
> >
> >	mdio {
> >		#address-cells = <1>;
> >		#size-cells = <0>;
> >
> >		ethphy0: ethernet-phy@0 {
> >			compatible = "micrel,ksz8031";
> >			reg = <0>;
> >		};
> >	};
> >};
> >
> >
> >some clue how to configure this phy register setting correctly?
> 
>    Tried specifying "micrel,rmii-reference-clock-select-25-mhz"
> property in the PHY node?
> 

No, I expect my RMII reference clock on 50 MHz. So I thought that
rmii-reference-clock-select-25-mhz isn't the right setting for me here.

If I manually set bit 7 in the 0x1f register to true The Phy only works
until the next ifconfig eth0 up/down cycle. After the Phy Reset Bit 7 is
false again and Phy isn't working anymore.


Best Regards,

Oliver

Re: Micrel Phy KSZ8031 clock select setting in dts

[Sascha Hauer]

On Sun, Jun 19, 2016 at 09:29:41PM +0200, Oliver Graute wrote:
> On 17/06/16, Sergei Shtylyov wrote:
> > On 06/17/2016 04:04 PM, Oliver Graute wrote:
> > 
> > >I try to enable a Micrel KSZ8031 in my imx6ul board device tree. But i'am
> > >struggeling with the setting for KSZPHY_RMII_REF_CLK_SEL BIT(7). In my
> > >revision of this Micrel KSZ8031 Phy the Bit(7) has to be true. The 0x1f
> > >register must be 0x8180.
> > >
> > >How can I configure this register setting into my DTS?
> > >
> > >I already checked Documentation/devicetree/bindings/net/micrel.txt
> > >
> > >but i'am not sure if this still up to date. There where some reworks
> > >after git commit 86dc1342
> > >
> > >some other commits related to this Phy clock setting I checked
> > >
> > >commit 1fadee0c3
> > >commit b838b4aced
> > >
> > >my non working device tree blob for the phy is:
> > >
> > >&fec1 {
> > >	pinctrl-names = "default";
> > >	pinctrl-0 = <&pinctrl_enet1>;
> > >	phy-mode = "rmii";
> > >	rmmi-ref-clk-sel = <1>;
> > >	phy-handle = <&ethphy0>;
> > >	status = "okay";
> > >
> > >	mdio {
> > >		#address-cells = <1>;
> > >		#size-cells = <0>;
> > >
> > >		ethphy0: ethernet-phy@0 {
> > >			compatible = "micrel,ksz8031";
> > >			reg = <0>;
> > >		};
> > >	};
> > >};
> > >
> > >
> > >some clue how to configure this phy register setting correctly?
> > 
> >    Tried specifying "micrel,rmii-reference-clock-select-25-mhz"
> > property in the PHY node?
> > 
> 
> No, I expect my RMII reference clock on 50 MHz. So I thought that
> rmii-reference-clock-select-25-mhz isn't the right setting for me here.

You misunderstand the meaning of this property. It is not for specifying
25MHz. Instead, it's for specifying the polarity of the
KSZPHY_RMII_REF_CLK_SEL bit.
Background is that the Micrel Phys come with different default input
frequencies. Unfortunately Micrel did not change the default value of
this bit for the different variants, instead they kept the default value
the same and inverted the meaning for the different variants.
Sergei is right, you have to set
micrel,rmii-reference-clock-select-25-mhz.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: Micrel Phy KSZ8031 clock select setting in dts

[Oliver Graute]

On 20/06/16, Sascha Hauer wrote:
> On Sun, Jun 19, 2016 at 09:29:41PM +0200, Oliver Graute wrote:
> > On 17/06/16, Sergei Shtylyov wrote:
> > > On 06/17/2016 04:04 PM, Oliver Graute wrote:
> > > 
> > > >I try to enable a Micrel KSZ8031 in my imx6ul board device tree. But i'am
> > > >struggeling with the setting for KSZPHY_RMII_REF_CLK_SEL BIT(7). In my
> > > >revision of this Micrel KSZ8031 Phy the Bit(7) has to be true. The 0x1f
> > > >register must be 0x8180.
> > > >
> > > >How can I configure this register setting into my DTS?
> > > >
> > > >I already checked Documentation/devicetree/bindings/net/micrel.txt
> > > >
> > > >but i'am not sure if this still up to date. There where some reworks
> > > >after git commit 86dc1342
> > > >
> > > >some other commits related to this Phy clock setting I checked
> > > >
> > > >commit 1fadee0c3
> > > >commit b838b4aced
> > > >
> > > >my non working device tree blob for the phy is:
> > > >
> > > >&fec1 {
> > > >	pinctrl-names = "default";
> > > >	pinctrl-0 = <&pinctrl_enet1>;
> > > >	phy-mode = "rmii";
> > > >	rmmi-ref-clk-sel = <1>;
> > > >	phy-handle = <&ethphy0>;
> > > >	status = "okay";
> > > >
> > > >	mdio {
> > > >		#address-cells = <1>;
> > > >		#size-cells = <0>;
> > > >
> > > >		ethphy0: ethernet-phy@0 {
> > > >			compatible = "micrel,ksz8031";
> > > >			reg = <0>;
> > > >		};
> > > >	};
> > > >};
> > > >
> > > >
> > > >some clue how to configure this phy register setting correctly?
> > > 
> > >    Tried specifying "micrel,rmii-reference-clock-select-25-mhz"
> > > property in the PHY node?
> > > 
> > 
> > No, I expect my RMII reference clock on 50 MHz. So I thought that
> > rmii-reference-clock-select-25-mhz isn't the right setting for me here.
> 
> You misunderstand the meaning of this property. It is not for specifying
> 25MHz. Instead, it's for specifying the polarity of the
> KSZPHY_RMII_REF_CLK_SEL bit.
> Background is that the Micrel Phys come with different default input
> frequencies. Unfortunately Micrel did not change the default value of
> this bit for the different variants, instead they kept the default value
> the same and inverted the meaning for the different variants.
> Sergei is right, you have to set
> micrel,rmii-reference-clock-select-25-mhz.

thx you both for confirming that. Can you tell me also if the clock
statement s fine in my dts?

&fec1 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_enet1>;
	phy-mode = "rmii";
	micrel,rmii-reference-clock-select-25-mhz;
	clocks,rmii-ref;


because the kernel does not pass my printk in the probe function of micrel.c.


clk = devm_clk_get(&phydev->dev, "rmii-ref");
	/* NOTE: clk may be NULL if building without CONFIG_HAVE_CLK */
	if (!IS_ERR_OR_NULL(clk)) {
		printk(KERN_DEBUG "kszphy_probe clk\n");

I checked that CONFIG_HAVE_CLK is enabled.

Best regards,

Oliver

Re: Micrel Phy KSZ8031 clock select setting in dts

[Andrew Lunn]

> &fec1 {
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&pinctrl_enet1>;
> 	phy-mode = "rmii";
> 	micrel,rmii-reference-clock-select-25-mhz;
> 	clocks,rmii-ref;

You are adding phy properties, not MAC properties. Please put them in
the phy node.

    Andrew

Re: Micrel Phy KSZ8031 clock select setting in dts

[Oliver Graute]

On 20/06/16, Andrew Lunn wrote:
> > &fec1 {
> > 	pinctrl-names = "default";
> > 	pinctrl-0 = <&pinctrl_enet1>;
> > 	phy-mode = "rmii";
> > 	micrel,rmii-reference-clock-select-25-mhz;
> > 	clocks,rmii-ref;
> 
> You are adding phy properties, not MAC properties. Please put them in
> the phy node.

yes, you are right. I fixed this and added the clock like sascha and
sergei proposed. (thx to you all)

my dts node now looks like this:

&fec1 {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_enet1>;
	phy-mode = "rmii";
	status = "okay";

	mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		ethphy0: ethernet-phy@0 {
			compatible = "micrel,ksz8031";
			micrel,rmii-reference-clock-select-25-mhz;
			clocks = <&mdc>;
			clock-names = "rmii-ref";
			phy-handle = <&ethphy0>;
			reg = <0>;
		};

	};

	mdc: rmii-ref {
		#clock-cells = <0>;
		compatible ="fixed-clock";
		clock-frequency = <50000000>;
	};
};


But I also needed to invert the behavior of KSZPHY_RMII_REF_CLK_SEL in
the micrel.c driver to get everything working with my revison of Micrel Phy.

If I understood you right this should not be necessary. So something in
dts is still wrong.


diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 7f4e042..198a24f 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -181,10 +181,14 @@ static int kszphy_rmii_clk_sel(struct phy_device *phydev, bool val)
 		return ctrl;
 	}
 
-	if (val)
+	if (val){
+		printk(KERN_DEBUG "if kszphy_rmii_clk_sel val=0x%x \n", val);
 		ctrl |= KSZPHY_RMII_REF_CLK_SEL;
-	else
-		ctrl &= ~KSZPHY_RMII_REF_CLK_SEL;
+	}
+	else{
+		printk(KERN_DEBUG "else kszphy_rmii_clk_sel val=0x%x \n", val);
+		ctrl |= KSZPHY_RMII_REF_CLK_SEL;
+	}

Best Regards,

Oliver

Re: Micrel Phy KSZ8031 clock select setting in dts

[Sascha Hauer]

On Mon, Jun 20, 2016 at 07:14:06PM +0200, Oliver Graute wrote:
> On 20/06/16, Andrew Lunn wrote:
> > > &fec1 {
> > > 	pinctrl-names = "default";
> > > 	pinctrl-0 = <&pinctrl_enet1>;
> > > 	phy-mode = "rmii";
> > > 	micrel,rmii-reference-clock-select-25-mhz;
> > > 	clocks,rmii-ref;
> > 
> > You are adding phy properties, not MAC properties. Please put them in
> > the phy node.
> 
> yes, you are right. I fixed this and added the clock like sascha and
> sergei proposed. (thx to you all)
> 
> my dts node now looks like this:
> 
> &fec1 {
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&pinctrl_enet1>;
> 	phy-mode = "rmii";
> 	status = "okay";
> 
> 	mdio {
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		ethphy0: ethernet-phy@0 {
> 			compatible = "micrel,ksz8031";
> 			micrel,rmii-reference-clock-select-25-mhz;
> 			clocks = <&mdc>;
> 			clock-names = "rmii-ref";
> 			phy-handle = <&ethphy0>;
> 			reg = <0>;
> 		};
> 
> 	};
> 
> 	mdc: rmii-ref {
> 		#clock-cells = <0>;
> 		compatible ="fixed-clock";
> 		clock-frequency = <50000000>;
> 	};
> };

To make that clear: Which phy do you have: KSZ8031RNL or KSZ8031RNLI?
The former has 25MHz default input clock whereas the latter has 50MHz
default input clock.

I assume you have the KSZ8031RNL and use it with 50MHz (so the non
default case), hence you have to set the KSZPHY_RMII_REF_CLK_SEL bit.
The "micrel,rmii-reference-clock-select-25-mhz" means "Setting the bit
selects 25MHz" which is not the case here, so you have to remove the
property.
Could it be that your initial setting just did not work because of other
mistakes, like no correct clock?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: Micrel Phy KSZ8031 clock select setting in dts

[Oliver Graute]

On 21/06/16, Sascha Hauer wrote:
> On Mon, Jun 20, 2016 at 07:14:06PM +0200, Oliver Graute wrote:
> > On 20/06/16, Andrew Lunn wrote:
> > > > &fec1 {
> > > > 	pinctrl-names = "default";
> > > > 	pinctrl-0 = <&pinctrl_enet1>;
> > > > 	phy-mode = "rmii";
> > > > 	micrel,rmii-reference-clock-select-25-mhz;
> > > > 	clocks,rmii-ref;
> > > 
> > > You are adding phy properties, not MAC properties. Please put them in
> > > the phy node.
> > 
> > yes, you are right. I fixed this and added the clock like sascha and
> > sergei proposed. (thx to you all)
> > 
> > my dts node now looks like this:
> > 
> > &fec1 {
> > 	pinctrl-names = "default";
> > 	pinctrl-0 = <&pinctrl_enet1>;
> > 	phy-mode = "rmii";
> > 	status = "okay";
> > 
> > 	mdio {
> > 		#address-cells = <1>;
> > 		#size-cells = <0>;
> > 
> > 		ethphy0: ethernet-phy@0 {
> > 			compatible = "micrel,ksz8031";
> > 			micrel,rmii-reference-clock-select-25-mhz;
> > 			clocks = <&mdc>;
> > 			clock-names = "rmii-ref";
> > 			phy-handle = <&ethphy0>;
> > 			reg = <0>;
> > 		};
> > 
> > 	};
> > 
> > 	mdc: rmii-ref {
> > 		#clock-cells = <0>;
> > 		compatible ="fixed-clock";
> > 		clock-frequency = <50000000>;
> > 	};
> > };
> 
> To make that clear: Which phy do you have: KSZ8031RNL or KSZ8031RNLI?
> The former has 25MHz default input clock whereas the latter has 50MHz
> default input clock.

according the circuit diagram I have KSZ803RNLI.

> 
> I assume you have the KSZ8031RNL and use it with 50MHz (so the non
> default case), hence you have to set the KSZPHY_RMII_REF_CLK_SEL bit.
> The "micrel,rmii-reference-clock-select-25-mhz" means "Setting the bit
> selects 25MHz" which is not the case here, so you have to remove the
> property.

ok if I removing the property "micrel,rmii-reference-clock-select-25-mhz"
and removing my changes in micrel.c it now works as well.


> Could it be that your initial setting just did not work because of other
> mistakes, like no correct clock?

yes that was the case. Many thanks for the helpful notes.

Best Regards,

Oliver

Re: Micrel Phy KSZ8031 clock select setting in dts

[Sergei Shtylyov]

Hello.

On 6/20/2016 3:08 PM, Oliver Graute wrote:

> thx you both for confirming that. Can you tell me also if the clock
> statement s fine in my dts?
>
> &fec1 {
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&pinctrl_enet1>;
> 	phy-mode = "rmii";
> 	micrel,rmii-reference-clock-select-25-mhz;
> 	clocks,rmii-ref;

    You need:

	clocks = <&your_clock>;
	clock-names = "rmii-ref";

in the PHY node (not the MAC node as you did).

[...]

> Best regards,
>
> Oliver

MBR, Sergei

Re: Micrel Phy KSZ8031 clock select setting in dts

[Sascha Hauer]

On Mon, Jun 20, 2016 at 03:56:10PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 6/20/2016 3:08 PM, Oliver Graute wrote:
> 
> >thx you both for confirming that. Can you tell me also if the clock
> >statement s fine in my dts?
> >
> >&fec1 {
> >	pinctrl-names = "default";
> >	pinctrl-0 = <&pinctrl_enet1>;
> >	phy-mode = "rmii";
> >	micrel,rmii-reference-clock-select-25-mhz;
> >	clocks,rmii-ref;
> 
>    You need:
> 
> 	clocks = <&your_clock>;
> 	clock-names = "rmii-ref";

With a clock node specified like:

	your_clock: rmii-ref {
		#clock-cells = <0>;
		compatible = "fixed-clock";
		clock-frequency = <50000000>;
	};

(assuming this is a fixed clock which can't be retrieved from elsewhere
in the clock tree)

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |