[U-Boot] arm: socfpga: set skew settings for ethernet phy

[U-Boot] arm: socfpga: set skew settings for ethernet phy

[dinguyen at opensource.altera.com]

From: Dinh Nguyen <dinguyen@opensource.altera.com>

Set the PHY skew settings for the ethernet phy on the SOCFPGA devkit.

Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
Cc: Vince Bridgers <vbridger@opensource.altera.com>
Cc: Pavel Machek <pavel@denx.de>
Cc: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@ti.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Wolfgang Denk <wd@denx.de>
---
 board/altera/socfpga/socfpga_cyclone5.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/board/altera/socfpga/socfpga_cyclone5.c b/board/altera/socfpga/socfpga_cyclone5.c
index 0f81d89..a2831c8 100644
--- a/board/altera/socfpga/socfpga_cyclone5.c
+++ b/board/altera/socfpga/socfpga_cyclone5.c
@@ -8,7 +8,9 @@
 #include <asm/arch/reset_manager.h>
 #include <asm/io.h>
 
+#include <micrel.h>
 #include <netdev.h>
+#include <phy.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -39,3 +41,13 @@ int board_init(void)
 
 	return 0;
 }
+
+int board_phy_config(struct phy_device *phydev)
+{
+	ksz9021_phy_extended_write(phydev, MII_KSZ9021_EXT_RGMII_RX_DATA_SKEW,
+				   0x0);
+	ksz9021_phy_extended_write(phydev, MII_KSZ9021_EXT_RGMII_TX_DATA_SKEW,
+				   0x0);
+	ksz9021_phy_extended_write(phydev, MII_KSZ9021_EXT_RGMII_CLOCK_SKEW,
+				   0xf0f0);
+}
-- 
2.0.3

[U-Boot] arm: socfpga: set skew settings for ethernet phy

[Pavel Machek]

Hi!

> From: Dinh Nguyen <dinguyen@opensource.altera.com>
> 
> Set the PHY skew settings for the ethernet phy on the SOCFPGA
> devkit.

Should the timing configuration be done in the config file...?

Marek disliked getting the values from the environment, and I'm not
sure if that is neccessary. It can be removed if needed.

Signed-off-by: Pavel Machek <pavel@denx.de> 

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 507b9a3..06a31b0 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -112,17 +112,46 @@ static int ksz9021_phy_extwrite(struct phy_device *phydev, int addr,
 	return ksz9021_phy_extended_write(phydev, regnum, val);
 }
 
+
+
 /* Micrel ksz9021 */
 static int ksz9021_config(struct phy_device *phydev)
 {
 	unsigned ctrl1000 = 0;
 	const unsigned master = CTRL1000_PREFER_MASTER |
-			CTRL1000_CONFIG_MASTER | CTRL1000_MANUAL_CONFIG;
+		CTRL1000_CONFIG_MASTER | CTRL1000_MANUAL_CONFIG;
 	unsigned features = phydev->drv->features;
 
+	printf("ksz9021: configuring\n");
+
+	printf("Configuring PHY skew timing for %s\n",
+	       phydev->drv->name);
+
+	/* min rx data delay */
+	if (ksz9021_phy_extended_write(phydev,
+				       MII_KSZ9021_EXT_RGMII_RX_DATA_SKEW,
+				       getenv_ulong(CONFIG_KSZ9021_DATA_SKEW_ENV, 16,
+						    CONFIG_KSZ9021_DATA_SKEW_VAL)) < 0)
+		return -1;
+	/* min tx data delay */
+	if (ksz9021_phy_extended_write(phydev,
+				       MII_KSZ9021_EXT_RGMII_TX_DATA_SKEW,
+				       getenv_ulong(CONFIG_KSZ9021_DATA_SKEW_ENV, 16,
+						    CONFIG_KSZ9021_DATA_SKEW_VAL)) < 0)
+		return -1;
+	/* max rx/tx clock delay, min rx/tx control */
+	if (ksz9021_phy_extended_write(phydev,
+				       MII_KSZ9021_EXT_RGMII_CLOCK_SKEW,
+				       getenv_ulong(CONFIG_KSZ9021_CLK_SKEW_ENV, 16,
+						    CONFIG_KSZ9021_CLK_SKEW_VAL)) < 0)
+		return -1;
+
+
+
+
 	if (getenv("disable_giga"))
 		features &= ~(SUPPORTED_1000baseT_Half |
-				SUPPORTED_1000baseT_Full);
+			      SUPPORTED_1000baseT_Full);
 	/* force master mode for 1000BaseT due to chip errata */
 	if (features & SUPPORTED_1000baseT_Half)
 		ctrl1000 |= ADVERTISE_1000HALF | master;
