[U-Boot] [PATCH] phy/marvell: Rewrite the MV88E1111 phy config function based on kernel code

[Roy Zang]

The original m88es_config() does not do the SGMII mode
initialization and is buggy. Rewrite the function according to
3.0.6 kernel function m88e_config_init() in drivers/net/phy/marvell.c

Signed-off-by: Roy Zang 
Acked-by: Andy Fleming 
Cc: Kumar Gala 
---
Tested on P1023RDS board
 drivers/net/phy/marvell.c |  100 ++---
 1 files changed, 94 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index bd1cdc4..635a114 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -43,6 +43,24 @@
 #define MIIM_88E_PHY_LED_DIRECT0x4100
 #define MIIM_88E_PHY_LED_COMBINE   0x411C
 
+/* 88E Extended PHY Specific Control Register */
+#define MIIM_88E_PHY_EXT_CR0x14
+#define MIIM_88E_RX_DELAY  0x80
+#define MIIM_88E_TX_DELAY  0x2
+
+/* 88E Extended PHY Specific Status Register */
+#define MIIM_88E_PHY_EXT_SR0x1b
+#define MIIM_88E_HWCFG_MODE_MASK   0xf
+#define MIIM_88E_HWCFG_MODE_COPPER_RGMII   0xb
+#define MIIM_88E_HWCFG_MODE_FIBER_RGMII0x3
+#define MIIM_88E_HWCFG_MODE_SGMII_NO_CLK   0x4
+#define MIIM_88E_HWCFG_MODE_COPPER_RTBI0x9
+#define MIIM_88E_HWCFG_FIBER_COPPER_AUTO   0x8000
+#define MIIM_88E_HWCFG_FIBER_COPPER_RES0x2000
+
+#define MIIM_88E_COPPER0
+#define MIIM_88E_FIBER 1
+
 /* 88E1118 PHY defines */
 #define MIIM_88E1118_PHY_PAGE  22
 #define MIIM_88E1118_PHY_LED_PAGE  3
@@ -167,14 +185,84 @@ static int m88es_config(struct phy_device *phydev)
(phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
(phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
-   reg = phy_read(phydev, MDIO_DEVAD_NONE, 0x1b);
-   reg = (reg & 0xfff0) | 0xb;
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1b, reg);
-   } else {
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1b, 0x1f);
+   reg = phy_read(phydev,
+   MDIO_DEVAD_NONE, MIIM_88E_PHY_EXT_CR);
+   if ((phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
+   (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)) {
+   reg |= (MIIM_88E_RX_DELAY | MIIM_88E_TX_DELAY);
+   } else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
+   reg &= ~MIIM_88E_TX_DELAY;
+   reg |= MIIM_88E_RX_DELAY;
+   } else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+   reg &= ~MIIM_88E_RX_DELAY;
+   reg |= MIIM_88E_TX_DELAY;
+   }
+
+   phy_write(phydev,
+   MDIO_DEVAD_NONE, MIIM_88E_PHY_EXT_CR, reg);
+
+   reg = phy_read(phydev,
+   MDIO_DEVAD_NONE, MIIM_88E_PHY_EXT_SR);
+
+   reg &= ~(MIIM_88E_HWCFG_MODE_MASK);
+
+   if (reg & MIIM_88E_HWCFG_FIBER_COPPER_RES)
+   reg |= MIIM_88E_HWCFG_MODE_FIBER_RGMII;
+   else
+   reg |= MIIM_88E_HWCFG_MODE_COPPER_RGMII;
+
+   phy_write(phydev,
+   MDIO_DEVAD_NONE, MIIM_88E_PHY_EXT_SR, reg);
+   }
+
+   if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
+   reg = phy_read(phydev,
+   MDIO_DEVAD_NONE, MIIM_88E_PHY_EXT_SR);
+
+   reg &= ~(MIIM_88E_HWCFG_MODE_MASK);
+   reg |= MIIM_88E_HWCFG_MODE_SGMII_NO_CLK;
+   reg |= MIIM_88E_HWCFG_FIBER_COPPER_AUTO;
+
+   phy_write(phydev, MDIO_DEVAD_NONE,
+   MIIM_88E_PHY_EXT_SR, reg);
+   }
+
+   if (phydev->interface == PHY_INTERFACE_MODE_RTBI) {
+   reg = phy_read(phydev,
+   MDIO_DEVAD_NONE, MIIM_88E_PHY_EXT_CR);
+   reg |= (MIIM_88E_RX_DELAY | MIIM_88E_TX_DELAY);
+   phy_write(phydev,
+   MDIO_DEVAD_NONE, MIIM_88E_PHY_EXT_CR, reg);
+
+   reg = phy_read(phydev, MDIO_DEVAD_NONE,
+   MIIM_88E_PHY_EXT_SR);
+   reg &= ~(MIIM_88E_HWCFG_MODE_MASK |
+   MIIM_88E_HWCFG_FIBER_COPPER_RES);
+   reg |= 0x7 | MIIM_88E_HWCFG_FIBER_COPPER_AUTO;
+   phy_write(phydev, MDIO_DEVAD_NONE,
+   MIIM_88E_PHY_EXT_SR, reg);
+
+   /* soft reset */
+   phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
+   do
+   reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR);
+   while (reg & BMCR_RESET);
+
+   reg = phy_read(phydev, MDIO_DEVAD_NON

Re: [U-Boot] [PATCH] phy/marvell: Rewrite the MV88E1111 phy config function based on kernel code

[Wolfgang Denk]

Dear Roy Zang,

In message <1319178713-12472-2-git-send-email-tie-fei.z...@freescale.com> you 
wrote:
> The original m88es_config() does not do the SGMII mode
> initialization and is buggy. Rewrite the function according to
> 3.0.6 kernel function m88e_config_init() in drivers/net/phy/marvell.c
> 
> Signed-off-by: Roy Zang 
> Acked-by: Andy Fleming 
> Cc: Kumar Gala 
...
> + /* soft reset */
> + phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
> + do
> + reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR);
> + while (reg & BMCR_RESET);
...
> + /* soft reset */
> + phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
> + do
> + reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR);
> + while (reg & BMCR_RESET);

