[U-Boot] [PATCH] orion5x: optimize window size computation

[U-Boot] [PATCH] orion5x: optimize window size computation

[Albert Aribaud]

Signed-off-by: Chris Moore <moore@free.fr>
---

This is a simple optimization of the orion5x window size
computation. This code was contributed by Chris Moore so
I put his Signed-off-by rather than mine.

See <http://www.mail-archive.com/u-boot@lists.denx.de/msg37075.html>

 arch/arm/cpu/arm926ejs/orion5x/cpu.c |   30 ++++++++++++++++++++----------
 1 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/arch/arm/cpu/arm926ejs/orion5x/cpu.c b/arch/arm/cpu/arm926ejs/orion5x/cpu.c
index c36d7bf..5c443fe 100644
--- a/arch/arm/cpu/arm926ejs/orion5x/cpu.c
+++ b/arch/arm/cpu/arm926ejs/orion5x/cpu.c
@@ -48,24 +48,34 @@ void reset_cpu(unsigned long ignored)
 }
 
 /*
- * Window Size
+ * Compute Window Size field value from size expressed in bytes
  * Used with the Base register to set the address window size and location.
  * Must be programmed from LSB to MSB as sequence of ones followed by
  * sequence of zeros. The number of ones specifies the size of the window in
  * 64 KByte granularity (e.g., a value of 0x00FF specifies 256 = 16 MByte).
- * NOTE: A value of 0x0 specifies 64-KByte size.
+ * NOTES:
+ * 1) A sizeval equal to 0x0 specifies 4 TB.
+ * 2) A return value of 0x0 specifies 64 KB.
  */
 unsigned int orion5x_winctrl_calcsize(unsigned int sizeval)
 {
-	int i;
-	unsigned int j = 0;
-	u32 val = sizeval >> 1;
+	/*
+	 * Calculate the number of 64 KiB blocks needed minus one (rounding up).
+	 * For sizeval > 0 this is equivalent to:
+	 * sizeval = (u32) ceil((double) sizeval / 65536.0) - 1
+	 */
+	sizeval = (sizeval - 1) >> 16;
 
-	for (i = 0; val >= 0x10000; i++) {
-		j |= (1 << i);
-		val = val >> 1;
-	}
-	return 0x0000ffff & j;
+	/*
+	 * Propagate 'one' bits to the right by 'oring' them.
+	 * We need only treat bits 15-0.
+	 */
+	sizeval |= sizeval >> 1;  /* 'Or' bit 15 onto bit 14 */
+	sizeval |= sizeval >> 2;  /* 'Or' bits 15-14 onto bits 13-12 */
+	sizeval |= sizeval >> 4;  /* 'Or' bits 15-12 onto bits 11-8 */
+	sizeval |= sizeval >> 8;  /* 'Or' bits 15-8 onto bits 7-0*/
+
+	return sizeval;
 }
 
 /*
-- 
1.7.0.4

[U-Boot] [PATCH] orion5x: optimize window size computation

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de 
> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
> Sent: Tuesday, October 05, 2010 3:52 AM
> To: u-boot at lists.denx.de
> Subject: [U-Boot] [PATCH] orion5x: optimize window size computation
> 
> 
> Signed-off-by: Chris Moore <moore@free.fr>
> ---
> 
> This is a simple optimization of the orion5x window size
> computation. This code was contributed by Chris Moore so
> I put his Signed-off-by rather than mine.

This is wrong, you should be singed-off since you are posting and Chris can be contributor
Or let him post the patch.

> 
> See <http://www.mail-archive.com/u-boot@lists.denx.de/msg37075.html>
> 
>  arch/arm/cpu/arm926ejs/orion5x/cpu.c |   30 
> ++++++++++++++++++++----------
>  1 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/cpu/arm926ejs/orion5x/cpu.c 
> b/arch/arm/cpu/arm926ejs/orion5x/cpu.c
> index c36d7bf..5c443fe 100644
> --- a/arch/arm/cpu/arm926ejs/orion5x/cpu.c
> +++ b/arch/arm/cpu/arm926ejs/orion5x/cpu.c
> @@ -48,24 +48,34 @@ void reset_cpu(unsigned long ignored)
>  }
>  
>  /*
> - * Window Size
> + * Compute Window Size field value from size expressed in bytes
>   * Used with the Base register to set the address window 
> size and location.
>   * Must be programmed from LSB to MSB as sequence of ones followed by
>   * sequence of zeros. The number of ones specifies the size 
> of the window in
>   * 64 KByte granularity (e.g., a value of 0x00FF specifies 
> 256 = 16 MByte).
> - * NOTE: A value of 0x0 specifies 64-KByte size.
> + * NOTES:
> + * 1) A sizeval equal to 0x0 specifies 4 TB.
> + * 2) A return value of 0x0 specifies 64 KB.
>   */
>  unsigned int orion5x_winctrl_calcsize(unsigned int sizeval)
>  {
> -	int i;
> -	unsigned int j = 0;
> -	u32 val = sizeval >> 1;
> +	/*
> +	 * Calculate the number of 64 KiB blocks needed minus 
> one (rounding up).
> +	 * For sizeval > 0 this is equivalent to:
> +	 * sizeval = (u32) ceil((double) sizeval / 65536.0) - 1
> +	 */
> +	sizeval = (sizeval - 1) >> 16;
>  
> -	for (i = 0; val >= 0x10000; i++) {
> -		j |= (1 << i);
> -		val = val >> 1;
> -	}
> -	return 0x0000ffff & j;
> +	/*
> +	 * Propagate 'one' bits to the right by 'oring' them.
> +	 * We need only treat bits 15-0.
> +	 */
> +	sizeval |= sizeval >> 1;  /* 'Or' bit 15 onto bit 14 */
> +	sizeval |= sizeval >> 2;  /* 'Or' bits 15-14 onto bits 13-12 */
> +	sizeval |= sizeval >> 4;  /* 'Or' bits 15-12 onto bits 11-8 */
> +	sizeval |= sizeval >> 8;  /* 'Or' bits 15-8 onto bits 7-0*/
> +
> +	return sizeval;

BTW: How much this saves on size?

Regards..
Prafulla ..

[U-Boot] [PATCH] orion5x: optimize window size computation

[Chris Moore]

  Hi Prafulla,

Le 05/10/2010 07:57, Prafulla Wadaskar a ?crit :
>
>
>> -----Original Message-----
>> From: u-boot-bounces at lists.denx.de
>> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
>> Sent: Tuesday, October 05, 2010 3:52 AM
>> To: u-boot at lists.denx.de
>> Subject: [U-Boot] [PATCH] orion5x: optimize window size computation
>>
>>
>> Signed-off-by: Chris Moore<moore@free.fr>
>> ---
>>
>> This is a simple optimization of the orion5x window size
>> computation. This code was contributed by Chris Moore so
>> I put his Signed-off-by rather than mine.
> This is wrong, you should be singed-off since you are posting and Chris can be contributor
> Or let him post the patch.
>

I asked Albert to post the patch as I am mainly a U-Boot lurker and I 
have no U-Boot git tree.
If there is a problem I don't mind if he signs off either with or 
without me.

>> See<http://www.mail-archive.com/u-boot@lists.denx.de/msg37075.html>
>>
>>   arch/arm/cpu/arm926ejs/orion5x/cpu.c |   30
>> ++++++++++++++++++++----------
>>   1 files changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/cpu/arm926ejs/orion5x/cpu.c
>> b/arch/arm/cpu/arm926ejs/orion5x/cpu.c
>> index c36d7bf..5c443fe 100644
>> --- a/arch/arm/cpu/arm926ejs/orion5x/cpu.c
>> +++ b/arch/arm/cpu/arm926ejs/orion5x/cpu.c
>> @@ -48,24 +48,34 @@ void reset_cpu(unsigned long ignored)
>>   }
>>
>>   /*
>> - * Window Size
>> + * Compute Window Size field value from size expressed in bytes
>>    * Used with the Base register to set the address window
>> size and location.
>>    * Must be programmed from LSB to MSB as sequence of ones followed by
>>    * sequence of zeros. The number of ones specifies the size
>> of the window in
>>    * 64 KByte granularity (e.g., a value of 0x00FF specifies
>> 256 = 16 MByte).
>> - * NOTE: A value of 0x0 specifies 64-KByte size.
>> + * NOTES:
>> + * 1) A sizeval equal to 0x0 specifies 4 TB.
>> + * 2) A return value of 0x0 specifies 64 KB.
>>    */
>>   unsigned int orion5x_winctrl_calcsize(unsigned int sizeval)
>>   {
>> -	int i;
>> -	unsigned int j = 0;
>> -	u32 val = sizeval>>  1;
>> +	/*
>> +	 * Calculate the number of 64 KiB blocks needed minus
>> one (rounding up).
>> +	 * For sizeval>  0 this is equivalent to:
>> +	 * sizeval = (u32) ceil((double) sizeval / 65536.0) - 1
>> +	 */
>> +	sizeval = (sizeval - 1)>>  16;
>>
>> -	for (i = 0; val>= 0x10000; i++) {
>> -		j |= (1<<  i);
>> -		val = val>>  1;
>> -	}
>> -	return 0x0000ffff&  j;
>> +	/*
>> +	 * Propagate 'one' bits to the right by 'oring' them.
>> +	 * We need only treat bits 15-0.
>> +	 */
>> +	sizeval |= sizeval>>  1;  /* 'Or' bit 15 onto bit 14 */
>> +	sizeval |= sizeval>>  2;  /* 'Or' bits 15-14 onto bits 13-12 */
>> +	sizeval |= sizeval>>  4;  /* 'Or' bits 15-12 onto bits 11-8 */
>> +	sizeval |= sizeval>>  8;  /* 'Or' bits 15-8 onto bits 7-0*/
>> +
>> +	return sizeval;
> BTW: How much this saves on size?
>

It is not so much a question of size. I am afraid that the other version 
was just plain *wrong* :(
See my previous post here: 
http://www.mail-archive.com/u-boot at lists.denx.de/msg36996.html
In this post I presented two versions:
a) a corrected version along the lines of the original,
b) a version similar to the above.

