[U-Boot] [PATCH] powerpc/fm: remove the TBIPA setting on platform code

[U-Boot] [PATCH] powerpc/fm: remove the TBIPA setting on platform code

[Roy Zang]

TBIPA has been set in dtsec_init_phy () funciton in drivers/net/fm/eth.c

So remove the duplicate code on platform Ethernet code.

Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
Cc: Andy Fleming <afleming@freescale.com>
Cc: Kumar Gala <galak@kernel.crashing.org>
---
Tested on P5020DS
 board/freescale/corenet_ds/eth_hydra.c |    8 --------
 board/freescale/corenet_ds/eth_p4080.c |    8 --------
 board/freescale/p2041rdb/eth.c         |    8 --------
 3 files changed, 0 insertions(+), 24 deletions(-)

diff --git a/board/freescale/corenet_ds/eth_hydra.c b/board/freescale/corenet_ds/eth_hydra.c
index 91b3408..639358d 100644
--- a/board/freescale/corenet_ds/eth_hydra.c
+++ b/board/freescale/corenet_ds/eth_hydra.c
@@ -395,7 +395,6 @@ void fdt_fixup_board_enet(void *fdt)
 int board_eth_init(bd_t *bis)
 {
 #ifdef CONFIG_FMAN_ENET
-	struct dtsec *tsec = (void *)CONFIG_SYS_FSL_FM1_DTSEC1_ADDR;
 	struct fsl_pq_mdio_info dtsec_mdio_info;
 	struct tgec_mdio_info tgec_mdio_info;
 	unsigned int i, slot;
@@ -405,13 +404,6 @@ int board_eth_init(bd_t *bis)
 
 	initialize_lane_to_slot();
 
-	/*
-	 * Set TBIPA on FM1 at DTSEC1.  This is needed for configurations
-	 * where FM1 at DTSEC1 isn't used directly, since it provides
-	 * MDIO for other ports.
-	 */
-	out_be32(&tsec->tbipa, CONFIG_SYS_TBIPA_VALUE);
-
 	/* We want to use the PIXIS to configure MUX routing, not GPIOs. */
 	setbits_8(&pixis->brdcfg2, BRDCFG2_REG_GPIO_SEL);
 
diff --git a/board/freescale/corenet_ds/eth_p4080.c b/board/freescale/corenet_ds/eth_p4080.c
index d4657f7..208b97a 100644
--- a/board/freescale/corenet_ds/eth_p4080.c
+++ b/board/freescale/corenet_ds/eth_p4080.c
@@ -295,7 +295,6 @@ int board_eth_init(bd_t *bis)
 {
 #ifdef CONFIG_FMAN_ENET
 	ccsr_gpio_t *pgpio = (void *)(CONFIG_SYS_MPC85xx_GPIO_ADDR);
-	struct dtsec *tsec = (void *)CONFIG_SYS_FSL_FM1_DTSEC1_ADDR;
 	int i;
 	struct fsl_pq_mdio_info dtsec_mdio_info;
 	struct tgec_mdio_info tgec_mdio_info;
@@ -321,13 +320,6 @@ int board_eth_init(bd_t *bis)
 		SLOT5, /* 17 - Bank 3:D */
 	};
 
-	/*
-	 * Set TBIPA on FM1 at DTSEC1.  This is needed for configurations
-	 * where FM1 at DTSEC1 isn't used directly, since it provides
-	 * MDIO for other ports.
-	 */
-	out_be32(&tsec->tbipa, CONFIG_SYS_TBIPA_VALUE);
-
 	/* Initialize the mdio_mux array so we can recognize empty elements */
 	for (i = 0; i < NUM_FM_PORTS; i++)
 		mdio_mux[i] = EMI_NONE;
diff --git a/board/freescale/p2041rdb/eth.c b/board/freescale/p2041rdb/eth.c
index 0a1dfa7..4b0d577 100644
--- a/board/freescale/p2041rdb/eth.c
+++ b/board/freescale/p2041rdb/eth.c
@@ -139,7 +139,6 @@ void board_ft_fman_fixup_port(void *fdt, char *compat, phys_addr_t addr,
 int board_eth_init(bd_t *bis)
 {
 #ifdef CONFIG_FMAN_ENET
-	struct dtsec *tsec = (void *)CONFIG_SYS_FSL_FM1_DTSEC1_ADDR;
 	struct fsl_pq_mdio_info dtsec_mdio_info;
 	struct tgec_mdio_info tgec_mdio_info;
 	unsigned int i, slot;
@@ -149,13 +148,6 @@ int board_eth_init(bd_t *bis)
 
 	initialize_lane_to_slot();
 
-	/*
-	 * Set TBIPA on FM1 at DTSEC1.  This is needed for configurations
-	 * where FM1@DTSEC1 isn't used directly, since it provides
-	 * MDIO for other ports.
-	 */
-	out_be32(&tsec->tbipa, CONFIG_SYS_TBIPA_VALUE);
-
 	dtsec_mdio_info.regs =
 		(struct tsec_mii_mng *)CONFIG_SYS_FM1_DTSEC1_MDIO_ADDR;
 	dtsec_mdio_info.name = DEFAULT_FM_MDIO_NAME;
-- 
1.6.0.6

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

[Roy Zang]

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

Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
Acked-by: Andy Fleming <afleming@freescale.com>
Cc: Kumar Gala <galak@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_88E1111_PHY_LED_DIRECT	0x4100
 #define MIIM_88E1111_PHY_LED_COMBINE	0x411C
 
+/* 88E1111 Extended PHY Specific Control Register */
+#define MIIM_88E1111_PHY_EXT_CR		0x14
+#define MIIM_88E1111_RX_DELAY		0x80
+#define MIIM_88E1111_TX_DELAY		0x2
+
+/* 88E1111 Extended PHY Specific Status Register */
+#define MIIM_88E1111_PHY_EXT_SR		0x1b
+#define MIIM_88E1111_HWCFG_MODE_MASK		0xf
+#define MIIM_88E1111_HWCFG_MODE_COPPER_RGMII	0xb
+#define MIIM_88E1111_HWCFG_MODE_FIBER_RGMII	0x3
+#define MIIM_88E1111_HWCFG_MODE_SGMII_NO_CLK	0x4
+#define MIIM_88E1111_HWCFG_MODE_COPPER_RTBI	0x9
+#define MIIM_88E1111_HWCFG_FIBER_COPPER_AUTO	0x8000
+#define MIIM_88E1111_HWCFG_FIBER_COPPER_RES	0x2000
+
+#define MIIM_88E1111_COPPER		0
+#define MIIM_88E1111_FIBER		1
+
 /* 88E1118 PHY defines */
 #define MIIM_88E1118_PHY_PAGE		22
 #define MIIM_88E1118_PHY_LED_PAGE	3
@@ -167,14 +185,84 @@ static int m88e1111s_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_88E1111_PHY_EXT_CR);
+		if ((phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
+			(phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)) {
+			reg |= (MIIM_88E1111_RX_DELAY | MIIM_88E1111_TX_DELAY);
+		} else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
+			reg &= ~MIIM_88E1111_TX_DELAY;
+			reg |= MIIM_88E1111_RX_DELAY;
+		} else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+			reg &= ~MIIM_88E1111_RX_DELAY;
+			reg |= MIIM_88E1111_TX_DELAY;
+		}
+
+		phy_write(phydev,
+			MDIO_DEVAD_NONE, MIIM_88E1111_PHY_EXT_CR, reg);
+
+		reg = phy_read(phydev,
+			MDIO_DEVAD_NONE, MIIM_88E1111_PHY_EXT_SR);
+
+		reg &= ~(MIIM_88E1111_HWCFG_MODE_MASK);
+
+		if (reg & MIIM_88E1111_HWCFG_FIBER_COPPER_RES)
+			reg |= MIIM_88E1111_HWCFG_MODE_FIBER_RGMII;
+		else
+			reg |= MIIM_88E1111_HWCFG_MODE_COPPER_RGMII;
+
+		phy_write(phydev,
+			MDIO_DEVAD_NONE, MIIM_88E1111_PHY_EXT_SR, reg);
+	}
+
+	if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
+		reg = phy_read(phydev,
+			MDIO_DEVAD_NONE, MIIM_88E1111_PHY_EXT_SR);
+
+		reg &= ~(MIIM_88E1111_HWCFG_MODE_MASK);
+		reg |= MIIM_88E1111_HWCFG_MODE_SGMII_NO_CLK;
+		reg |= MIIM_88E1111_HWCFG_FIBER_COPPER_AUTO;
+
+		phy_write(phydev, MDIO_DEVAD_NONE,
+			MIIM_88E1111_PHY_EXT_SR, reg);
+	}
+
+	if (phydev->interface == PHY_INTERFACE_MODE_RTBI) {
+		reg = phy_read(phydev,
+			MDIO_DEVAD_NONE, MIIM_88E1111_PHY_EXT_CR);
+		reg |= (MIIM_88E1111_RX_DELAY | MIIM_88E1111_TX_DELAY);
+		phy_write(phydev,
+			MDIO_DEVAD_NONE, MIIM_88E1111_PHY_EXT_CR, reg);
+
+		reg = phy_read(phydev, MDIO_DEVAD_NONE,
+			MIIM_88E1111_PHY_EXT_SR);
+		reg &= ~(MIIM_88E1111_HWCFG_MODE_MASK |
+			MIIM_88E1111_HWCFG_FIBER_COPPER_RES);
+		reg |= 0x7 | MIIM_88E1111_HWCFG_FIBER_COPPER_AUTO;
+		phy_write(phydev, MDIO_DEVAD_NONE,
+			MIIM_88E1111_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_NONE,
+			MIIM_88E1111_PHY_EXT_SR);
+		reg &= ~(MIIM_88E1111_HWCFG_MODE_MASK |
+			MIIM_88E1111_HWCFG_FIBER_COPPER_RES);
+		reg |= MIIM_88E1111_HWCFG_MODE_COPPER_RTBI |
+			MIIM_88E1111_HWCFG_FIBER_COPPER_AUTO;
+		phy_write(phydev, MDIO_DEVAD_NONE,
+			MIIM_88E1111_PHY_EXT_SR, reg);
 	}
 
