[U-Boot] [PATCH] Revert "net: phy: delay only if reset handler is registered"
This reverts commit 59370f3fcd135089c402c93720a87c688abe600c.
This commit breaks ethernet on at least mx6 Riotboard and mx6sxsabresd boards.
Reported-by: Catalin Crenguta catalin.crenguta@gmail.com Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- drivers/net/phy/phy.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index d7364ff..9e68f1d 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -771,13 +771,11 @@ struct phy_device *phy_find_by_mask(struct mii_dev *bus, unsigned phy_mask, phy_interface_t interface) { /* Reset the bus */ - if (bus->reset) { + if (bus->reset) bus->reset(bus);
- /* Wait 15ms to make sure the PHY has come out of hard reset */ - udelay(15000); - } - + /* Wait 15ms to make sure the PHY has come out of hard reset */ + udelay(15000); return get_phy_device_by_mask(bus, phy_mask, interface); }
Hi Fabio,
On 17.11.2015 17:25, Fabio Estevam wrote:
This reverts commit 59370f3fcd135089c402c93720a87c688abe600c.
This commit breaks ethernet on at least mx6 Riotboard and mx6sxsabresd boards.
Reported-by: Catalin Crenguta catalin.crenguta@gmail.com Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
drivers/net/phy/phy.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index d7364ff..9e68f1d 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -771,13 +771,11 @@ struct phy_device *phy_find_by_mask(struct mii_dev *bus, unsigned phy_mask, phy_interface_t interface) { /* Reset the bus */
- if (bus->reset) {
- if (bus->reset) bus->reset(bus);
/* Wait 15ms to make sure the PHY has come out of hard reset */udelay(15000);- }
- /* Wait 15ms to make sure the PHY has come out of hard reset */
- udelay(15000); return get_phy_device_by_mask(bus, phy_mask, interface); }
I'm not sure if this revert is the right way to solve this problem. Please take a look at my answer a few weeks ago:
https://www.mail-archive.com/u-boot@lists.denx.de/msg191206.html
As mentioned in my mail above, the delay should be added after the deassertion of the PHY reset signal. And not here in phy_find_by_mask() for all boards.
I just checked the code in mx6sxsabresd.c. Here also the delay is missing:
/* Reset AR8031 PHY */ gpio_direction_output(IMX_GPIO_NR(2, 7) , 0); udelay(500); gpio_set_value(IMX_GPIO_NR(2, 7), 1);
Could you please test with this change:
/* Reset AR8031 PHY */ gpio_direction_output(IMX_GPIO_NR(2, 7) , 0); udelay(500); gpio_set_value(IMX_GPIO_NR(2, 7), 1); + udelay(1500);
Or even better, check how long the reset needs to be inactive before the PHY starts to work.
Thanks, Stefan
Hi Stefan,
On Wed, Nov 18, 2015 at 3:54 AM, Stefan Roese stefan.roese@gmail.com wrote:
I'm not sure if this revert is the right way to solve this problem. Please take a look at my answer a few weeks ago:
https://www.mail-archive.com/u-boot@lists.denx.de/msg191206.html
As mentioned in my mail above, the delay should be added after the deassertion of the PHY reset signal. And not here in phy_find_by_mask() for all boards.
I just checked the code in mx6sxsabresd.c. Here also the delay is missing:
/* Reset AR8031 PHY */ gpio_direction_output(IMX_GPIO_NR(2, 7) , 0); udelay(500); gpio_set_value(IMX_GPIO_NR(2, 7), 1);Could you please test with this change:
/* Reset AR8031 PHY */ gpio_direction_output(IMX_GPIO_NR(2, 7) , 0); udelay(500); gpio_set_value(IMX_GPIO_NR(2, 7), 1);
udelay(1500);Or even better, check how long the reset needs to be inactive before the PHY starts to work.
Sure, I have even tried:
--- a/board/freescale/mx6sxsabresd/mx6sxsabresd.c +++ b/board/freescale/mx6sxsabresd/mx6sxsabresd.c @@ -163,8 +163,9 @@ static int setup_fec(void)
/* Reset AR8031 PHY */ gpio_direction_output(IMX_GPIO_NR(2, 7) , 0); - udelay(500); + udelay(50000); gpio_set_value(IMX_GPIO_NR(2, 7), 1); + udelay(50000);
reg = readl(&anatop->pll_enet); reg |= BM_ANADIG_PLL_ENET_REF_25M_ENABLE;
and it does not work here:
U-Boot 2016.01-rc1-00022-gfe52456-dirty (Nov 18 2015 - 09:28:02 -0200)
CPU: Freescale i.MX6SX rev1.0 996 MHz (running at 792 MHz) CPU: Extended Commercial temperature grade (-20C to 105C) at 44C Reset cause: POR Board: MX6SX SABRE SDB I2C: ready DRAM: 1 GiB PMIC: PFUZE100 ID=0x10 MMC: FSL_SDHC: 0, FSL_SDHC: 1, FSL_SDHC: 2 PCI: pcie phy link never came up In: serial Out: serial Err: serial Net: Board Net Initialization Failed No ethernet found. Hit any key to stop autoboot: 0 Booting from net ... No ethernet found. No ethernet found. Bad Linux ARM zImage magic!
Regards,
Fabio Estevam
Hi Fabio,
On Di, 2015-11-17 at 14:25 -0200, Fabio Estevam wrote:
This reverts commit 59370f3fcd135089c402c93720a87c688abe600c.
This commit breaks ethernet on at least mx6 Riotboard and mx6sxsabresd boards.
Reported-by: Catalin Crenguta catalin.crenguta@gmail.com Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
drivers/net/phy/phy.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index d7364ff..9e68f1d 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -771,13 +771,11 @@ struct phy_device *phy_find_by_mask(struct mii_dev *bus, unsigned phy_mask, phy_interface_t interface) { /* Reset the bus */
- if (bus->reset) {
- if (bus->reset)
bus->reset(bus);
/* Wait 15ms to make sure the PHY has come out ofhard reset */
udelay(15000);- }
- /* Wait 15ms to make sure the PHY has come out of hard reset
*/
- udelay(15000);
return get_phy_device_by_mask(bus, phy_mask, interface); }
I think this is not the right thing to do here. It is true that the AR8035 ethernet chip of the RioTboard needs the clock to be stable for at least 1ms before RESET can be deasserted. This why it fails, if there is no MII reset function defined.
I think the right place to handle reset delays is the board_eth_init(). The value of 15ms currently used looks to me like an arbitrary value. I am not sure where this value is comming from. Some chips need more, some less time to delay after a reset.
This is snippet of how I do it for a custom i.MX28-EVK-based board:
int board_eth_init(bd_t *bis) { cpu_eth_init(bis); /* Power-on FEC */ gpio_direction_output(MX28_PAD_LCD_D21__GPIO_1_21, 0); /* Reset FEC PHY */ gpio_direction_output(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 0); /* Deassert nRST after 25 ms from power-up on (= t_purstd) */ udelay(25000); gpio_set_value(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 1); fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE); }
Best regards Jörg Krause
Hi Jörg,
On Wed, Nov 18, 2015 at 6:44 AM, Jörg Krause joerg.krause@embedded.rocks wrote:
I think this is not the right thing to do here. It is true that the AR8035 ethernet chip of the RioTboard needs the clock to be stable for at least 1ms before RESET can be deasserted. This why it fails, if there is no MII reset function defined.
In my case I am testing on a mx6sxsabresd which has two AR8031 chips.
I think the right place to handle reset delays is the board_eth_init(). The value of 15ms currently used looks to me like an arbitrary value. I am not sure where this value is comming from. Some chips need more, some less time to delay after a reset.
This is snippet of how I do it for a custom i.MX28-EVK-based board:
int board_eth_init(bd_t *bis) { cpu_eth_init(bis); /* Power-on FEC */ gpio_direction_output(MX28_PAD_LCD_D21__GPIO_1_21, 0); /* Reset FEC PHY */ gpio_direction_output(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 0); /* Deassert nRST after 25 ms from power-up on (= t_purstd) */ udelay(25000); gpio_set_value(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 1); fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE); }
We currently reset the Ethernet PHY inside setup_fec() inside board_eth_init().
Please check board/freescale/mx6sxsabresd/mx6sxsabresd.c.
I have also tried increasing the reset time and still do not have Ethernel functional.
Regards,
Fabio Estevam
On Mi, 2015-11-18 at 09:34 -0200, Fabio Estevam wrote:
Hi Jörg,
On Wed, Nov 18, 2015 at 6:44 AM, Jörg Krause joerg.krause@embedded.rocks wrote:
I think this is not the right thing to do here. It is true that the AR8035 ethernet chip of the RioTboard needs the clock to be stable for at least 1ms before RESET can be deasserted. This why it fails, if there is no MII reset function defined.
In my case I am testing on a mx6sxsabresd which has two AR8031 chips.
I think the right place to handle reset delays is the board_eth_init(). The value of 15ms currently used looks to me like an arbitrary value. I am not sure where this value is comming from. Some chips need more, some less time to delay after a reset.
This is snippet of how I do it for a custom i.MX28-EVK-based board:
int board_eth_init(bd_t *bis) { cpu_eth_init(bis); /* Power-on FEC */ gpio_direction_output(MX28_PAD_LCD_D21__GPIO_1_21, 0); /* Reset FEC PHY */ gpio_direction_output(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 0); /* Deassert nRST after 25 ms from power-up on (= t_purstd) */ udelay(25000); gpio_set_value(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 1); fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE); }
We currently reset the Ethernet PHY inside setup_fec() inside board_eth_init().
Please check board/freescale/mx6sxsabresd/mx6sxsabresd.c.
Ok, I checked. This is what happens in setup_fec():
1) Enable AR8031 power 2) Assert AR8031 RESET 3) Delay 0.5ms 4) Deassert AR8031 RESET 5) Enable anatop clock
Shouldn't the clock be enabled and become stable before deasserting the RESET line?
I have also tried increasing the reset time and still do not have Ethernel functional.
The AR8031 datasheets recommands a delay of 10ms.
Best regards Jörg Krause
Hi Jörg ,
On Thu, Nov 19, 2015 at 6:13 AM, Jörg Krause joerg.krause@embedded.rocks wrote:
Ok, I checked. This is what happens in setup_fec():
- Enable AR8031 power
- Assert AR8031 RESET
- Delay 0.5ms
- Deassert AR8031 RESET
- Enable anatop clock
Shouldn't the clock be enabled and become stable before deasserting the RESET line?
I have also tried increasing the reset time and still do not have Ethernel functional.
The AR8031 datasheets recommands a delay of 10ms.
Ok, I will test your proposal below on Monday when I get access to my mx6sxsabresd, thanks.
--- a/board/freescale/mx6sxsabresd/mx6sxsabresd.c +++ b/board/freescale/mx6sxsabresd/mx6sxsabresd.c @@ -150,11 +150,15 @@ static int setup_fec(void) { struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR; struct anatop_regs *anatop = (struct anatop_regs *)ANATOP_BASE_ADDR; - int reg; + int reg, ret;
/* Use 125MHz anatop loopback REF_CLK1 for ENET1 */ clrsetbits_le32(&iomuxc_regs->gpr[1], IOMUX_GPR1_FEC1_MASK, 0);
+ ret = enable_fec_anatop_clock(0, ENET_125MHZ); + if (ret) + return ret; + imx_iomux_v3_setup_multiple_pads(phy_control_pads, ARRAY_SIZE(phy_control_pads));
@@ -163,14 +167,14 @@ static int setup_fec(void)
/* Reset AR8031 PHY */ gpio_direction_output(IMX_GPIO_NR(2, 7) , 0); - udelay(500); + mdelay(10); gpio_set_value(IMX_GPIO_NR(2, 7), 1);
reg = readl(&anatop->pll_enet); reg |= BM_ANADIG_PLL_ENET_REF_25M_ENABLE; writel(reg, &anatop->pll_enet);
- return enable_fec_anatop_clock(0, ENET_125MHZ); + return 0; }
int board_eth_init(bd_t *bis)
Hi Jörg ,
On Fri, Nov 20, 2015 at 6:37 PM, Fabio Estevam festevam@gmail.com wrote:
Ok, I will test your proposal below on Monday when I get access to my mx6sxsabresd, thanks.
Your proposal worked fine, thanks. Will send it as a formal patch.
On Mo, 2015-11-23 at 16:16 -0200, Fabio Estevam wrote:
Hi Jörg ,
On Fri, Nov 20, 2015 at 6:37 PM, Fabio Estevam festevam@gmail.com wrote:
Ok, I will test your proposal below on Monday when I get access to my mx6sxsabresd, thanks.
Your proposal worked fine, thanks. Will send it as a formal patch.
This is good news!
Jörg
Hi Jörg,
On Mon, Nov 23, 2015 at 4:16 PM, Fabio Estevam festevam@gmail.com wrote:
Hi Jörg ,
On Fri, Nov 20, 2015 at 6:37 PM, Fabio Estevam festevam@gmail.com wrote:
Ok, I will test your proposal below on Monday when I get access to my mx6sxsabresd, thanks.
Your proposal worked fine, thanks. Will send it as a formal patch.
Ethernet on mx6cuboxi is also broken because of 59370f3fcd13508 (""net: phy: delay only if reset handler is registered").
I tried doing:
--- a/board/solidrun/mx6cuboxi/mx6cuboxi.c +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c @@ -143,7 +143,7 @@ static void setup_iomux_enet(void) SETUP_IOMUX_PADS(enet_pads);
gpio_direction_output(ETH_PHY_RESET, 0); - mdelay(2); + mdelay(10); gpio_set_value(ETH_PHY_RESET, 1); }
,but it did not help.
Any suggestions?
Thanks,
Fabio Estevam
Tom and Joe,
On Wed, Dec 23, 2015 at 1:42 PM, Fabio Estevam festevam@gmail.com wrote:
Ethernet on mx6cuboxi is also broken because of 59370f3fcd13508 (""net: phy: delay only if reset handler is registered").
I tried doing:
--- a/board/solidrun/mx6cuboxi/mx6cuboxi.c +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c @@ -143,7 +143,7 @@ static void setup_iomux_enet(void) SETUP_IOMUX_PADS(enet_pads);
gpio_direction_output(ETH_PHY_RESET, 0);
mdelay(2);
mdelay(10); gpio_set_value(ETH_PHY_RESET, 1);}
,but it did not help.
Any suggestions?
To avoid Ethernet breakage on at least Wandboard and Cubox-i boards on 2016.01, can we apply this revert patch in the meantime?
Hi Fabio,
On Mi, 2015-12-23 at 13:42 -0200, Fabio Estevam wrote:
Hi Jörg,
On Mon, Nov 23, 2015 at 4:16 PM, Fabio Estevam festevam@gmail.com wrote:
Hi Jörg ,
On Fri, Nov 20, 2015 at 6:37 PM, Fabio Estevam festevam@gmail.com wrote:
Ok, I will test your proposal below on Monday when I get access to my mx6sxsabresd, thanks.
Your proposal worked fine, thanks. Will send it as a formal patch.
Ethernet on mx6cuboxi is also broken because of 59370f3fcd13508 (""net: phy: delay only if reset handler is registered").
I tried doing:
--- a/board/solidrun/mx6cuboxi/mx6cuboxi.c +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c @@ -143,7 +143,7 @@ static void setup_iomux_enet(void) SETUP_IOMUX_PADS(enet_pads);
gpio_direction_output(ETH_PHY_RESET, 0); - mdelay(2); + mdelay(10); gpio_set_value(ETH_PHY_RESET, 1); }
,but it did not help.
Any suggestions?
Sorry for the long delay due to christmas vacation.
Do you know which PHY is used on mx6cuboxi?
I am wondering if the ANATOP clock setting is correct (25MHz)?
http://git.denx.de/?p=u-boot.git;a=blob;f=board/solidrun/mx6cuboxi/mx6c uboxi.c;h=fc37f1eef06da5147e5403d4272d220836c9cfbc;hb=HEAD#l167
Does it help to increase the delay and set it to 15ms?
Best regards Jörg Krause
Hi Jörg,
On Mon, Jan 4, 2016 at 10:49 AM, Jörg Krause joerg.krause@embedded.rocks wrote:
Do you know which PHY is used on mx6cuboxi?
It is an AR8035.
I am wondering if the ANATOP clock setting is correct (25MHz)?
http://git.denx.de/?p=u-boot.git;a=blob;f=board/solidrun/mx6cuboxi/mx6c uboxi.c;h=fc37f1eef06da5147e5403d4272d220836c9cfbc;hb=HEAD#l167
Yes, this seems correct for this board.
Does it help to increase the delay and set it to 15ms?
It does not help.
Thanks,
Fabio Estevam
Hi Fabio,
On Mo, 2016-01-04 at 10:57 -0200, Fabio Estevam wrote:
Hi Jörg,
On Mon, Jan 4, 2016 at 10:49 AM, Jörg Krause joerg.krause@embedded.rocks wrote:
Do you know which PHY is used on mx6cuboxi?
It is an AR8035.
I am wondering if the ANATOP clock setting is correct (25MHz)?
http://git.denx.de/?p=u-boot.git;a=blob;f=board/solidrun/mx6cuboxi/ mx6c uboxi.c;h=fc37f1eef06da5147e5403d4272d220836c9cfbc;hb=HEAD#l167
Yes, this seems correct for this board.
Does it help to increase the delay and set it to 15ms?
It does not help.
mx6cuboxi.h defines CONFIG_FEC_MXC and CONFIG_PHYLIB, but does not use cpu_eth_init() [which calls fecmxc_initialize()]. Is there any reason for this?
Jörg
Hi Jörg,
On Mon, Jan 4, 2016 at 11:48 AM, Jörg Krause joerg.krause@embedded.rocks wrote:
mx6cuboxi.h defines CONFIG_FEC_MXC and CONFIG_PHYLIB, but does not use cpu_eth_init() [which calls fecmxc_initialize()]. Is there any reason for this?
Will check it.
I am able to get Ethernet functional now if I do the same initialization as in mx6qsabresd, which uses AR8031.
Will test more and submit the patches later today.
Thanks
Hi Fabio,
On Mon, Jan 4, 2016 at 8:24 AM, Fabio Estevam festevam@gmail.com wrote:
Hi Jörg,
On Mon, Jan 4, 2016 at 11:48 AM, Jörg Krause joerg.krause@embedded.rocks wrote:
mx6cuboxi.h defines CONFIG_FEC_MXC and CONFIG_PHYLIB, but does not use cpu_eth_init() [which calls fecmxc_initialize()]. Is there any reason for this?
Will check it.
I am able to get Ethernet functional now if I do the same initialization as in mx6qsabresd, which uses AR8031.
Will test more and submit the patches later today.
Does this mean you rescind the request to apply this revert patch?
Thanks, -Joe
Hi Joe,
On Mon, Jan 4, 2016 at 4:35 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
Does this mean you rescind the request to apply this revert patch?
That's correct. There is no need to revert the patch.
I have sent patches (via i.MX code) that fix Ethernet on mx6cuboxi.
Thanks
participants (5)
-
Fabio Estevam -
Fabio Estevam -
Joe Hershberger -
Jörg Krause -
Stefan Roese