The loop version may be slightly shorter in code size, particularly if 
one removes the unnecessary and with 0x0000ffff at the end.
But aesthetically I find the version above much more pleasing.
(Didn't Donald Knuth write "The *Art* of Computer Programming"?)
It is also much faster for large window sizes but this probably doesn't 
matter here.

Cheers,
Chris

[U-Boot] [PATCH] orion5x: optimize window size computation

[Albert ARIBAUD]

Le 05/10/2010 23:40, Chris Moore a ?crit :
> Hi Prafulla,
>
> Le 05/10/2010 07:57, Prafulla Wadaskar a ?crit :
>>
>>
>>> -----Original Message-----
>>> From: u-boot-bounces at lists.denx.de
>>> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
>>> Sent: Tuesday, October 05, 2010 3:52 AM
>>> To: u-boot at lists.denx.de
>>> Subject: [U-Boot] [PATCH] orion5x: optimize window size computation
>>>
>>>
>>> Signed-off-by: Chris Moore<moore@free.fr>
>>> ---
>>>
>>> This is a simple optimization of the orion5x window size
>>> computation. This code was contributed by Chris Moore so
>>> I put his Signed-off-by rather than mine.
>> This is wrong, you should be singed-off since you are posting and
>> Chris can be contributor
>> Or let him post the patch.
>
> I asked Albert to post the patch as I am mainly a U-Boot lurker and I
> have no U-Boot git tree.
> If there is a problem I don't mind if he signs off either with or
> without me.

Or how about a double Signed-off-by?

>> BTW: How much this saves on size?

More than 53% of the code size! That's 28 bytes instead of 60. :)

> It is not so much a question of size. I am afraid that the other version
> was just plain *wrong* :(

Indeed. Prafulla, you'll remember that there was a generally horribly 
wrong version before, which I'd fixed with what I thought was a correct 
version, and is for many sizes, principally those of the for 2**N, but 
not all. Chris' version fixes all cases.

> The loop version may be slightly shorter in code size, particularly if
> one removes the unnecessary and with 0x0000ffff at the end.

It's not: yours is shorter, thanks to the compiler being able to 
optimize things.

> But aesthetically I find the version above much more pleasing.
> (Didn't Donald Knuth write "The *Art* of Computer Programming"?)
> It is also much faster for large window sizes but this probably doesn't
> matter here.

Your code is 7 instructions flat whatever the size, whereas the loop has 
a fixed 9 instruction setup and exit penalty, plus 5 instructions per 
bit shift (plus one literal). So your code is *always* faster.

> Cheers,
> Chris

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] orion5x: optimize window size computation

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud at free.fr] 
> Sent: Wednesday, October 06, 2010 11:22 AM
> To: Chris Moore
> Cc: Prafulla Wadaskar; u-boot at lists.denx.de; Ashish Karkare; 
> Prabhanjan Sarnaik
> Subject: Re: [U-Boot] [PATCH] orion5x: optimize window size 
> computation
> 
> Le 05/10/2010 23:40, Chris Moore a ?crit :
> > Hi Prafulla,
> >
> > Le 05/10/2010 07:57, Prafulla Wadaskar a ?crit :
> >>
> >>
> >>> -----Original Message-----
> >>> From: u-boot-bounces at lists.denx.de
> >>> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
> >>> Sent: Tuesday, October 05, 2010 3:52 AM
> >>> To: u-boot at lists.denx.de
> >>> Subject: [U-Boot] [PATCH] orion5x: optimize window size 
> computation
> >>>
> >>>
> >>> Signed-off-by: Chris Moore<moore@free.fr>
> >>> ---
> >>>
> >>> This is a simple optimization of the orion5x window size
> >>> computation. This code was contributed by Chris Moore so
> >>> I put his Signed-off-by rather than mine.
> >> This is wrong, you should be singed-off since you are posting and
> >> Chris can be contributor
> >> Or let him post the patch.

Ack

> >
> > I asked Albert to post the patch as I am mainly a U-Boot 
> lurker and I
> > have no U-Boot git tree.
> > If there is a problem I don't mind if he signs off either with or
> > without me.
> 
> Or how about a double Signed-off-by?

BTW: double Signed-off-by is not a bad idea, lot other patches follows the same.
I wouldn't mind to ack such patches.
May be Wolfgang can better comment on this.

> 
> >> BTW: How much this saves on size?
> 
> More than 53% of the code size! That's 28 bytes instead of 60. :)

Great!! Thanks Chris, we will update the same for Kirkwood too.

Regards..
Prafulla . .

[U-Boot] Mvbge driver broken on kirkwood platforms after ARM relocation

[Prafulla Wadaskar]

Hi Albert

After rebasing to new ARM relocation code base and updating Kirkwood board support.
I am unable to get my network driver through (mvgbe)

Have you tested this on edminv2 platform?
If it is working at your end? Can you please cross check the same with Kirkwood platform?

Regards..
Prafulla . .

[U-Boot] [PATCH] orion5x: optimize window size computation

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <F766E4F80769BD478052FB6533FA745D19A69E25C7@SC-VEXCH4.marvell.com> you wrote:
> 
> > Or how about a double Signed-off-by?
> 
> BTW: double Signed-off-by is not a bad idea, lot other patches follows the =
> same.
> I wouldn't mind to ack such patches.
> May be Wolfgang can better comment on this.

Yes, double SoB is OK here. [Too many SoBs never hurt; it's too few
that are bad.]

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 at denx.de
We Klingons believe as you do -- the sick should die. Only the strong
should live.
	-- Kras, "Friday's Child", stardate 3497.2

[U-Boot] Mvbge driver broken on kirkwood platforms after ARM relocation

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <F766E4F80769BD478052FB6533FA745D19A69E25C8@SC-VEXCH4.marvell.com> you wrote:
> 
> After rebasing to new ARM relocation code base and updating Kirkwood board support.
> I am unable to get my network driver through (mvgbe)
> 
> Have you tested this on edminv2 platform?
> If it is working at your end? Can you please cross check the same with Kirkwood platform?

Try running the "dcache off" command before accessing the network and
see if this changes anything.

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 at denx.de
Extended Epstein-Heisenberg Principle: In an R & D orbit, only  2  of
the  existing 3 parameters can be defined simultaneously. The parame-
ters are: task, time and resources ($).

[U-Boot] [PATCH] orion5x: optimize window size computation

[Albert ARIBAUD]

Le 06/10/2010 15:29, Wolfgang Denk a ?crit :
> Dear Prafulla Wadaskar,
>
> In message<F766E4F80769BD478052FB6533FA745D19A69E25C7@SC-VEXCH4.marvell.com>  you wrote:
>>
>>> Or how about a double Signed-off-by?
>>
>> BTW: double Signed-off-by is not a bad idea, lot other patches follows the =
>> same.
>> I wouldn't mind to ack such patches.
>> May be Wolfgang can better comment on this.
>
> Yes, double SoB is OK here. [Too many SoBs never hurt; it's too few
> that are bad.]

Thanks. Do I need to repost this patch with a double SoB?

> Best regards,
>
> Wolfgang Denk

Amicalement,
-- 
Albert.

[U-Boot] Mvbge driver broken on kirkwood platforms after ARM relocation

[Albert ARIBAUD]

Le 06/10/2010 15:30, Wolfgang Denk a ?crit :
> Dear Prafulla Wadaskar,
>
> In message<F766E4F80769BD478052FB6533FA745D19A69E25C8@SC-VEXCH4.marvell.com>  you wrote:
>>
>> After rebasing to new ARM relocation code base and updating Kirkwood board support.
>> I am unable to get my network driver through (mvgbe)
>>
>> Have you tested this on edminv2 platform?
>> If it is working at your end? Can you please cross check the same with Kirkwood platform?

I am positive that mvgbe driver works, because it initially did not on 
my edminiv2, and I traced the root cause to gd being trashed between 
board_init_f and board_init_r; that was fixed before I submitted this patch.

I will test on the openrd platform.

> Try running the "dcache off" command before accessing the network and
> see if this changes anything.

Good suggestion. FTR, I did all of my tests with dcache off only -- 
never tried to activate any cache.

> Best regards,
>
> Wolfgang Denk

Amicalement,
-- 
Albert.

[U-Boot] Mvbge driver broken on kirkwood platforms after ARM relocation

[Albert ARIBAUD]

Le 06/10/2010 15:54, Albert ARIBAUD a ?crit :

>>> Have you tested this on edminv2 platform?
>>> If it is working at your end? Can you please cross check the same with Kirkwood platform?
>
> I am positive that mvgbe driver works, because it initially did not on
> my edminiv2, and I traced the root cause to gd being trashed between
> board_init_f and board_init_r; that was fixed before I submitted this patch.
>
> I will test on the openrd platform.

BTW: Prafulla, what toolchain are you using for your kirkwood platforms?

Amicalement,
-- 
Albert.

[U-Boot] Mvbge driver broken on kirkwood platforms after ARM relocation

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de 
> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert ARIBAUD
> Sent: Wednesday, October 06, 2010 7:26 PM
> To: Albert ARIBAUD
> Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik
> Subject: Re: [U-Boot] Mvbge driver broken on kirkwood 
> platforms after ARM relocation
> 
> Le 06/10/2010 15:54, Albert ARIBAUD a ?crit :
> 
> >>> Have you tested this on edminv2 platform?
> >>> If it is working at your end? Can you please cross check 
> the same with Kirkwood platform?
> >
> > I am positive that mvgbe driver works, because it initially 
> did not on
> > my edminiv2, and I traced the root cause to gd being trashed between
> > board_init_f and board_init_r; that was fixed before I 
> submitted this patch.
> >
> > I will test on the openrd platform.
> 
> BTW: Prafulla, what toolchain are you using for your kirkwood 
> platforms?

Hi Albert

I tried with Gcc ver 4.1.2 (Red Hat 4.1.2-33.fa1) and gcc ver 4.4.1 (Sourcery G++ Lite 2009q3-68)
In both the cases it fails.

Further I debugged the issue and found that- it fails in drivers/net/mvgbe.c at mvgbe_send() in while loop.
It is clear that the Scheduled tx DMA xter is not getting completed, nor reporting error.

Then I checked with other board having switch instead of phy, and it works !!.

Whereas any release earlier to ARM relocation patches works very well (i.e. diff against u-boot-marvell.git/master and u-boot-arm.git)

So I am unable to figure out whom to address?

For me this happened since I moved to new ARM relocation code base. So initial doubt goes there.

If you have openrd_base you can give a try

Regards..
Prafulla . .

> 
> Amicalement,
> -- 
> Albert.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 

[U-Boot] Mvbge driver broken on kirkwood platforms after ARM relocation

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de] 
> Sent: Wednesday, October 06, 2010 7:00 PM
> To: Prafulla Wadaskar
> Cc: Albert ARIBAUD; u-boot at lists.denx.de; Ashish Karkare; 
> Prabhanjan Sarnaik
> Subject: Re: [U-Boot] Mvbge driver broken on kirkwood 
> platforms after ARM relocation
> 
> Dear Prafulla Wadaskar,
> 
> In message 
> <F766E4F80769BD478052FB6533FA745D19A69E25C8@SC-VEXCH4.marvell.
> com> you wrote:
> > 
> > After rebasing to new ARM relocation code base and updating 
> Kirkwood board support.
> > I am unable to get my network driver through (mvgbe)
> > 
> > Have you tested this on edminv2 platform?
> > If it is working at your end? Can you please cross check 
> the same with Kirkwood platform?
> 
> Try running the "dcache off" command before accessing the network and
> see if this changes anything.

