[U-Boot] [PATCH] ARM: mxs: Do not reconfigure FEC clock in FEC init

[U-Boot] [PATCH] ARM: mxs: Do not reconfigure FEC clock in FEC init

[Marek Vasut]

Do not reconfigure the FEC clock during board_eth_init(), otherwise
the FEC might have stability issues, refuse to autonegotiate link
entirely or even corrupt packets while indicating correct checksum
on them. Instead, move the FEC clock init to board_early_init_f(),
where all the other upstream clock are initialized and also make
sure there is proper stabilization delay.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Hector Palacios <hector.palacios@digi.com>
Cc: Oliver Metz <oliver@freetz.org>
Cc: Otavio Salvador <otavio@ossystems.com.br>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Tom Rini <trini@ti.com>
---
 board/bluegiga/apx4devkit/apx4devkit.c   | 10 ++++------
 board/denx/m28evk/m28evk.c               | 21 +++++++++++----------
 board/freescale/mx28evk/mx28evk.c        |  9 +++++----
 board/schulercontrol/sc_sps_1/sc_sps_1.c | 19 +++++++++++--------
 4 files changed, 31 insertions(+), 28 deletions(-)

NOTE: I'd like to get this into current release as it fixes a serious
      issue I observe here on the MX28EVK (packets being corrupted on
      long transfers initiated early after boot). Please test this and
      report back ASAP.

diff --git a/board/bluegiga/apx4devkit/apx4devkit.c b/board/bluegiga/apx4devkit/apx4devkit.c
index 08e79bd..044a182 100644
--- a/board/bluegiga/apx4devkit/apx4devkit.c
+++ b/board/bluegiga/apx4devkit/apx4devkit.c
@@ -39,6 +39,10 @@ int board_early_init_f(void)
 	/* SSP0 clock at 96MHz */
 	mxs_set_sspclk(MXC_SSPCLK0, 96000, 0);
 
+#ifdef CONFIG_CMD_NET
+	cpu_eth_init(NULL);
+	udelay(10000);
+#endif
 	return 0;
 }
 
@@ -79,12 +83,6 @@ int board_eth_init(bd_t *bis)
 	int ret;
 	struct eth_device *dev;
 