diff --git a/include/configs/socfpga_cyclone5.h b/include/configs/socfpga_cyclone5.h
index 8d54bf89..53a3d71 100644
--- a/include/configs/socfpga_cyclone5.h
+++ b/include/configs/socfpga_cyclone5.h
@@ -53,19 +56,29 @@
 
 /* Ethernet on SoC (EMAC) */
 #if defined(CONFIG_CMD_NET)
-#define CONFIG_EMAC_BASE		SOCFPGA_EMAC0_ADDRESS
+#define CONFIG_EMAC_BASE		SOCFPGA_EMAC1_ADDRESS
 #define CONFIG_PHY_INTERFACE_MODE	PHY_INTERFACE_MODE_RGMII
-#define CONFIG_EPHY0_PHY_ADDR		0
-
-/* PHY */
 #define CONFIG_EPHY1_PHY_ADDR		4
+
 #define CONFIG_PHY_MICREL
 #define CONFIG_PHY_MICREL_KSZ9021
-#define CONFIG_KSZ9021_CLK_SKEW_ENV	"micrel-ksz9021-clk-skew"
-#define CONFIG_KSZ9021_CLK_SKEW_VAL	0xf0f0
-#define CONFIG_KSZ9021_DATA_SKEW_ENV	"micrel-ksz9021-data-skew"
-#define CONFIG_KSZ9021_DATA_SKEW_VAL	0x0
 
+/* phy */
+#define CONFIG_EPHY0_PHY_ADDR          0
+#define CONFIG_EPHY1_PHY_ADDR          4
+#define CONFIG_KSZ9021_CLK_SKEW_ENV    "micrel-ksz9021-clk-skew"
+#define CONFIG_KSZ9021_CLK_SKEW_VAL    0xf0f0
+#define CONFIG_KSZ9021_DATA_SKEW_ENV   "micrel-ksz9021-data-skew"
+#define CONFIG_KSZ9021_DATA_SKEW_VAL   0x0
+/* Type of PHY available */
+#define SOCFPGA_PHYSEL_ENUM_GMII       0x0
+#define SOCFPGA_PHYSEL_ENUM_MII                0x1
+#define SOCFPGA_PHYSEL_ENUM_RGMII      0x2
+#define SOCFPGA_PHYSEL_ENUM_RMII       0x3
+
+
+#else
+#error Youll need ethernet :-)
 #endif
 
 /* Extra Environment */

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] arm: socfpga: set skew settings for ethernet phy

[Wolfgang Denk]

Dear Pavel Machek,

In message <20141113120956.GB30779@amd> you wrote:
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 507b9a3..06a31b0 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -112,17 +112,46 @@ static int ksz9021_phy_extwrite(struct phy_device *phydev, int addr,
>  	return ksz9021_phy_extended_write(phydev, regnum, val);
>  }
>  
> +
> +

Please don't add excessive white space / blank lines.

> +	printf("ksz9021: configuring\n");
> +
> +	printf("Configuring PHY skew timing for %s\n",
> +	       phydev->drv->name);

Should these eventually be changed into debug() ?

Output gets really noisy otherwise.

>  	if (getenv("disable_giga"))

Should we not rather use the standard "hwconf" approach here?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There is a time in the tides of men, Which, taken at its flood, leads
on to success. On the other hand, don't count on it.   - T. K. Lawson

[U-Boot] arm: socfpga: set skew settings for ethernet phy

[Pavel Machek]

Hi!

On Thu 2014-11-13 13:20:52, Wolfgang Denk wrote:
> Dear Pavel Machek,
> 
> In message <20141113120956.GB30779@amd> you wrote:
> > 
> > diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> > index 507b9a3..06a31b0 100644
> > --- a/drivers/net/phy/micrel.c
> > +++ b/drivers/net/phy/micrel.c
> > @@ -112,17 +112,46 @@ static int ksz9021_phy_extwrite(struct phy_device *phydev, int addr,
> >  	return ksz9021_phy_extended_write(phydev, regnum, val);
> >  }
> >  
> > +
> > +
> 
> Please don't add excessive white space / blank lines.
> 
> > +	printf("ksz9021: configuring\n");
> > +
> > +	printf("Configuring PHY skew timing for %s\n",
> > +	       phydev->drv->name);
> 
> Should these eventually be changed into debug() ?
> 
> Output gets really noisy otherwise.
> 
> >  	if (getenv("disable_giga"))
> 
> Should we not rather use the standard "hwconf" approach here?

Comments accepted (this was just a test patch I hand edited it, thats
why it was so ugly), but we need to decide, first, which approach to
take. (Configuration in environment vs. configuration in config file
vs. hardcoded configuration in .c file.). Then, clean patch can be
prepared.

