[U-Boot] [PATCH 1/1] net: lpc32xx: improve download rate in RMII phy mode

[U-Boot] [PATCH 1/1] net: lpc32xx: improve download rate in RMII phy mode

[LEMIEUX, SYLVAIN]

This is a minor update to LPC32xx MAC driver patch that add support for the RMII phy mode.

In the legacy BSP from NXP, in RMII phy mode, an extra 10ms delay was present after the release of the soft reset.

In my tests (connected using RMII phy mode to a Micrel KSZ8031RNL), the download rate was 20% to 25% slower if the extra 10ms delay was not present.

This patch actually requires four other lpc32xx patches that are not yet available in u-boot/master or u-boot-arm/master:
1) http://patchwork.ozlabs.org/patch/489100/
2) http://patchwork.ozlabs.org/patch/489190/
3) http://patchwork.ozlabs.org/patch/491419/
4) http://patchwork.ozlabs.org/patch/491420/

Signed-off-by: Sylvain Lemieux <slemieux@tycoint.com>
---
 drivers/net/lpc32xx_eth.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/lpc32xx_eth.c b/drivers/net/lpc32xx_eth.c
index bb8d1ec..207d721 100644
--- a/drivers/net/lpc32xx_eth.c
+++ b/drivers/net/lpc32xx_eth.c
@@ -634,6 +634,8 @@ int lpc32xx_eth_initialize(bd_t *bis)

        /* Release SOFT reset to let MII talk to PHY */
        clrbits_le32(&regs->mac1, MAC1_SOFT_RESET);
+       if (lpc32xx_eth.phy_rmii)
+               udelay(10000);

        /* register driver before talking to phy */
        eth_register(dev);
--
1.7.12.4


________________________________

This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you are not the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any action in respect of any information contained in it. If you have received this e-mail in error, please notify the sender immediately by e-mail and immediately destroy this e-mail and its attachments.

[U-Boot] [PATCH 1/1] net: lpc32xx: improve download rate in RMII phy mode

[Vladimir Zapolskiy]

Hi Sylvain,

On 07.07.2015 22:13, LEMIEUX, SYLVAIN wrote:
> This is a minor update to LPC32xx MAC driver patch that add support for the RMII phy mode.
> 
> In the legacy BSP from NXP, in RMII phy mode, an extra 10ms delay was present after the release of the soft reset.
> 
> In my tests (connected using RMII phy mode to a Micrel KSZ8031RNL), the download rate was 20% to 25% slower if the extra 10ms delay was not present.

I found some time to test this change, and actually on my environment I
don't observe any improvement, below are some details:

* tested on devkit3250 board with SMSC LAN8700 phy connected to LPC3250
MAC over RMII,
* to download files to RAM I used tftp protocol,
* file download is started immediately on boot, boot delay is set to 0,
bootcmd runs dhcp and then tftp,
* I downloaded files of 1 MiB and 48 MiB size, both cases produce the
same download speed of about 1.15 MiB/second, presence/absence of the
proposed 10ms delay in lpc32xx_eth_initialize() has no impact on
download speed,
* the speed is not limited by external network hardware or topology,
absolutely in the same environment an iMX6 board is capable to download
files over tftp at 3.5 - 4.0 MiB per second rate,
* 20% to 25% download speed improvement (1.15 MiB/s to expected ~1.4
MiB/s) is not observed after applying your proposed change, probably in
your environment this delay is needed due to your board, network
connection or PHY specifics, is Micrel KSZ8031RNL support comes from
legacy BSP?,
* I can not confirm that an additional delay on board/MAC init is needed
here regardless of its presence in legacy BSP since it defers board
initialization by 10ms with no visual download speed improvement, also
any possible relation between this delay and download speed is not obvious,
* download speed in my legacy BSP dated by 2010 is about 3-4 times
slower than in vanilla U-boot, unfortunately the exact figure is not
reported.

If the change has positive impact for you, I would propose to add a
delay in your board file, e.g. in board_early_init_f() after
lpc32xx_eth_initialize() call and retest, right at the moment I hesitate
to add a 10ms delay to _all_ users of LPC32xx MAC / RMII phy.

Also it would be nice to know the exact download speed on your end for
* current version of lpc32xx_eth.c with my recent patches applied,
* current version of lpc32xx_eth.c with my recent patches applied and
your change,
* legacy BSP.

A couple of general recommendations regarding posting changes:
* too long lines in commit message,
* it seems that your mail server screws up tab symbols from the mail
body, the patch can not be applied as is,
* also it seems that your mail server repacks messages, so that a
downloaded version of the sent patch is not a valid patch, from the mail
headers:

  Content-Transfer-Encoding: base64

