[U-Boot] [PATCH] powerpc/fm: remove the TBIPA setting on platform code
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@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); - /* 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@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); - /* 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@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;
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);
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);doreg = 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
-----Original Message----- From: Wolfgang Denk [mailto:wd@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 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);doreg = 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"
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: wd@denx.de A supercomputer is a machine that runs an endless loop in 2 seconds.
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
-----Original Message----- From: Wolfgang Denk [mailto:wd@denx.de] Sent: Monday, October 24, 2011 3:37 AM To: Zang Roy-R61911 Cc: u-boot@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
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
-----Original Message----- From: Wolfgang Denk [mailto:wd@denx.de] Sent: Monday, October 24, 2011 13:24 PM To: Zang Roy-R61911 Cc: u-boot@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
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
-----Original Message----- From: Wolfgang Denk [mailto:wd@denx.de] Sent: Tuesday, October 25, 2011 3:05 AM To: Zang Roy-R61911 Cc: u-boot@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
participants (3)
-
Roy Zang -
Wolfgang Denk -
Zang Roy-R61911