I tried this too, I have disabled dcache in init.
.. no difference.

Debugging continued..

Regards..
Prafulla . .

[U-Boot] [PATCH] orion5x: optimize window size computation

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud at free.fr] 
> Sent: Wednesday, October 06, 2010 7:17 PM
> To: Wolfgang Denk
> Cc: Prafulla Wadaskar; u-boot at lists.denx.de; Ashish Karkare; 
> Prabhanjan Sarnaik
> Subject: Re: [PATCH] orion5x: optimize window size computation
> 
> Le 06/10/2010 15:29, Wolfgang Denk a ?crit :
> > Dear Prafulla Wadaskar,
> >
> > In 
> message<F766E4F80769BD478052FB6533FA745D19A69E25C7@SC-VEXCH4.m
> arvell.com>  you wrote:
> >>
> >>> Or how about a double Signed-off-by?
> >>
> >> BTW: double Signed-off-by is not a bad idea, lot other 
> patches follows the =
> >> same.
> >> I wouldn't mind to ack such patches.
> >> May be Wolfgang can better comment on this.
> >
> > Yes, double SoB is OK here. [Too many SoBs never hurt; it's too few
> > that are bad.]
> 
> Thanks. Do I need to repost this patch with a double SoB?

Yes, that will be good

Regards..
Prafulla . .