I wanted to sign it off, so that it is clear it can be used as a basis
for better patch, but I was aware it is not clean enough for merging
-- how do I do that?

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] arm: socfpga: set skew settings for ethernet phy

[Albert ARIBAUD]

Hello Pavel,

> I wanted to sign it off, so that it is clear it can be used as a basis
> for better patch, but I was aware it is not clean enough for merging
> -- how do I do that?

Why not simply tag it [RFC]?

> Thanks,
> 									Pavel
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Amicalement,
-- 
Albert.

[U-Boot] arm: socfpga: set skew settings for ethernet phy

[Pavel Machek]

On Thu 2014-11-13 13:32:26, Albert ARIBAUD wrote:
> Hello Pavel,
> 
> > I wanted to sign it off, so that it is clear it can be used as a basis
> > for better patch, but I was aware it is not clean enough for merging
> > -- how do I do that?
> 
> Why not simply tag it [RFC]?

Well, dunno, I was not asking for code review at that point. There was
a patch submitted, and I was asking if maybe different approach is
better.

Subject already said "Re: ...", that is usually disussion, not a patch
that needs to be reviewed.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] arm: socfpga: set skew settings for ethernet phy

[Marek Vasut]

On Thursday, November 13, 2014 at 01:48:41 PM, Pavel Machek wrote:
> On Thu 2014-11-13 13:32:26, Albert ARIBAUD wrote:
> > Hello Pavel,
> > 
> > > I wanted to sign it off, so that it is clear it can be used as a basis
> > > for better patch, but I was aware it is not clean enough for merging
> > > -- how do I do that?
> > 
> > Why not simply tag it [RFC]?
> 
> Well, dunno, I was not asking for code review at that point. There was
> a patch submitted, and I was asking if maybe different approach is
> better.
> 
> Subject already said "Re: ...", that is usually disussion, not a patch
> that needs to be reviewed.

This discussion is moot, let's stick to the subject guys, thanks!

Best regards,
Marek Vasut

[U-Boot] arm: socfpga: script for installing u-boot / u-boot-SPL

[Pavel Machek]

It is very easy to overwrite partition on your harddrive when all you
want is installing new u-boot(-SPL). Plus... figuring out needed steps
for installation is not trivial, either. This automates installation,
and provides safety net by checking signature on existing partition.

[I managed to overwrite partition once, and in second incident second
mistake -- -EPERM -- saved my data] 

Signed-off-by: Pavel Machek <pavel@denx.de>


diff --git a/tools/socfpga_install b/tools/socfpga_install
new file mode 100755
index 0000000..b62d3e77
--- /dev/null
+++ b/tools/socfpga_install
@@ -0,0 +1,39 @@
+#!/usr/bin/python3
+
+import subprocess
+import sys
+import os
+
+print("install device ", sys.argv[1])
+device = open(sys.argv[1], "r+b")
+
+sig = device.read(0x44)
+if sig[0x40] != 0x41 or sig[0x41] != 0x53 or \
+   sig[0x42] != 0x30 or sig[0x43] != 0x31:
+    print("Do not see AS01 signature, do you have right partition?")
+    sys.exit(1)
+
+print("u-boot-spl.bin at ", sys.argv[2])
+spl = sys.argv[2]
+
+res = subprocess.check_call(["tools/mkimage", "-T", "socfpgaimage", "-d", spl, "img.tmp"])
+if res != 0:
+   print("mkimage failed.")
+   sys.exit(1)
+device.seek(0)
+
+spl_bin = open("img.tmp", "rb").read()
+if len(spl_bin) != 65536:
+   print("Preloader image has wrong length.")
+   sys.exit(1)
+ 
+device.write(spl_bin)
+device.write(spl_bin)
+device.write(spl_bin)
+device.write(spl_bin)
+
+print("u-boot.img at ", sys.argv[3])
+uboot_bin = open(sys.argv[3], "rb").read()
+print("Have u-boot, %d bytes" % len(uboot_bin))
+device.write(uboot_bin)
+print("All done.")
\ No newline at end of file


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[U-Boot] arm: socfpga: script for installing u-boot / u-boot-SPL

[Marek Vasut]

On Tuesday, November 25, 2014 at 12:14:38 PM, Pavel Machek wrote:
> It is very easy to overwrite partition on your harddrive when all you
> want is installing new u-boot(-SPL). Plus... figuring out needed steps
> for installation is not trivial, either. This automates installation,
> and provides safety net by checking signature on existing partition.
> 
> [I managed to overwrite partition once, and in second incident second
> mistake -- -EPERM -- saved my data]
> 
> Signed-off-by: Pavel Machek <pavel@denx.de>

+CC Stefano, he has that SWUP tool so he might chime in.

Best regards,
Marek Vasut