Re: [U-Boot] [U-boot] [Patch] net: phy: marvell: add errata w/a for 88E151* chips

2014-10-30 Thread Stefan Roese

On 29.10.2014 19:38, Ivan Khoronzhuk wrote:

From: Hao Zhang hzh...@ti.com

As per Marvell Release Notes - Alaska 88E1510/88E1518/88E1512/88E1514
Rev A0, Errata Section 3.1 Marvell PHY has an errata which requires
that certain registers get written in order to restart
autonegotiation.

Signed-off-by: Hao Zhang hzh...@ti.com
Signed-off-by: Ivan Khoronzhuk ivan.khoronz...@ti.com
---
  drivers/net/phy/marvell.c | 51 ++-
  1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index d2ecadc..425db94 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -276,6 +276,55 @@ static int m88es_config(struct phy_device *phydev)
return 0;
  }

+/**
+ * m88e1518_phy_writebits - write bits to a register
+ */
+void m88e1518_phy_writebits(struct phy_device *phydev,
+  u8 reg_num, u16 offset, u16 len, u16 data)
+{
+   u16 reg, mask;
+
+   if ((len + offset) = 16)
+   mask = 0 - (1  offset);
+   else
+   mask = (1  (len + offset)) - (1  offset);
+
+   reg = phy_read(phydev, MDIO_DEVAD_NONE, reg_num);
+
+   reg = ~mask;
+   reg |= data  offset;
+
+   phy_write(phydev, MDIO_DEVAD_NONE, reg_num, reg);
+}
+
+static int m88e1518_config(struct phy_device *phydev)
+{
+   /*
+* As per Marvell Release Notes - Alaska 88E1510/88E1518/88E1512/88E1514
+* Rev A0, Errata Section 3.1
+*/
+   phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00ff); /* reg page 0xff */
+   phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x214B);
+   phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2144);
+   phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x0C28);
+   phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2146);
+   phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xB233);
+   phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x214D);
+   phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xCC0C);
+   phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
+   phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x); /* reg page 0 */
+   phy_write(phydev, MDIO_DEVAD_NONE, 22, 18); /* reg page 18 */
+   /* Write HWCFG_MODE = SGMII to Copper */
+   m88e1518_phy_writebits(phydev, 20, 0, 3, 1);


Won't this set the mode to SGMII for all users of this code? I know of 
at least one board that uses this driver and uses RGMII. So you 
shouldn't set this mode here to SGMII unconditionally.


Thanks,
Stefan

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [U-boot] [Patch] net: phy: marvell: add errata w/a for 88E151* chips

2014-10-30 Thread Ivan Khoronzhuk

On 10/30/2014 08:53 AM, Stefan Roese wrote:

On 29.10.2014 19:38, Ivan Khoronzhuk wrote:

From: Hao Zhang hzh...@ti.com

As per Marvell Release Notes - Alaska 88E1510/88E1518/88E1512/88E1514
Rev A0, Errata Section 3.1 Marvell PHY has an errata which requires
that certain registers get written in order to restart
autonegotiation.

Signed-off-by: Hao Zhang hzh...@ti.com
Signed-off-by: Ivan Khoronzhuk ivan.khoronz...@ti.com
---
  drivers/net/phy/marvell.c | 51 
++-

  1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index d2ecadc..425db94 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -276,6 +276,55 @@ static int m88es_config(struct phy_device 
*phydev)

  return 0;
  }

+/**
+ * m88e1518_phy_writebits - write bits to a register
+ */
+void m88e1518_phy_writebits(struct phy_device *phydev,
+   u8 reg_num, u16 offset, u16 len, u16 data)
+{
+u16 reg, mask;
+
+if ((len + offset) = 16)
+mask = 0 - (1  offset);
+else
+mask = (1  (len + offset)) - (1  offset);
+
+reg = phy_read(phydev, MDIO_DEVAD_NONE, reg_num);
+
+reg = ~mask;
+reg |= data  offset;
+
+phy_write(phydev, MDIO_DEVAD_NONE, reg_num, reg);
+}
+
+static int m88e1518_config(struct phy_device *phydev)
+{
+/*
+ * As per Marvell Release Notes - Alaska 
88E1510/88E1518/88E1512/88E1514

+ * Rev A0, Errata Section 3.1
+ */
+phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00ff);/* reg page 
0xff */

+phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x214B);
+phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2144);
+phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x0C28);
+phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2146);
+phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xB233);
+phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x214D);
+phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xCC0C);
+phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
+phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x);/* reg page 0 */
+phy_write(phydev, MDIO_DEVAD_NONE, 22, 18);/* reg page 18 */
+/* Write HWCFG_MODE = SGMII to Copper */
+m88e1518_phy_writebits(phydev, 20, 0, 3, 1);


