Patchwork [U-Boot] ppc4xx/POST: Change ethernet test loop count from 192 to 16

login
register
mail settings
Submitter Stefan Roese
Date Nov. 26, 2010, 2:44 p.m.
Message ID <1290782654-9242-1-git-send-email-sr@denx.de>
Download mbox | patch
Permalink /patch/73186/
State Superseded
Headers show

Comments

Stefan Roese - Nov. 26, 2010, 2:44 p.m.
This patch changes the PPC4xx ethernet POST loop test count from
currently 192 (256 - 64) to 16 which should be enough. The main reason
for this is to reduce the boot time on boards using this POST test,
like the lwmon5 board. This change reduces the boot time by about
600ms on the lwmon5 board.

Signed-off-by: Stefan Roese <sr@denx.de>
---
 post/cpu/ppc4xx/ether.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Wolfgang Denk - Nov. 26, 2010, 3:13 p.m.
Dear Stefan Roese,

In message <1290782654-9242-1-git-send-email-sr@denx.de> you wrote:
> This patch changes the PPC4xx ethernet POST loop test count from
> currently 192 (256 - 64) to 16 which should be enough. The main reason
> for this is to reduce the boot time on boards using this POST test,
> like the lwmon5 board. This change reduces the boot time by about
> 600ms on the lwmon5 board.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> ---
>  post/cpu/ppc4xx/ether.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/post/cpu/ppc4xx/ether.c b/post/cpu/ppc4xx/ether.c
> index 7f44f38..1593a8d 100644
> --- a/post/cpu/ppc4xx/ether.c
> +++ b/post/cpu/ppc4xx/ether.c
> @@ -76,8 +76,8 @@ DECLARE_GLOBAL_DATA_PTR;
>  #define SDR0_MFR_ETH_CLK_SEL_V(n)	((0x01<<27) / (n+1))
>  #endif
>  
> -#define MIN_PACKET_LENGTH	64
> -#define MAX_PACKET_LENGTH	256
> +#define MIN_PACKET_LENGTH	256
> +#define MAX_PACKET_LENGTH	(256 + 16)

Maybe it does indeed make sense to test a wider range of package
sizes.  Actually I'd even like to see longer packets tested as well.

How about:

	#define MIN_PACKET_LENGTH    64
	#define MAX_PACKET_LENGTH    1518

and changing the

	for (l = MIN_PACKET_LENGTH; l <= MAX_PACKET_LENGTH; l++) {
into
	for (l = MIN_PACKET_LENGTH; l <= MAX_PACKET_LENGTH; l+=91) {

Then you still have 16 tests, but with a much wider range of packet
sizes (64...1429).

Best regards,

Wolfgang Denk
Stefan Roese - Nov. 26, 2010, 3:27 p.m.
Hi Wolfgang,

On Friday 26 November 2010 16:13:23 Wolfgang Denk wrote:
> > -#define MIN_PACKET_LENGTH	64
> > -#define MAX_PACKET_LENGTH	256
> > +#define MIN_PACKET_LENGTH	256
> > +#define MAX_PACKET_LENGTH	(256 + 16)
> 
> Maybe it does indeed make sense to test a wider range of package
> sizes.  Actually I'd even like to see longer packets tested as well.
> 
> How about:
> 
> 	#define MIN_PACKET_LENGTH    64
> 	#define MAX_PACKET_LENGTH    1518
> 
> and changing the
> 
> 	for (l = MIN_PACKET_LENGTH; l <= MAX_PACKET_LENGTH; l++) {
> into
> 	for (l = MIN_PACKET_LENGTH; l <= MAX_PACKET_LENGTH; l+=91) {
> 
> Then you still have 16 tests, but with a much wider range of packet
> sizes (64...1429).

I don't like this "l+=91" statement. How about making it a bit more flexible. 
Something like this:

#define MIN_PACKET_LENGTH		64
#define MAX_PACKET_LENGTH		1518
#ifndef CONFIG_SYS_POST_ETH_LOOPS
#define CONFIG_SYS_POST_ETH_LOOPS	10
#endif
#define PACKET_INCR	((MAX_PACKET_LENGTH - MIN_PACKET_LENGTH) / \
			 CONFIG_SYS_POST_ETH_LOOPS)

and

for (l = MIN_PACKET_LENGTH; l <= MAX_PACKET_LENGTH; l += PACKET_INCR) {


This way, boards could also override the default loop counter. I switched to a 
default of 10 this time. This still seems enough for me. Especially with the 
longer frames now.

What do you think?

Cheers,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de
Wolfgang Denk - Nov. 26, 2010, 3:37 p.m.
Dear Stefan Roese,

In message <201011261627.25617.sr@denx.de> you wrote:
> 
> I don't like this "l+=91" statement. How about making it a bit more flexible. 
> Something like this:
> 
> #define MIN_PACKET_LENGTH		64
> #define MAX_PACKET_LENGTH		1518
> #ifndef CONFIG_SYS_POST_ETH_LOOPS
> #define CONFIG_SYS_POST_ETH_LOOPS	10
> #endif
> #define PACKET_INCR	((MAX_PACKET_LENGTH - MIN_PACKET_LENGTH) / \
> 			 CONFIG_SYS_POST_ETH_LOOPS)

Agreed.  For the final version, please check how MAX_PACKET_LENGTH is
used (I didn't do this) and verify that 1518 is the correct value to
use here.

Thanks.

Best regards,

Wolfgang Denk

Patch

diff --git a/post/cpu/ppc4xx/ether.c b/post/cpu/ppc4xx/ether.c
index 7f44f38..1593a8d 100644
--- a/post/cpu/ppc4xx/ether.c
+++ b/post/cpu/ppc4xx/ether.c
@@ -76,8 +76,8 @@  DECLARE_GLOBAL_DATA_PTR;
 #define SDR0_MFR_ETH_CLK_SEL_V(n)	((0x01<<27) / (n+1))
 #endif
 
-#define MIN_PACKET_LENGTH	64
-#define MAX_PACKET_LENGTH	256
+#define MIN_PACKET_LENGTH	256
+#define MAX_PACKET_LENGTH	(256 + 16)
 #define TEST_NUM		1
 
 static volatile mal_desc_t tx __cacheline_aligned;