[U-Boot] Mvbge driver broken on kirkwood platforms after ARM relocation

[Albert ARIBAUD]

Le 06/10/2010 16:14, Prafulla Wadaskar a ?crit :
>
>
>> -----Original Message-----
>> From: u-boot-bounces at lists.denx.de
>> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert ARIBAUD
>> Sent: Wednesday, October 06, 2010 7:26 PM
>> To: Albert ARIBAUD
>> Cc: u-boot at lists.denx.de; Ashish Karkare; Prabhanjan Sarnaik
>> Subject: Re: [U-Boot] Mvbge driver broken on kirkwood
>> platforms after ARM relocation
>>
>> Le 06/10/2010 15:54, Albert ARIBAUD a ?crit :
>>
>>>>> Have you tested this on edminv2 platform?
>>>>> If it is working at your end? Can you please cross check
>> the same with Kirkwood platform?
>>>
>>> I am positive that mvgbe driver works, because it initially
>> did not on
>>> my edminiv2, and I traced the root cause to gd being trashed between
>>> board_init_f and board_init_r; that was fixed before I
>> submitted this patch.
>>>
>>> I will test on the openrd platform.
>>
>> BTW: Prafulla, what toolchain are you using for your kirkwood
>> platforms?
>
> Hi Albert
>
> I tried with Gcc ver 4.1.2 (Red Hat 4.1.2-33.fa1) and gcc ver 4.4.1 (Sourcery G++ Lite 2009q3-68)
> In both the cases it fails.
>
> Further I debugged the issue and found that- it fails in drivers/net/mvgbe.c at mvgbe_send() in while loop.
> It is clear that the Scheduled tx DMA xter is not getting completed, nor reporting error.
>
> Then I checked with other board having switch instead of phy, and it works !!.
>
> Whereas any release earlier to ARM relocation patches works very well (i.e. diff against u-boot-marvell.git/master and u-boot-arm.git)
>
> So I am unable to figure out whom to address?
>
> For me this happened since I moved to new ARM relocation code base. So initial doubt goes there.
>
> If you have openrd_base you can give a try
>
> Regards..
> Prafulla . .