-	ret = cpu_eth_init(bis);
-	if (ret) {
-		printf("FEC MXS: Unable to init FEC clocks\n");
-		return ret;
-	}
-
 	ret = fecmxc_initialize(bis);
 	if (ret) {
 		printf("FEC MXS: Unable to init FEC\n");
diff --git a/board/denx/m28evk/m28evk.c b/board/denx/m28evk/m28evk.c
index 33d38cf..5065ee8 100644
--- a/board/denx/m28evk/m28evk.c
+++ b/board/denx/m28evk/m28evk.c
@@ -26,6 +26,9 @@ DECLARE_GLOBAL_DATA_PTR;
  */
 int board_early_init_f(void)
 {
+	struct mxs_clkctrl_regs *clkctrl_regs =
+		(struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
+
 	/* IO0 clock at 480MHz */
 	mxs_set_ioclk(MXC_IOCLK0, 480000);
 	/* IO1 clock at 480MHz */
@@ -36,6 +39,14 @@ int board_early_init_f(void)
 	/* SSP2 clock at 160MHz */
 	mxs_set_sspclk(MXC_SSPCLK2, 160000, 0);
 
+#ifdef CONFIG_CMD_NET
+	cpu_eth_init(NULL);
+	clrsetbits_le32(&clkctrl_regs->hw_clkctrl_enet,
+		CLKCTRL_ENET_TIME_SEL_MASK | CLKCTRL_ENET_CLK_OUT_EN,
+		CLKCTRL_ENET_TIME_SEL_RMII_CLK);
+	udelay(10000);
+#endif
+
 #ifdef	CONFIG_CMD_USB
 	mxs_iomux_setup_pad(MX28_PAD_SSP2_SS1__USB1_OVERCURRENT);
 	mxs_iomux_setup_pad(MX28_PAD_AUART3_TX__GPIO_3_13 |
@@ -110,19 +121,9 @@ int fecmxc_mii_postcall(int phy)
 
 int board_eth_init(bd_t *bis)
 {
-	struct mxs_clkctrl_regs *clkctrl_regs =
-		(struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
 	struct eth_device *dev;
 	int ret;
 
-	ret = cpu_eth_init(bis);
-	if (ret)
-		return ret;
-
-	clrsetbits_le32(&clkctrl_regs->hw_clkctrl_enet,
-		CLKCTRL_ENET_TIME_SEL_MASK | CLKCTRL_ENET_CLK_OUT_EN,
-		CLKCTRL_ENET_TIME_SEL_RMII_CLK);
-
 #if !defined(CONFIG_DENX_M28_V11) && !defined(CONFIG_DENX_M28_V10)
 	/* Reset the new PHY */
 	gpio_direction_output(MX28_PAD_AUART2_RTS__GPIO_3_11, 0);
diff --git a/board/freescale/mx28evk/mx28evk.c b/board/freescale/mx28evk/mx28evk.c
index 5005fe2..2c93c44 100644
--- a/board/freescale/mx28evk/mx28evk.c
+++ b/board/freescale/mx28evk/mx28evk.c
@@ -41,6 +41,11 @@ int board_early_init_f(void)
 	/* SSP2 clock at 160MHz */
 	mxs_set_sspclk(MXC_SSPCLK2, 160000, 0);
 
+#ifdef CONFIG_CMD_NET
+	cpu_eth_init(NULL);
+	udelay(10000);
+#endif
+
 #ifdef	CONFIG_CMD_USB
 	mxs_iomux_setup_pad(MX28_PAD_SSP2_SS1__USB1_OVERCURRENT);
 	mxs_iomux_setup_pad(MX28_PAD_AUART2_RX__GPIO_3_8 |
@@ -102,10 +107,6 @@ int board_eth_init(bd_t *bis)
 	struct eth_device *dev;
 	int ret;
 
-	ret = cpu_eth_init(bis);
-	if (ret)
-		return ret;
-
 	/* MX28EVK uses ENET_CLK PAD to drive FEC clock */
 	writel(CLKCTRL_ENET_TIME_SEL_RMII_CLK | CLKCTRL_ENET_CLK_OUT_EN,
 	       &clkctrl_regs->hw_clkctrl_enet);
diff --git a/board/schulercontrol/sc_sps_1/sc_sps_1.c b/board/schulercontrol/sc_sps_1/sc_sps_1.c
index 7f0b591..9d3c970 100644
--- a/board/schulercontrol/sc_sps_1/sc_sps_1.c
+++ b/board/schulercontrol/sc_sps_1/sc_sps_1.c
@@ -26,6 +26,9 @@ DECLARE_GLOBAL_DATA_PTR;
  */
 int board_early_init_f(void)
 {
+	struct mxs_clkctrl_regs *clkctrl_regs =
+		(struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
+
 	/* IO0 clock at 480MHz */
 	mxs_set_ioclk(MXC_IOCLK0, 480000);
 	/* IO1 clock at 480MHz */
@@ -36,6 +39,14 @@ int board_early_init_f(void)
 	/* SSP2 clock at 96MHz */
 	mxs_set_sspclk(MXC_SSPCLK2, 96000, 0);
 
+#ifdef CONFIG_CMD_NET
+	cpu_eth_init(NULL);
+	clrsetbits_le32(&clkctrl_regs->hw_clkctrl_enet,
+		CLKCTRL_ENET_TIME_SEL_MASK,
+		CLKCTRL_ENET_TIME_SEL_RMII_CLK | CLKCTRL_ENET_CLK_OUT_EN);
+	udelay(10000);
+#endif
+
 #ifdef	CONFIG_CMD_USB
 	mxs_iomux_setup_pad(MX28_PAD_AUART1_CTS__USB0_OVERCURRENT);
 	mxs_iomux_setup_pad(MX28_PAD_AUART2_TX__GPIO_3_9 |
@@ -69,16 +80,8 @@ int board_mmc_init(bd_t *bis)
 #ifdef	CONFIG_CMD_NET
 int board_eth_init(bd_t *bis)
 {
-	struct mxs_clkctrl_regs *clkctrl_regs =
-		(struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
 	int ret;
 
-	ret = cpu_eth_init(bis);
-
-	clrsetbits_le32(&clkctrl_regs->hw_clkctrl_enet,
-		CLKCTRL_ENET_TIME_SEL_MASK,
-		CLKCTRL_ENET_TIME_SEL_RMII_CLK | CLKCTRL_ENET_CLK_OUT_EN);
-
 	ret = fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE);
 	if (ret) {
 		printf("FEC MXS: Unable to init FEC0\n");
-- 
1.8.4.rc3

[U-Boot] [PATCH] ARM: mxs: Do not reconfigure FEC clock in FEC init

[Marek Vasut]

Hi,

CCing Alexandre and Hector. Can you test please?

> Do not reconfigure the FEC clock during board_eth_init(), otherwise
> the FEC might have stability issues, refuse to autonegotiate link
> entirely or even corrupt packets while indicating correct checksum
> on them. Instead, move the FEC clock init to board_early_init_f(),
> where all the other upstream clock are initialized and also make
> sure there is proper stabilization delay.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Hector Palacios <hector.palacios@digi.com>
> Cc: Oliver Metz <oliver@freetz.org>
> Cc: Otavio Salvador <otavio@ossystems.com.br>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Tom Rini <trini@ti.com>
> ---
>  board/bluegiga/apx4devkit/apx4devkit.c   | 10 ++++------
>  board/denx/m28evk/m28evk.c               | 21 +++++++++++----------
>  board/freescale/mx28evk/mx28evk.c        |  9 +++++----
>  board/schulercontrol/sc_sps_1/sc_sps_1.c | 19 +++++++++++--------
>  4 files changed, 31 insertions(+), 28 deletions(-)
> 
> NOTE: I'd like to get this into current release as it fixes a serious
>       issue I observe here on the MX28EVK (packets being corrupted on
>       long transfers initiated early after boot). Please test this and
>       report back ASAP.
> 
> diff --git a/board/bluegiga/apx4devkit/apx4devkit.c
> b/board/bluegiga/apx4devkit/apx4devkit.c index 08e79bd..044a182 100644
> --- a/board/bluegiga/apx4devkit/apx4devkit.c
> +++ b/board/bluegiga/apx4devkit/apx4devkit.c
> @@ -39,6 +39,10 @@ int board_early_init_f(void)
>  	/* SSP0 clock at 96MHz */
>  	mxs_set_sspclk(MXC_SSPCLK0, 96000, 0);
> 
> +#ifdef CONFIG_CMD_NET
> +	cpu_eth_init(NULL);
> +	udelay(10000);
> +#endif
>  	return 0;
>  }
> 
> @@ -79,12 +83,6 @@ int board_eth_init(bd_t *bis)
>  	int ret;
>  	struct eth_device *dev;
> 
> -	ret = cpu_eth_init(bis);
> -	if (ret) {
> -		printf("FEC MXS: Unable to init FEC clocks\n");
> -		return ret;
> -	}
> -
>  	ret = fecmxc_initialize(bis);
>  	if (ret) {
>  		printf("FEC MXS: Unable to init FEC\n");
> diff --git a/board/denx/m28evk/m28evk.c b/board/denx/m28evk/m28evk.c
> index 33d38cf..5065ee8 100644
> --- a/board/denx/m28evk/m28evk.c
> +++ b/board/denx/m28evk/m28evk.c
> @@ -26,6 +26,9 @@ DECLARE_GLOBAL_DATA_PTR;
>   */
>  int board_early_init_f(void)
>  {
> +	struct mxs_clkctrl_regs *clkctrl_regs =
> +		(struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
> +
>  	/* IO0 clock at 480MHz */
>  	mxs_set_ioclk(MXC_IOCLK0, 480000);
>  	/* IO1 clock at 480MHz */
> @@ -36,6 +39,14 @@ int board_early_init_f(void)
>  	/* SSP2 clock at 160MHz */
>  	mxs_set_sspclk(MXC_SSPCLK2, 160000, 0);
> 
> +#ifdef CONFIG_CMD_NET
> +	cpu_eth_init(NULL);
> +	clrsetbits_le32(&clkctrl_regs->hw_clkctrl_enet,
> +		CLKCTRL_ENET_TIME_SEL_MASK | CLKCTRL_ENET_CLK_OUT_EN,
> +		CLKCTRL_ENET_TIME_SEL_RMII_CLK);
> +	udelay(10000);
> +#endif
> +
>  #ifdef	CONFIG_CMD_USB
>  	mxs_iomux_setup_pad(MX28_PAD_SSP2_SS1__USB1_OVERCURRENT);
>  	mxs_iomux_setup_pad(MX28_PAD_AUART3_TX__GPIO_3_13 |
> @@ -110,19 +121,9 @@ int fecmxc_mii_postcall(int phy)
> 
>  int board_eth_init(bd_t *bis)
>  {
> -	struct mxs_clkctrl_regs *clkctrl_regs =
> -		(struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
>  	struct eth_device *dev;
>  	int ret;
> 
> -	ret = cpu_eth_init(bis);
> -	if (ret)
> -		return ret;
> -
> -	clrsetbits_le32(&clkctrl_regs->hw_clkctrl_enet,
> -		CLKCTRL_ENET_TIME_SEL_MASK | CLKCTRL_ENET_CLK_OUT_EN,
> -		CLKCTRL_ENET_TIME_SEL_RMII_CLK);
> -
>  #if !defined(CONFIG_DENX_M28_V11) && !defined(CONFIG_DENX_M28_V10)
>  	/* Reset the new PHY */
>  	gpio_direction_output(MX28_PAD_AUART2_RTS__GPIO_3_11, 0);
> diff --git a/board/freescale/mx28evk/mx28evk.c
> b/board/freescale/mx28evk/mx28evk.c index 5005fe2..2c93c44 100644
> --- a/board/freescale/mx28evk/mx28evk.c
> +++ b/board/freescale/mx28evk/mx28evk.c
> @@ -41,6 +41,11 @@ int board_early_init_f(void)
>  	/* SSP2 clock at 160MHz */
>  	mxs_set_sspclk(MXC_SSPCLK2, 160000, 0);
> 
> +#ifdef CONFIG_CMD_NET
> +	cpu_eth_init(NULL);
> +	udelay(10000);
> +#endif
> +
>  #ifdef	CONFIG_CMD_USB
>  	mxs_iomux_setup_pad(MX28_PAD_SSP2_SS1__USB1_OVERCURRENT);
>  	mxs_iomux_setup_pad(MX28_PAD_AUART2_RX__GPIO_3_8 |
> @@ -102,10 +107,6 @@ int board_eth_init(bd_t *bis)
>  	struct eth_device *dev;
>  	int ret;
> 
> -	ret = cpu_eth_init(bis);
> -	if (ret)
> -		return ret;
> -
>  	/* MX28EVK uses ENET_CLK PAD to drive FEC clock */
>  	writel(CLKCTRL_ENET_TIME_SEL_RMII_CLK | CLKCTRL_ENET_CLK_OUT_EN,
>  	       &clkctrl_regs->hw_clkctrl_enet);
> diff --git a/board/schulercontrol/sc_sps_1/sc_sps_1.c
> b/board/schulercontrol/sc_sps_1/sc_sps_1.c index 7f0b591..9d3c970 100644
> --- a/board/schulercontrol/sc_sps_1/sc_sps_1.c
> +++ b/board/schulercontrol/sc_sps_1/sc_sps_1.c
> @@ -26,6 +26,9 @@ DECLARE_GLOBAL_DATA_PTR;
>   */
>  int board_early_init_f(void)
>  {
> +	struct mxs_clkctrl_regs *clkctrl_regs =
> +		(struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
> +
>  	/* IO0 clock at 480MHz */
>  	mxs_set_ioclk(MXC_IOCLK0, 480000);
>  	/* IO1 clock at 480MHz */
> @@ -36,6 +39,14 @@ int board_early_init_f(void)
>  	/* SSP2 clock at 96MHz */
>  	mxs_set_sspclk(MXC_SSPCLK2, 96000, 0);
> 
> +#ifdef CONFIG_CMD_NET
> +	cpu_eth_init(NULL);
> +	clrsetbits_le32(&clkctrl_regs->hw_clkctrl_enet,
> +		CLKCTRL_ENET_TIME_SEL_MASK,
> +		CLKCTRL_ENET_TIME_SEL_RMII_CLK | CLKCTRL_ENET_CLK_OUT_EN);
> +	udelay(10000);
> +#endif
> +
>  #ifdef	CONFIG_CMD_USB
>  	mxs_iomux_setup_pad(MX28_PAD_AUART1_CTS__USB0_OVERCURRENT);
>  	mxs_iomux_setup_pad(MX28_PAD_AUART2_TX__GPIO_3_9 |
> @@ -69,16 +80,8 @@ int board_mmc_init(bd_t *bis)
>  #ifdef	CONFIG_CMD_NET
>  int board_eth_init(bd_t *bis)
>  {
> -	struct mxs_clkctrl_regs *clkctrl_regs =
> -		(struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
>  	int ret;
> 
> -	ret = cpu_eth_init(bis);
> -
> -	clrsetbits_le32(&clkctrl_regs->hw_clkctrl_enet,
> -		CLKCTRL_ENET_TIME_SEL_MASK,
> -		CLKCTRL_ENET_TIME_SEL_RMII_CLK | CLKCTRL_ENET_CLK_OUT_EN);
> -
>  	ret = fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE);
>  	if (ret) {
>  		printf("FEC MXS: Unable to init FEC0\n");

[U-Boot] [PATCH] ARM: mxs: Do not reconfigure FEC clock in FEC init

[Hector Palacios]

Dear Marek,

On 10/13/2013 05:20 PM, Marek Vasut wrote:
> Do not reconfigure the FEC clock during board_eth_init(), otherwise
> the FEC might have stability issues, refuse to autonegotiate link
> entirely or even corrupt packets while indicating correct checksum
> on them. Instead, move the FEC clock init to board_early_init_f(),
> where all the other upstream clock are initialized and also make
> sure there is proper stabilization delay.

Do you have any means to reproduce the problem?
I have seldom seen times when the Ethernet did not work, but it was so infrequent that 
it was impossible to know where it came from or how to reproduce it.


[...]

> diff --git a/board/denx/m28evk/m28evk.c b/board/denx/m28evk/m28evk.c
> index 33d38cf..5065ee8 100644
> --- a/board/denx/m28evk/m28evk.c
> +++ b/board/denx/m28evk/m28evk.c
> @@ -26,6 +26,9 @@ DECLARE_GLOBAL_DATA_PTR;
>    */
>   int board_early_init_f(void)
>   {
> +	struct mxs_clkctrl_regs *clkctrl_regs =
> +		(struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
> +

You may want to wrap these within #ifdef CONFIG_CMD_NET to avoid a warning if not defined.


> diff --git a/board/schulercontrol/sc_sps_1/sc_sps_1.c b/board/schulercontrol/sc_sps_1/sc_sps_1.c
> index 7f0b591..9d3c970 100644
> --- a/board/schulercontrol/sc_sps_1/sc_sps_1.c
> +++ b/board/schulercontrol/sc_sps_1/sc_sps_1.c
> @@ -26,6 +26,9 @@ DECLARE_GLOBAL_DATA_PTR;
>    */
>   int board_early_init_f(void)
>   {
> +	struct mxs_clkctrl_regs *clkctrl_regs =
> +		(struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
> +

And the same here.

It works ok, but I can't say if it fixes exactly those seldom initialization problems 
because I don't have a reliable way to reproduce them.

Tested-by: Hector Palacios <hector.palacios@digi.com>

Best regards,
--
Hector Palacios

[U-Boot] [PATCH] ARM: mxs: Do not reconfigure FEC clock in FEC init

[Hector Palacios]

Dear Marek,

On 10/13/2013 05:20 PM, Marek Vasut wrote:
> Do not reconfigure the FEC clock during board_eth_init(), otherwise
> the FEC might have stability issues, refuse to autonegotiate link
> entirely or even corrupt packets while indicating correct checksum
> on them. Instead, move the FEC clock init to board_early_init_f(),
> where all the other upstream clock are initialized and also make
> sure there is proper stabilization delay.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Hector Palacios <hector.palacios@digi.com>
> Cc: Oliver Metz <oliver@freetz.org>
> Cc: Otavio Salvador <otavio@ossystems.com.br>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Tom Rini <trini@ti.com>
> ---
>   board/bluegiga/apx4devkit/apx4devkit.c   | 10 ++++------
>   board/denx/m28evk/m28evk.c               | 21 +++++++++++----------
>   board/freescale/mx28evk/mx28evk.c        |  9 +++++----
>   board/schulercontrol/sc_sps_1/sc_sps_1.c | 19 +++++++++++--------
>   4 files changed, 31 insertions(+), 28 deletions(-)

[...]

> diff --git a/board/freescale/mx28evk/mx28evk.c b/board/freescale/mx28evk/mx28evk.c
> index 5005fe2..2c93c44 100644
> --- a/board/freescale/mx28evk/mx28evk.c
> +++ b/board/freescale/mx28evk/mx28evk.c
> @@ -41,6 +41,11 @@ int board_early_init_f(void)
>   	/* SSP2 clock at 160MHz */
>   	mxs_set_sspclk(MXC_SSPCLK2, 160000, 0);
>
> +#ifdef CONFIG_CMD_NET
> +	cpu_eth_init(NULL);
> +	udelay(10000);
> +#endif
> +
>   #ifdef	CONFIG_CMD_USB
>   	mxs_iomux_setup_pad(MX28_PAD_SSP2_SS1__USB1_OVERCURRENT);
>   	mxs_iomux_setup_pad(MX28_PAD_AUART2_RX__GPIO_3_8 |
> @@ -102,10 +107,6 @@ int board_eth_init(bd_t *bis)
>   	struct eth_device *dev;
>   	int ret;
>
> -	ret = cpu_eth_init(bis);
> -	if (ret)
> -		return ret;
> -
>   	/* MX28EVK uses ENET_CLK PAD to drive FEC clock */
>   	writel(CLKCTRL_ENET_TIME_SEL_RMII_CLK | CLKCTRL_ENET_CLK_OUT_EN,
>   	       &clkctrl_regs->hw_clkctrl_enet);

Why didn't you move the clk enable instruction on mx28evk, as you did with the rest of 
platforms?


Best regards,
--
Hector Palacios

[U-Boot] [PATCH] ARM: mxs: Do not reconfigure FEC clock in FEC init

[Stefano Babic]

Hi Marek,

On 13/10/2013 17:20, Marek Vasut wrote:
> Do not reconfigure the FEC clock during board_eth_init(), otherwise
> the FEC might have stability issues, refuse to autonegotiate link
> entirely or even corrupt packets while indicating correct checksum
> on them. Instead, move the FEC clock init to board_early_init_f(),
> where all the other upstream clock are initialized and also make
> sure there is proper stabilization delay.

However, it seems to me that moving the code remove the problem on the
mx28evk without finding the cause. Really there is still a big chunk of
time between the initialization of the clock and the first packet is
sent to the network, when the autonegotiation should occur.

> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Hector Palacios <hector.palacios@digi.com>
> Cc: Oliver Metz <oliver@freetz.org>
> Cc: Otavio Salvador <otavio@ossystems.com.br>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Tom Rini <trini@ti.com>
> ---
>  board/bluegiga/apx4devkit/apx4devkit.c   | 10 ++++------
>  board/denx/m28evk/m28evk.c               | 21 +++++++++++----------
>  board/freescale/mx28evk/mx28evk.c        |  9 +++++----
>  board/schulercontrol/sc_sps_1/sc_sps_1.c | 19 +++++++++++--------
>  4 files changed, 31 insertions(+), 28 deletions(-)
> 
> NOTE: I'd like to get this into current release as it fixes a serious
>       issue I observe here on the MX28EVK (packets being corrupted on
>       long transfers initiated early after boot). Please test this and
>       report back ASAP.
> 
> diff --git a/board/bluegiga/apx4devkit/apx4devkit.c b/board/bluegiga/apx4devkit/apx4devkit.c
> index 08e79bd..044a182 100644
> --- a/board/bluegiga/apx4devkit/apx4devkit.c
> +++ b/board/bluegiga/apx4devkit/apx4devkit.c
> @@ -39,6 +39,10 @@ int board_early_init_f(void)
>  	/* SSP0 clock at 96MHz */
>  	mxs_set_sspclk(MXC_SSPCLK0, 96000, 0);
>  
> +#ifdef CONFIG_CMD_NET
> +	cpu_eth_init(NULL);
> +	udelay(10000);
> +#endif

This looks like a hack...

Calling cpu_eth_init() without parameters is absolutely an exception in
u-boot, there is no other instance / SOC doing this.

Are we sure that the issue you see on mx28evk are reproducible on the
other boards ? You are starting to fix all boards, but you report
corruption only on one board.

>  	return 0;
>  }
>  
> @@ -79,12 +83,6 @@ int board_eth_init(bd_t *bis)
>  	int ret;
>  	struct eth_device *dev;
>  
> -	ret = cpu_eth_init(bis);
> -	if (ret) {
> -		printf("FEC MXS: Unable to init FEC clocks\n");
> -		return ret;
> -	}
> -
>  	ret = fecmxc_initialize(bis);
>  	if (ret) {
>  		printf("FEC MXS: Unable to init FEC\n");
> diff --git a/board/denx/m28evk/m28evk.c b/board/denx/m28evk/m28evk.c
> index 33d38cf..5065ee8 100644
> --- a/board/denx/m28evk/m28evk.c
> +++ b/board/denx/m28evk/m28evk.c
> @@ -26,6 +26,9 @@ DECLARE_GLOBAL_DATA_PTR;
>   */
>  int board_early_init_f(void)
>  {
> +	struct mxs_clkctrl_regs *clkctrl_regs =
> +		(struct mxs_clkctrl_regs *)MXS_CLKCTRL_BASE;
> +
>  	/* IO0 clock at 480MHz */
>  	mxs_set_ioclk(MXC_IOCLK0, 480000);
>  	/* IO1 clock at 480MHz */
> @@ -36,6 +39,14 @@ int board_early_init_f(void)
>  	/* SSP2 clock at 160MHz */
>  	mxs_set_sspclk(MXC_SSPCLK2, 160000, 0);
>  
> +#ifdef CONFIG_CMD_NET
> +	cpu_eth_init(NULL);
> +	clrsetbits_le32(&clkctrl_regs->hw_clkctrl_enet,
> +		CLKCTRL_ENET_TIME_SEL_MASK | CLKCTRL_ENET_CLK_OUT_EN,
> +		CLKCTRL_ENET_TIME_SEL_RMII_CLK);
> +	udelay(10000);

It seems to me that the most obvious change is the 10mS delay you added
after setting the clock. If we find that such delay is really necessary,
should we not fix it insife the cpu_eth_init() else in all board
initialization functions ?

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] ARM: mxs: Do not reconfigure FEC clock in FEC init

[Marek Vasut]

Dear Stefano Babic,

> Hi Marek,
> 
> On 13/10/2013 17:20, Marek Vasut wrote:
> > Do not reconfigure the FEC clock during board_eth_init(), otherwise
> > the FEC might have stability issues, refuse to autonegotiate link
> > entirely or even corrupt packets while indicating correct checksum
> > on them. Instead, move the FEC clock init to board_early_init_f(),
> > where all the other upstream clock are initialized and also make
> > sure there is proper stabilization delay.
> 
> However, it seems to me that moving the code remove the problem on the
> mx28evk without finding the cause. Really there is still a big chunk of
> time between the initialization of the clock and the first packet is
> sent to the network, when the autonegotiation should occur.

Forget this, the issue is still present. Damn!

It's at least a little clearer to me what happens now. The FEC fails to transmit 
an TFTP block ACK packet -> the server waits 5 seconds -> sends ARP packet -> 
FEC replies and resends the TFTP block ACK again. Why does the FEC swallow a TX 
packet, I don't know.

Best regards,
Marek Vasut

[U-Boot] [PATCH] ARM: mxs: Do not reconfigure FEC clock in FEC init

[Stefano Babic]

Hi Marek,

On 15/10/2013 23:19, Marek Vasut wrote:
> 
> Forget this, the issue is still present. Damn!
> 
> It's at least a little clearer to me what happens now. The FEC fails to transmit 
> an TFTP block ACK packet -> the server waits 5 seconds -> sends ARP packet -> 
> FEC replies and resends the TFTP block ACK again. Why does the FEC swallow a TX 
> packet, I don't know.

Anyway, it is strange that this happens only with this board. It appears
as the SOC sends or thinks to have sent, but the packet is corrupted or
not sent at all on the physical layer. Maybe you can check on the other
side of the connection on the switch and/or on the server, if some
errors are detected.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

[U-Boot] [PATCH] ARM: mxs: Do not reconfigure FEC clock in FEC init

[Marek Vasut]

Dear Stefano Babic,

> Hi Marek,
> 
> On 15/10/2013 23:19, Marek Vasut wrote:
> > Forget this, the issue is still present. Damn!
> > 
> > It's at least a little clearer to me what happens now. The FEC fails to
> > transmit an TFTP block ACK packet -> the server waits 5 seconds -> sends
> > ARP packet -> FEC replies and resends the TFTP block ACK again. Why does
> > the FEC swallow a TX packet, I don't know.
> 
> Anyway, it is strange that this happens only with this board. It appears
> as the SOC sends or thinks to have sent, but the packet is corrupted or
> not sent at all on the physical layer. Maybe you can check on the other
> side of the connection on the switch and/or on the server, if some
> errors are detected.

This happens on multiple boards and we're sometimes getting reports about this 
issue. So this is not a single board issue. Maybe we should figure out what do 
these boards have in common.

Best regards,
Marek Vasut