-	phy_write(phydev, MDIO_DEVAD_NONE, 0x14, 0x0cd2);
+	/* 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);
 
 	genphy_config_aneg(phydev);
 
-- 
1.6.0.6

[U-Boot] [PATCH] powerpc/fm: remove the TBIPA setting on platform code

[Wolfgang Denk]

Dear Roy Zang,

In message <1319178713-12472-1-git-send-email-tie-fei.zang@freescale.com> you wrote:
> TBIPA has been set in dtsec_init_phy () funciton in drivers/net/fm/eth.c
> 
> So remove the duplicate code on platform Ethernet code.
> 
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> Cc: Andy Fleming <afleming@freescale.com>
> Cc: Kumar Gala <galak@kernel.crashing.org>

Please change the Subject: so everybody understands what you are
doing. "powerpc/fm" is not exactly clear to everybody, and neither is
TBIPA.

Nor is clear which processors / processor families / boards are
affected.

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: wd at denx.de
People are always a lot more complicated than you  think.  It's  very
important to remember that.             - Terry Pratchett, _Truckers_

[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.zang@freescale.com> you wrote:
> The original m88e1111s_config() does not do the SGMII mode
> initialization and is buggy. Rewrite the function according to
> 3.0.6 kernel function m88e1111_config_init() in drivers/net/phy/marvell.c
> 
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> Acked-by: Andy Fleming <afleming@freescale.com>
> Cc: Kumar Gala <galak@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: wd at denx.de
A supercomputer is a machine that runs an endless loop in 2 seconds.

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

[Zang Roy-R61911]

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Monday, October 24, 2011 3:42 AM
> To: Zang Roy-R61911
> Cc: u-boot at lists.denx.de; Kumar Gala
> Subject: Re: [U-Boot] [PATCH] phy/marvell: Rewrite the MV88E1111 phy config
> function based on kernel code
> 
> Dear Roy Zang,
> 
> In message <1319178713-12472-2-git-send-email-tie-fei.zang@freescale.com> you
> wrote:
> > The original m88e1111s_config() does not do the SGMII mode
> > initialization and is buggy. Rewrite the function according to
> > 3.0.6 kernel function m88e1111_config_init() in drivers/net/phy/marvell.c
> >
> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > Acked-by: Andy Fleming <afleming@freescale.com>
> > Cc: Kumar Gala <galak@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 MV88E1111 user manual requests "any changes to HWCFG_MODE of Extended PHY Specific Status Register must be followed by software reset to take effect"

[U-Boot] [PATCH] powerpc/fm: remove the TBIPA setting on platform code

[Zang Roy-R61911]

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Monday, October 24, 2011 3:37 AM
> To: Zang Roy-R61911
> Cc: u-boot at lists.denx.de; Fleming Andy-AFLEMING; Kumar Gala
> Subject: Re: [U-Boot] [PATCH] powerpc/fm: remove the TBIPA setting on platform
> code
> 
> Dear Roy Zang,
> 
> In message <1319178713-12472-1-git-send-email-tie-fei.zang@freescale.com> you
> wrote:
> > TBIPA has been set in dtsec_init_phy () funciton in drivers/net/fm/eth.c
> >
> > So remove the duplicate code on platform Ethernet code.
> >
> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > Cc: Andy Fleming <afleming@freescale.com>
> > Cc: Kumar Gala <galak@kernel.crashing.org>
> 
> Please change the Subject: so everybody understands what you are
> doing. "powerpc/fm" is not exactly clear to everybody, and neither is
> TBIPA.
> 
> Nor is clear which processors / processor families / boards are
> affected.
Per my understand, subject is a summary of the patch. poweper/fm and TBIPA should almost be OK for the subject. I can point out that the code is about the network code in subject.
for example, Subject:
powerpc/fm: remove the TBIPA setting on platform network related code

Then I add more in the patch description to explain fm, TBIPA, processors, processor families/boards affected.

Thanks.
Roy

[U-Boot] [PATCH] powerpc/fm: remove the TBIPA setting on platform code

[Wolfgang Denk]

Dear Zang Roy-R61911,

In message <2239AC579C7D3646A720227A37E02681200AAA@039-SN1MPN1-004.039d.mgd.msft.net> you wrote:
> 
> > Please change the Subject: so everybody understands what you are
> > doing. "powerpc/fm" is not exactly clear to everybody, and neither is
> > TBIPA.
> > 
> > Nor is clear which processors / processor families / boards are
> > affected.
> Per my understand, subject is a summary of the patch. poweper/fm and TBIPA should almost be OK for the subject. I can point out that the code is about the network code in subject.
> for example, Subject:
> powerpc/fm: remove the TBIPA setting on platform network related code

No.  I have not the lightest idea what FM (Frequency Modulation?) or
TBIPA might be.

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: wd at denx.de
[Doctors and Bartenders], We both get the same two kinds of customers
-- the living and the dying.
	-- Dr. Boyce, "The Menagerie" ("The Cage"), stardate unknown

[U-Boot] [PATCH] powerpc/fm: remove the TBIPA setting on platform code

[Zang Roy-R61911]

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Monday, October 24, 2011 13:24 PM
> To: Zang Roy-R61911
> Cc: u-boot at lists.denx.de; Fleming Andy-AFLEMING; Kumar Gala
> Subject: Re: [U-Boot] [PATCH] powerpc/fm: remove the TBIPA setting on platform
> code
> 
> Dear Zang Roy-R61911,
> 
> In message <2239AC579C7D3646A720227A37E02681200AAA@039-SN1MPN1-
> 004.039d.mgd.msft.net> you wrote:
> >
> > > Please change the Subject: so everybody understands what you are
> > > doing. "powerpc/fm" is not exactly clear to everybody, and neither is
> > > TBIPA.
> > >
> > > Nor is clear which processors / processor families / boards are
> > > affected.
> > Per my understand, subject is a summary of the patch. poweper/fm and TBIPA
> should almost be OK for the subject. I can point out that the code is about the
> network code in subject.
> > for example, Subject:
> > powerpc/fm: remove the TBIPA setting on platform network related code
> 
> No.  I have not the lightest idea what FM (Frequency Modulation?) or
> TBIPA might be.

Any way you are the owner.
How about this way:
Subject: net/frame manager: remove TBI PHY address register setting on platform related code

Then I add the other more detailed required information in the description body?
Thanks.
Roy

[U-Boot] [PATCH] powerpc/fm: remove the TBIPA setting on platform code

[Wolfgang Denk]

Dear Zang Roy-R61911,

In message <2239AC579C7D3646A720227A37E02681200C29@039-SN1MPN1-004.039d.mgd.msft.net> you wrote:
> 
> How about this way:
> Subject: net/frame manager: remove TBI PHY address register setting on plat=
> form related code
> 
> Then I add the other more detailed required information in the description =
> body?

You are putting too much low level detail into the Subject: line,
while leaving out important higher level information, like
architecture, SoC, etc.

How about something like:

	QorIQ: fix network frame manager settings

Then put all the rest into the commit message.

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: wd at denx.de
"I used to think that the brain was the most wonderful  organ  in  my
body. Then I realized who was telling me this."        - Emo Phillips

[U-Boot] [PATCH] powerpc/fm: remove the TBIPA setting on platform code

[Zang Roy-R61911]

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Tuesday, October 25, 2011 3:05 AM
> To: Zang Roy-R61911
> Cc: u-boot at lists.denx.de; Fleming Andy-AFLEMING; Kumar Gala
> Subject: Re: [U-Boot] [PATCH] powerpc/fm: remove the TBIPA setting on platform
> code
> 
> Dear Zang Roy-R61911,
> 
> In message <2239AC579C7D3646A720227A37E02681200C29@039-SN1MPN1-
> 004.039d.mgd.msft.net> you wrote:
> >
> > How about this way:
> > Subject: net/frame manager: remove TBI PHY address register setting on plat=
> > form related code
> >
> > Then I add the other more detailed required information in the description =
> > body?
> 
> You are putting too much low level detail into the Subject: line,
> while leaving out important higher level information, like
> architecture, SoC, etc.
> 
> How about something like:
> 
> 	QorIQ: fix network frame manager settings
This subject is too big. What kind of network frame manager setting? It will be hard for someone to find useful information in the subject.
Prefer:
QorIQ: fix network frame manager TBI PHY address settings

Roy