This is suspiciously similar to the symptoms I had, which were: the loop 
checking for end of TX would never exit, because the TX descriptor's 
state word would never reflect the ownership change from DMA back to MPU 
that should happen at end of transfer, because the controllers BAR that 
was assumed to give it access to DRAM was trashed, because gd was 
trashed in the first place.

OTOH, if the same code works or not depending on whether you have a 
switch or phy, it rules out gd trashing.

Still... Please check the BARs in the controller.

I'll try on my openrd board. It has a phy, so I should be able to 
witness the issue.

Amicalement,
-- 
Albert.

[U-Boot] Mvbge driver broken on kirkwood platforms after ARM relocation

[Albert ARIBAUD]

Le 06/10/2010 16:22, Prafulla Wadaskar a ?crit :
>
>
>> -----Original Message-----
>> From: Wolfgang Denk [mailto:wd at denx.de]
>> Sent: Wednesday, October 06, 2010 7:00 PM
>> To: Prafulla Wadaskar
>> Cc: Albert ARIBAUD; u-boot at lists.denx.de; Ashish Karkare;
>> Prabhanjan Sarnaik
>> Subject: Re: [U-Boot] Mvbge driver broken on kirkwood
>> platforms after ARM relocation
>>
>> Dear Prafulla Wadaskar,
>>
>> In message
>> <F766E4F80769BD478052FB6533FA745D19A69E25C8@SC-VEXCH4.marvell.
>> com>  you wrote:
>>>
>>> After rebasing to new ARM relocation code base and updating
>> Kirkwood board support.
>>> I am unable to get my network driver through (mvgbe)
>>>
>>> Have you tested this on edminv2 platform?
>>> If it is working at your end? Can you please cross check
>> the same with Kirkwood platform?
>>
>> Try running the "dcache off" command before accessing the network and
>> see if this changes anything.
>
> I tried this too, I have disabled dcache in init.
> .. no difference.
>
> Debugging continued..
>
> Regards..
> Prafulla . .