> This patch actually requires four other lpc32xx patches that are not yet available in u-boot/master or u-boot-arm/master:
> 1) http://patchwork.ozlabs.org/patch/489100/
> 2) http://patchwork.ozlabs.org/patch/489190/
> 3) http://patchwork.ozlabs.org/patch/491419/
> 4) http://patchwork.ozlabs.org/patch/491420/
> 
> Signed-off-by: Sylvain Lemieux <slemieux@tycoint.com>
> ---
>  drivers/net/lpc32xx_eth.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/lpc32xx_eth.c b/drivers/net/lpc32xx_eth.c
> index bb8d1ec..207d721 100644
> --- a/drivers/net/lpc32xx_eth.c
> +++ b/drivers/net/lpc32xx_eth.c
> @@ -634,6 +634,8 @@ int lpc32xx_eth_initialize(bd_t *bis)
> 
>         /* Release SOFT reset to let MII talk to PHY */
>         clrbits_le32(&regs->mac1, MAC1_SOFT_RESET);
> +       if (lpc32xx_eth.phy_rmii)
> +               udelay(10000);
> 
>         /* register driver before talking to phy */
>         eth_register(dev);

--
With best wishes,
Vladimir

[U-Boot] [PATCH 1/1] net: lpc32xx: improve download rate in RMII phy mode

[LEMIEUX, SYLVAIN]

Hi Vladimir,

Thanks for taking the time to test this patch.

After further investigation, the issue was with our internal Micrel KSZ8031RNL phy implementation.
The extra 10ms delay is not require. This patch should be discard.

Thanks for the feedback regarding the mail system;
- for an internal mail address, the "tab" is not translated in "space".
- for an external e-mail, there is a translation that take place;
  we will look into the problem before sending the patch to support the Micrel KSZ8031RNL phy.


Sylvain Lemieux

-----Original Message-----
From: Vladimir Zapolskiy [mailto:vz at mleia.com]
Sent: 8-Jul-15 1:06 AM
To: LEMIEUX, SYLVAIN
Cc: joe.hershberger at ni.com; albert.aribaud at 3adev.fr; u-boot at lists.denx.de
Subject: Re: [PATCH 1/1] net: lpc32xx: improve download rate in RMII phy mode

Hi Sylvain,

On 07.07.2015 22:13, LEMIEUX, SYLVAIN wrote:
> This is a minor update to LPC32xx MAC driver patch that add support for the RMII phy mode.
>
> In the legacy BSP from NXP, in RMII phy mode, an extra 10ms delay was present after the release of the soft reset.
>
> In my tests (connected using RMII phy mode to a Micrel KSZ8031RNL), the download rate was 20% to 25% slower if the extra 10ms delay was not present.

I found some time to test this change, and actually on my environment I don't observe any improvement, below are some details:

* tested on devkit3250 board with SMSC LAN8700 phy connected to LPC3250 MAC over RMII,
* to download files to RAM I used tftp protocol,
* file download is started immediately on boot, boot delay is set to 0, bootcmd runs dhcp and then tftp,
* I downloaded files of 1 MiB and 48 MiB size, both cases produce the same download speed of about 1.15 MiB/second, presence/absence of the proposed 10ms delay in lpc32xx_eth_initialize() has no impact on download speed,
* the speed is not limited by external network hardware or topology, absolutely in the same environment an iMX6 board is capable to download files over tftp at 3.5 - 4.0 MiB per second rate,
* 20% to 25% download speed improvement (1.15 MiB/s to expected ~1.4
MiB/s) is not observed after applying your proposed change, probably in your environment this delay is needed due to your board, network connection or PHY specifics, is Micrel KSZ8031RNL support comes from legacy BSP?,
* I can not confirm that an additional delay on board/MAC init is needed here regardless of its presence in legacy BSP since it defers board initialization by 10ms with no visual download speed improvement, also any possible relation between this delay and download speed is not obvious,
* download speed in my legacy BSP dated by 2010 is about 3-4 times slower than in vanilla U-boot, unfortunately the exact figure is not reported.

If the change has positive impact for you, I would propose to add a delay in your board file, e.g. in board_early_init_f() after
lpc32xx_eth_initialize() call and retest, right at the moment I hesitate to add a 10ms delay to _all_ users of LPC32xx MAC / RMII phy.

Also it would be nice to know the exact download speed on your end for
* current version of lpc32xx_eth.c with my recent patches applied,
* current version of lpc32xx_eth.c with my recent patches applied and your change,
* legacy BSP.

A couple of general recommendations regarding posting changes:
* too long lines in commit message,
* it seems that your mail server screws up tab symbols from the mail body, the patch can not be applied as is,
* also it seems that your mail server repacks messages, so that a downloaded version of the sent patch is not a valid patch, from the mail
headers:

--
With best wishes,
Vladimir

________________________________

This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you are not the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any action in respect of any information contained in it. If you have received this e-mail in error, please notify the sender immediately by e-mail and immediately destroy this e-mail and its attachments.

[U-Boot] [PATCH 1/1] net: lpc32xx: improve download rate in RMII phy mode

[Albert ARIBAUD]

Hello SYLVAIN,

On Wed, 8 Jul 2015 17:41:54 +0000, LEMIEUX, SYLVAIN
<slemieux@Tycoint.com> wrote:
> Hi Vladimir,
> 
> Thanks for taking the time to test this patch.
> 
> After further investigation, the issue was with our internal Micrel KSZ8031RNL phy implementation.
> The extra 10ms delay is not require. This patch should be discard.

Ok--I've marked it "Rejected" in patchwork (and assigned it to myself
in case people later want to blame someone about the rejection).

Amicalement,
-- 
Albert.