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

2011-10-23 Thread 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 tie-fei.z...@freescale.com
 Acked-by: Andy Fleming aflem...@freescale.com
 Cc: Kumar Gala ga...@kernel.crashing.org
...
 + /* 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

2011-10-23 Thread 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 tie-fei.z...@freescale.com
  Acked-by: Andy Fleming aflem...@freescale.com
  Cc: Kumar Gala ga...@kernel.crashing.org
 ...
  +   /* 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

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

2011-10-21 Thread 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 tie-fei.z...@freescale.com
Acked-by: Andy Fleming aflem...@freescale.com
Cc: Kumar Gala ga...@kernel.crashing.org
---
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