Trying this on an openrd client board with the openrd_base config. Both 
boards have the same exact RAMs, however the DRAM: line is acting funny 
on me: fresh with my relocation patch above master, it says:

	SoC:   Kirkwood 88F6281_A0
	DRAM:  192.5 MiB

... while the actual RAM size is 512 MiB.

(Even considering that the original Marvell code may have the 
count-only-half bug that Chris' patch fixes, that's only 385 MiB... 
Weirder yet: adding Chris' patch above mine, I get 3.6 GiB... But let's 
chase only one rabbit at a time)

Prafulla, how much RAM does your build see on your board(s)?

Amicalement,
-- 
Albert.

[U-Boot] Mvbge driver broken on kirkwood platforms after ARM relocation

[Albert ARIBAUD]

Le 06/10/2010 17:56, Albert ARIBAUD a ?crit :
> Le 06/10/2010 16:22, Prafulla Wadaskar a ?crit :
>>
>>
>>> -----Original Message-----
>>> From: Wolfgang Denk [mailto:wd at denx.de]
>>> Sent: Wednesday, October 06, 2010 7:00 PM
>>> To: Prafulla Wadaskar
>>> Cc: Albert ARIBAUD; u-boot at lists.denx.de; Ashish Karkare;
>>> Prabhanjan Sarnaik
>>> Subject: Re: [U-Boot] Mvbge driver broken on kirkwood
>>> platforms after ARM relocation
>>>
>>> Dear Prafulla Wadaskar,
>>>
>>> In message
>>> <F766E4F80769BD478052FB6533FA745D19A69E25C8@SC-VEXCH4.marvell.
>>> com>   you wrote:
>>>>
>>>> After rebasing to new ARM relocation code base and updating
>>> Kirkwood board support.
>>>> I am unable to get my network driver through (mvgbe)
>>>>
>>>> Have you tested this on edminv2 platform?
>>>> If it is working at your end? Can you please cross check
>>> the same with Kirkwood platform?
>>>
>>> Try running the "dcache off" command before accessing the network and
>>> see if this changes anything.
>>
>> I tried this too, I have disabled dcache in init.
>> .. no difference.
>>
>> Debugging continued..
>>
>> Regards..
>> Prafulla . .
>
> Trying this on an openrd client board with the openrd_base config. Both
> boards have the same exact RAMs, however the DRAM: line is acting funny
> on me: fresh with my relocation patch above master, it says:
>
> 	SoC:   Kirkwood 88F6281_A0
> 	DRAM:  192.5 MiB
>
> ... while the actual RAM size is 512 MiB.
>
> (Even considering that the original Marvell code may have the
> count-only-half bug that Chris' patch fixes, that's only 385 MiB...
> Weirder yet: adding Chris' patch above mine, I get 3.6 GiB... But let's
> chase only one rabbit at a time)
>
> Prafulla, how much RAM does your build see on your board(s)?

globally defining DEBUG gives:

U-Boot 2010.09-00102-gb4a7c1c-dirty (Oct 06 2010 - 19:13:59)
OpenRD_base

	U-Boot code: 00600000 -> 0065DFBC  BSS: -> 006A46E0
	SoC:   Kirkwood 88F6281_A0
	monitor len: 000A46E0
	ramsize: 00000000