Do we really need this double reset?

Also, I dislike the potentially infinite loop here - please add a
timeout and an error exit.

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: w...@denx.de
A supercomputer is a machine that runs an endless loop in 2 seconds.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] phy/marvell: Rewrite the MV88E1111 phy config function based on kernel code

[Zang Roy-R61911]

> -Original Message-
> From: Wolfgang Denk [mailto:w...@denx.de]
> Sent: Monday, October 24, 2011 3:42 AM
> To: Zang Roy-R61911
> Cc: u-boot@lists.denx.de; Kumar Gala
> Subject: Re: [U-Boot] [PATCH] phy/marvell: Rewrite the MV88E phy config
> function based on kernel code
> 
> Dear Roy Zang,
> 
> In message <1319178713-12472-2-git-send-email-tie-fei.z...@freescale.com> you
> wrote:
> > The original m88es_config() does not do the SGMII mode
> > initialization and is buggy. Rewrite the function according to
> > 3.0.6 kernel function m88e_config_init() in drivers/net/phy/marvell.c
> >
> > Signed-off-by: Roy Zang 
> > Acked-by: Andy Fleming 
> > Cc: Kumar Gala 
> ...
> > +   /* soft reset */
> > +   phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
> > +   do
> > +   reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR);
> > +   while (reg & BMCR_RESET);
> ...
> > +   /* soft reset */
> > +   phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR, BMCR_RESET);
> > +   do
> > +   reg = phy_read(phydev, MDIO_DEVAD_NONE, MII_BMCR);
> > +   while (reg & BMCR_RESET);
> 
> Do we really need this double reset?
The MV88E user manual requests "any changes to HWCFG_MODE of Extended PHY 
Specific Status Register must be followed by software reset to take effect"
>From the code flow, double reset is only for RTBI mode, which really doubly 
>changes the HWCFG_MODE bits.

> 
> Also, I dislike the potentially infinite loop here - please add a
> timeout and an error exit.
This makes sense. Will update and resend.
Thanks.
Roy


> 
> 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: w...@denx.de
> A supercomputer is a machine that runs an endless loop in 2 seconds.


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