Skip to contentSkip to conversation

net: phy: marvell: add errata w/a for 88E151* chips

PATCH In review Ivan Khoronzhuk 3 participants 6 messages Oct 29, 2014 – Nov 6, 2014

Subsystems (2)
Network drivers Ethernet PHY
lore
Share & tools
lore

Ivan Khoronzhuk

author
#0 lore raw

PATCH 1/1 net: phy: marvell: add errata w/a for 88E151* chips

From: Hao Zhang <[email protected]>

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 <[email protected]>
Signed-off-by Ivan Khoronzhuk <[email protected]>
---
 drivers/net/phy/marvell.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)
drivers/net/phy/marvell.c +50 -1
index d2ecadc..425db94 100644
@@ -276,6 +276,55 @@ static int m88e1111s_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, 0x0000);	/* 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 m88e1111s_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 = 0xffffff0,
 	.features = PHY_GBIT_FEATURES,
-	.config = &m88e1111s_config,
+	.config = &m88e1518_config,
 	.startup = &m88e1011s_startup,
 	.shutdown = &genphy_shutdown,
 };
Signature · 1 line
1.8.3.2
#1 lore raw

PATCH 1/1 net: phy: marvell: add errata w/a for 88E151* chips

Quoted 51 lines · Ivan Khoronzhuk

From: Hao Zhang <[email protected]>

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 <[email protected]>
Signed-off-by Ivan Khoronzhuk <[email protected]>
---
  drivers/net/phy/marvell.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 50 insertions(+), 1 deletion(-)
drivers/net/phy/marvell.c +30 -0
index d2ecadc..425db94 100644
@@ -276,6 +276,55 @@ static int m88e1111s_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);

drivers/net/phy/marvell.c static int m88e1111s_config(struct phy_device *phydev) · line 318

Quoted 10 lines
drivers/net/phy/marvell.c +10 -0
+	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, 0x0000);	/* 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

author
#2 lore raw

PATCH 1/1 net: phy: marvell: add errata w/a for 88E151* chips

Quoted 71 lines · Stefan Roese
Quoted 55 lines · Ivan Khoronzhuk

From: Hao Zhang <[email protected]>

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 <[email protected]>
Signed-off-by Ivan Khoronzhuk <[email protected]>
---
  drivers/net/phy/marvell.c | 51 
++++++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 50 insertions(+), 1 deletion(-)
drivers/net/phy/marvell.c +25 -0
index d2ecadc..425db94 100644
@@ -276,6 +276,55 @@ static int m88e1111s_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

drivers/net/phy/marvell.c +3 -0
+     * Rev A0, Errata Section 3.1
+     */
+    phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00ff);    /* reg page

0xff */

drivers/net/phy/marvell.c +2 -0
+    phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x214B);
+    phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2144);

drivers/net/phy/marvell.c static int m88e1111s_config(struct phy_device *phydev) · line 318

Quoted 10 lines
drivers/net/phy/marvell.c +10 -0
+    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, 0x0000);    /* 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.

Quoted 3 lines

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!

Signature · 2 lines
Regards,
Ivan Khoronzhuk
#3 lore raw

PATCH 1/1 net: phy: marvell: add errata w/a for 88E151* chips

Quoted 30 lines · Ivan Khoronzhuk
Quoted 26 lines
Quoted 11 lines
drivers/net/phy/marvell.c +4 -0
+static int m88e1518_config(struct phy_device *phydev)
+{
+    /*
+     * As per Marvell Release Notes - Alaska

88E1510/88E1518/88E1512/88E1514

drivers/net/phy/marvell.c +3 -0
+     * Rev A0, Errata Section 3.1
+     */
+    phy_write(phydev, MDIO_DEVAD_NONE, 22, 0x00ff);    /* reg page

0xff */

drivers/net/phy/marvell.c +2 -0
+    phy_write(phydev, MDIO_DEVAD_NONE, 17, 0x214B);
+    phy_write(phydev, MDIO_DEVAD_NONE, 16, 0x2144);

drivers/net/phy/marvell.c static int m88e1111s_config(struct phy_device *phydev) · line 318

Quoted 10 lines
drivers/net/phy/marvell.c +10 -0
+    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, 0x0000);    /* 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.

Quoted 3 lines

Thanks,
Stefan

Quoted 7 lines

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

#4 lore raw
In reply to Ivan Khoronzhuk
Quoted 9 lines · Ivan

From: Hao Zhang <[email protected]>

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 <[email protected]>
Signed-off-by Ivan Khoronzhuk <[email protected]>

Applied to u-boot-ti/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141105/4efde4ca/attachment.pgp>
#5 lore raw
Quoted 9 lines · Tom Rini
Quoted 7 lines · Ivan

From: Hao Zhang <[email protected]>

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.

Quoted 4 lines
Quoted 2 lines
Signed-off-by Hao Zhang <[email protected]>
Signed-off-by Ivan Khoronzhuk <[email protected]>

Applied to u-boot-ti/master, thanks!

... Ivan pointed out there's a v2, dropping this one and getting the right one.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141106/523ddf7e/attachment.pgp>