Obviously RAM detection is bugged. This explains the following computations:

	TLB table at: ffff0000
	Top of RAM usable for U-Boot at: ffff0000
	Reserving 657k for U-Boot at: fff4b000
	Reserving 1152k for malloc() at: ffe2b000
	Reserving 48 Bytes for Board Info at: ffe2afd0
	Reserving 92 Bytes for Global Data at: ffe2af74
	New Stack Pointer is: ffe2af70


	RAM Configuration:
	Bank #0: 00000000 0 Bytes
	Bank #1: 00000000 0 Bytes
	Bank #2: 00000000 0 Bytes
	Bank #3: 00000000 3.3 GiB

This weird bank #3 one may be a consequence, or not, of the buggy RAM 
computation.

	relocation Offset is: ff94b000

Further debugging going on.

Amicalement,
-- 
Albert.

[U-Boot] Mvbge driver broken on kirkwood platforms after ARM relocation

[Albert ARIBAUD]

Le 06/10/2010 19:36, Albert ARIBAUD a ?crit :
> Le 06/10/2010 17:56, Albert ARIBAUD a ?crit :
>> Le 06/10/2010 16:22, Prafulla Wadaskar a ?crit :
>>>
>>>
>>>> -----Original Message-----
>>>> From: Wolfgang Denk [mailto:wd at denx.de]
>>>> Sent: Wednesday, October 06, 2010 7:00 PM
>>>> To: Prafulla Wadaskar
>>>> Cc: Albert ARIBAUD; u-boot at lists.denx.de; Ashish Karkare;
>>>> Prabhanjan Sarnaik
>>>> Subject: Re: [U-Boot] Mvbge driver broken on kirkwood
>>>> platforms after ARM relocation
>>>>
>>>> Dear Prafulla Wadaskar,
>>>>
>>>> In message
>>>> <F766E4F80769BD478052FB6533FA745D19A69E25C8@SC-VEXCH4.marvell.
>>>> com> you wrote:
>>>>>
>>>>> After rebasing to new ARM relocation code base and updating
>>>> Kirkwood board support.
>>>>> I am unable to get my network driver through (mvgbe)
>>>>>
>>>>> Have you tested this on edminv2 platform?
>>>>> If it is working at your end? Can you please cross check
>>>> the same with Kirkwood platform?
>>>>
>>>> Try running the "dcache off" command before accessing the network and
>>>> see if this changes anything.
>>>
>>> I tried this too, I have disabled dcache in init.
>>> .. no difference.
>>>
>>> Debugging continued..
>>>
>>> Regards..
>>> Prafulla . .
>>
>> Trying this on an openrd client board with the openrd_base config. Both
>> boards have the same exact RAMs, however the DRAM: line is acting funny
>> on me: fresh with my relocation patch above master, it says:
>>
>> SoC: Kirkwood 88F6281_A0
>> DRAM: 192.5 MiB
>>
>> ... while the actual RAM size is 512 MiB.
>>
>> (Even considering that the original Marvell code may have the
>> count-only-half bug that Chris' patch fixes, that's only 385 MiB...
>> Weirder yet: adding Chris' patch above mine, I get 3.6 GiB... But let's
>> chase only one rabbit at a time)
>>
>> Prafulla, how much RAM does your build see on your board(s)?
>
> globally defining DEBUG gives:
>
> U-Boot 2010.09-00102-gb4a7c1c-dirty (Oct 06 2010 - 19:13:59)
> OpenRD_base
>
> U-Boot code: 00600000 -> 0065DFBC BSS: -> 006A46E0
> SoC: Kirkwood 88F6281_A0
> monitor len: 000A46E0
> ramsize: 00000000
>
> Obviously RAM detection is bugged. This explains the following
> computations:
>
> TLB table at: ffff0000
> Top of RAM usable for U-Boot at: ffff0000
> Reserving 657k for U-Boot at: fff4b000
> Reserving 1152k for malloc() at: ffe2b000
> Reserving 48 Bytes for Board Info at: ffe2afd0
> Reserving 92 Bytes for Global Data at: ffe2af74
> New Stack Pointer is: ffe2af70
>
>
> RAM Configuration:
> Bank #0: 00000000 0 Bytes
> Bank #1: 00000000 0 Bytes
> Bank #2: 00000000 0 Bytes
> Bank #3: 00000000 3.3 GiB
>
> This weird bank #3 one may be a consequence, or not, of the buggy RAM
> computation.
>
> relocation Offset is: ff94b000
>
> Further debugging going on.

I think I got it.

You must define dram_init() and dram_init_banksize() in your board code 
(or in the SoC code if you can factorize it). Without it, default 
functions get called, and that does not work out well.

You can probably define dram_init() and dram_init_banksize() in the 
kirkwood SoC support code, like it is for orion5x.

Amicalement,
-- 
Albert.

[U-Boot] Mvbge driver broken on kirkwood platforms after ARM relocation

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud at free.fr] 
> Sent: Wednesday, October 06, 2010 11:24 PM
> To: Albert ARIBAUD
> Cc: Prafulla Wadaskar; u-boot at lists.denx.de; Ashish Karkare; 
> Prabhanjan at theia.denx.de; Prabhanjan Sarnaik
> Subject: Re: Mvbge driver broken on kirkwood platforms after 
> ARM relocation
> 
> Le 06/10/2010 19:36, Albert ARIBAUD a ?crit :
> > Le 06/10/2010 17:56, Albert ARIBAUD a ?crit :
> >> Le 06/10/2010 16:22, Prafulla Wadaskar a ?crit :
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Wolfgang Denk [mailto:wd at denx.de]
> >>>> Sent: Wednesday, October 06, 2010 7:00 PM
> >>>> To: Prafulla Wadaskar
> >>>> Cc: Albert ARIBAUD; u-boot at lists.denx.de; Ashish Karkare;
> >>>> Prabhanjan Sarnaik
> >>>> Subject: Re: [U-Boot] Mvbge driver broken on kirkwood
> >>>> platforms after ARM relocation
> >>>>
> >>>> Dear Prafulla Wadaskar,
> >>>>
> >>>> In message
> >>>> <F766E4F80769BD478052FB6533FA745D19A69E25C8@SC-VEXCH4.marvell.
> >>>> com> you wrote:
> >>>>>
> >>>>> After rebasing to new ARM relocation code base and updating
> >>>> Kirkwood board support.
> >>>>> I am unable to get my network driver through (mvgbe)
> >>>>>
> >>>>> Have you tested this on edminv2 platform?
> >>>>> If it is working at your end? Can you please cross check
> >>>> the same with Kirkwood platform?
> >>>>
> >>>> Try running the "dcache off" command before accessing 
> the network and
> >>>> see if this changes anything.
> >>>
> >>> I tried this too, I have disabled dcache in init.
> >>> .. no difference.
> >>>
> >>> Debugging continued..
> >>>
> >>> Regards..
> >>> Prafulla . .
> >>
> >> Trying this on an openrd client board with the openrd_base 
> config. Both
> >> boards have the same exact RAMs, however the DRAM: line is 
> acting funny
> >> on me: fresh with my relocation patch above master, it says:
> >>
> >> SoC: Kirkwood 88F6281_A0
> >> DRAM: 192.5 MiB
> >>
> >> ... while the actual RAM size is 512 MiB.
> >>
> >> (Even considering that the original Marvell code may have the
> >> count-only-half bug that Chris' patch fixes, that's only 385 MiB...
> >> Weirder yet: adding Chris' patch above mine, I get 3.6 
> GiB... But let's
> >> chase only one rabbit at a time)
> >>
> >> Prafulla, how much RAM does your build see on your board(s)?
> >
> > globally defining DEBUG gives:
> >
> > U-Boot 2010.09-00102-gb4a7c1c-dirty (Oct 06 2010 - 19:13:59)
> > OpenRD_base
> >
> > U-Boot code: 00600000 -> 0065DFBC BSS: -> 006A46E0
> > SoC: Kirkwood 88F6281_A0
> > monitor len: 000A46E0
> > ramsize: 00000000
> >
> > Obviously RAM detection is bugged. This explains the following
> > computations:
> >
> > TLB table at: ffff0000
> > Top of RAM usable for U-Boot at: ffff0000
> > Reserving 657k for U-Boot at: fff4b000
> > Reserving 1152k for malloc() at: ffe2b000
> > Reserving 48 Bytes for Board Info at: ffe2afd0
> > Reserving 92 Bytes for Global Data at: ffe2af74
> > New Stack Pointer is: ffe2af70
> >
> >
> > RAM Configuration:
> > Bank #0: 00000000 0 Bytes
> > Bank #1: 00000000 0 Bytes
> > Bank #2: 00000000 0 Bytes
> > Bank #3: 00000000 3.3 GiB
> >
> > This weird bank #3 one may be a consequence, or not, of the 
> buggy RAM
> > computation.
> >
> > relocation Offset is: ff94b000
> >
> > Further debugging going on.
> 
> I think I got it.
> 
> You must define dram_init() and dram_init_banksize() in your 
> board code 
> (or in the SoC code if you can factorize it). Without it, default 
> functions get called, and that does not work out well.
> 
> You can probably define dram_init() and dram_init_banksize() in the 
> kirkwood SoC support code, like it is for orion5x.

Hi Albert

You should apply these patches to get build errors and dram size fixed
http://lists.denx.de/pipermail/u-boot/2010-September/078069.html
http://lists.denx.de/pipermail/u-boot/2010-September/078071.html
http://lists.denx.de/pipermail/u-boot/2010-September/078070.html

Regards..
Prafulla . .

> 
> Amicalement,
> -- 
> Albert.
> 

[U-Boot] Mvbge driver broken on kirkwood platforms after ARM relocation

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.aribaud at free.fr] 
> Sent: Wednesday, October 06, 2010 11:24 PM
> To: Albert ARIBAUD
> Cc: Prafulla Wadaskar; u-boot at lists.denx.de; Ashish Karkare; 
> Prabhanjan at theia.denx.de; Prabhanjan Sarnaik
> Subject: Re: Mvbge driver broken on kirkwood platforms after 
> ARM relocation
>

Snip...

This is resolved now.
 dram_init_banksize() is need to be defined, if not done then it alters pre-configured dram bank0 by dram_init

Regards..
Prafulla . .