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

From: Hao Zhang hzhang@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 hzhang@ti.com Signed-off-by: Ivan Khoronzhuk ivan.khoronzhuk@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 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, };

On 29.10.2014 19:38, Ivan Khoronzhuk wrote:
From: Hao Zhang hzhang@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 hzhang@ti.com Signed-off-by: Ivan Khoronzhuk ivan.khoronzhuk@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 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);
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

On 10/30/2014 08:53 AM, Stefan Roese wrote:
On 29.10.2014 19:38, Ivan Khoronzhuk wrote:
From: Hao Zhang hzhang@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 hzhang@ti.com Signed-off-by: Ivan Khoronzhuk ivan.khoronzhuk@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 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);
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!

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, 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
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

On Wed, Oct 29, 2014 at 08:38:23PM +0200, Khoronzhuk, Ivan wrote:
From: Hao Zhang hzhang@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 hzhang@ti.com Signed-off-by: Ivan Khoronzhuk ivan.khoronzhuk@ti.com
Applied to u-boot-ti/master, thanks!

On Wed, Nov 05, 2014 at 04:30:33PM -0500, Tom Rini wrote:
On Wed, Oct 29, 2014 at 08:38:23PM +0200, Khoronzhuk, Ivan wrote:
From: Hao Zhang hzhang@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 hzhang@ti.com Signed-off-by: Ivan Khoronzhuk ivan.khoronzhuk@ti.com
Applied to u-boot-ti/master, thanks!
... Ivan pointed out there's a v2, dropping this one and getting the right one.
participants (3)
-
Ivan Khoronzhuk
-
Stefan Roese
-
Tom Rini