Won't this set the mode to SGMII for all users of this code? I know of 
at least one board that uses this driver and uses RGMII. So you 
shouldn't set this mode here to SGMII unconditionally.


Thanks,
Stefan



Yes.
I will put whole errata w/o under:
if (phydev-interface == PHY_INTERFACE_MODE_SGMII) {

}

as I can face it only for SGMII

Thanks!

--
Regards,
Ivan Khoronzhuk

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [U-boot] [Patch] net: phy: marvell: add errata w/a for 88E151* chips

2014-10-30 Thread Stefan Roese

On 30.10.2014 10:52, Ivan Khoronzhuk wrote:

+static int m88e1518_config(struct phy_device *phydev)
+{
+/*
+ * As per Marvell Release Notes - Alaska
88E1510/88E1518/88E1512/88E1514
+ * Rev A0, Errata Section 3.1
+ */
+phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00ff);/* reg page
0xff */
+phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x214B);
+phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2144);
+phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x0C28);
+phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2146);
+phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xB233);
+phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x214D);
+phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xCC0C);
+phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
+phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x);/* reg page 0 */
+phy_write(phydev, MDIO_DEVAD_NONE, 22, 18);/* reg page 18 */
+/* Write HWCFG_MODE = SGMII to Copper */
+m88e1518_phy_writebits(phydev, 20, 0, 3, 1);


Won't this set the mode to SGMII for all users of this code? I know of
at least one board that uses this driver and uses RGMII. So you
shouldn't set this mode here to SGMII unconditionally.

Thanks,
Stefan



Yes.
I will put whole errata w/o under:
if (phydev-interface == PHY_INTERFACE_MODE_SGMII) {

}

as I can face it only for SGMII


Yes. If this errata is really only for SGMII, then putting this code 
under this if() statement makes sense.


Thanks,
Stefan

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [U-boot] [Patch] net: phy: marvell: add errata w/a for 88E151* chips

2014-10-29 Thread Ivan Khoronzhuk
From: Hao Zhang hzh...@ti.com

As per Marvell Release Notes - Alaska 88E1510/88E1518/88E1512/88E1514
Rev A0, Errata Section 3.1 Marvell PHY has an errata which requires
that certain registers get written in order to restart
autonegotiation.

Signed-off-by: Hao Zhang hzh...@ti.com
Signed-off-by: Ivan Khoronzhuk ivan.khoronz...@ti.com
---
 drivers/net/phy/marvell.c | 51 ++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index d2ecadc..425db94 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -276,6 +276,55 @@ static int m88es_config(struct phy_device *phydev)
return 0;
 }
 
+/**
+ * m88e1518_phy_writebits - write bits to a register
+ */
+void m88e1518_phy_writebits(struct phy_device *phydev,
+  u8 reg_num, u16 offset, u16 len, u16 data)
+{
+   u16 reg, mask;
+
+   if ((len + offset) = 16)
+   mask = 0 - (1  offset);
+   else
+   mask = (1  (len + offset)) - (1  offset);
+
+   reg = phy_read(phydev, MDIO_DEVAD_NONE, reg_num);
+
+   reg = ~mask;
+   reg |= data  offset;
+
+   phy_write(phydev, MDIO_DEVAD_NONE, reg_num, reg);
+}
+
+static int m88e1518_config(struct phy_device *phydev)
+{
+   /*
+* As per Marvell Release Notes - Alaska 88E1510/88E1518/88E1512/88E1514
+* Rev A0, Errata Section 3.1
+*/
+   phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00ff); /* reg page 0xff */
+   phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x214B);
+   phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2144);
+   phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x0C28);
+   phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2146);
+   phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xB233);
+   phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x214D);
+   phy_write(phydev, MDIO_DEVAD_NONE, 17, 0xCC0C);
+   phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2159);
+   phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x); /* reg page 0 */
+   phy_write(phydev, MDIO_DEVAD_NONE, 22, 18); /* reg page 18 */
+   /* Write HWCFG_MODE = SGMII to Copper */
+   m88e1518_phy_writebits(phydev, 20, 0, 3, 1);
+
+   /* Phy reset */
+   m88e1518_phy_writebits(phydev, 20, 15, 1, 1);
+   phy_write(phydev, MDIO_DEVAD_NONE, 22, 0);  /* reg page 18 */
+   udelay(100);
+
+   return m88es_config(phydev);
+}
+
 /* Marvell 88E1118 */
 static int m88e1118_config(struct phy_device *phydev)
 {
@@ -493,7 +542,7 @@ static struct phy_driver M88E1518_driver = {
.uid = 0x1410dd1,
.mask = 0xff0,
.features = PHY_GBIT_FEATURES,
-   .config = m88es_config,
+   .config = m88e1518_config,
.startup = m88e1011s_startup,
.shutdown = genphy_shutdown,
 };
-- 
1.8.3.2

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot