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

[Chris Moore <moore@free.fr>]

  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