[U-Boot] [PATCH] Orion5x: bugfix: window size (mis)calculation
Chris Moore
moore at free.fr
Fri Aug 27 07:00:02 CEST 2010
Hi list,
Le 24/08/2010 15:27, Albert Aribaud a écrit :
> Fix orion5x_winctrl_calcsize() off-by-1 bug which caused mapping
> windows to be cut by half. This afected all windows including NOR
> flash (causing half the flash to be unaccessible) but DRAM was and
> still is fine as its size is determined otherwise.
>
> Signed-off-by: Albert Aribaud<albert.aribaud at free.fr>
> ---
> arch/arm/cpu/arm926ejs/orion5x/cpu.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/cpu/arm926ejs/orion5x/cpu.c b/arch/arm/cpu/arm926ejs/orion5x/cpu.c
> index 3740e33..260f88b 100644
> --- a/arch/arm/cpu/arm926ejs/orion5x/cpu.c
> +++ b/arch/arm/cpu/arm926ejs/orion5x/cpu.c
> @@ -61,7 +61,7 @@ unsigned int orion5x_winctrl_calcsize(unsigned int sizeval)
> unsigned int j = 0;
> u32 val = sizeval>> 1;
>
> - for (i = 0; val> 0x10000; i++) {
> + for (i = 0; val>= 0x10000; i++) {
> j |= (1<< i);
> val = val>> 1;
> }
Sorry for this late reply.
Both versions looked wrong to me but I couldn't test them at the time.
Albert's version gives the correct result for exact powers of 2.
This is admittedly the usual case.
However AFAICS it gives a wrong result for *all* other values above 64 KiB.
The following version gives the results I would expect:
unsigned int orion5x_winctrl_calcsize(unsigned int sizeval)
{
int i;
unsigned int j = 0;
u32 val = sizeval - 1;
for (i = 0; val >= 0x10000; i++) {
j |= (1 << i);
val = val >> 1;
}
return 0x0000ffff & j;
}
Notes:
a) Masking with 0x0000ffff is not necessary.
I kept it to keep the code as close as possible to the original.
b) A size of 0 on entry is stupid and must be considered as a special case.
The most useful is probably to treat 0 as 4 GiB.
This occurs naturally and needs no special handling.
Otherwise we could treat it as one 64 KiB page.
Otherwise we could BUG_ON.
However I have completely rewritten the function using a more efficient
algorithm.
It gives identical results to the previous one above.
For large window sizes this version is much faster.
It avoids the loop which repeatedly divides the size by 2.
u32 orion5x_winctrl_calcsize(u32 x)
{
/*
* Step 1: Get the most significant one in the right bit position.
*
* Calculate the number of 64 KiB blocks needed minus one (rounding up).
* It is equivalent to x = (u32) ceil((double) x / 65536.0) - 1
*/
x = (x - 1) >> 16;
/*
* Step 2: Copy the most significant one (MSO) into all bits to its
right.
*
* At this point the right shift above ensures that the 16 MSB of x
are zero.
* Propagate the MSO to all bits to its right (up to 15 bits).
*/
x |= x >> 1; /* Set the bit to the right of the MSO */
x |= x >> 2; /* Set the next 2 bits to the right of the MSO */
x |= x >> 4; /* Set the next 4 bits to the right of the MSO */
x |= x >> 8; /* Set the next 8 bits to the right of the MS0 */
return x;
}
Here are my test results for all four versions:
For range 0x00000000 to 0x00020001 the original orion5x_winctrl_calcsize
returns 0x0
For range 0x00020002 to 0x00040003 the original orion5x_winctrl_calcsize
returns 0x1
For range 0x00040004 to 0x00080007 the original orion5x_winctrl_calcsize
returns 0x3
For range 0x00080008 to 0x0010000f the original orion5x_winctrl_calcsize
returns 0x7
For range 0x00100010 to 0x0020001f the original orion5x_winctrl_calcsize
returns 0xf
For range 0x00200020 to 0x0040003f the original orion5x_winctrl_calcsize
returns 0x1f
For range 0x00400040 to 0x0080007f the original orion5x_winctrl_calcsize
returns 0x3f
For range 0x00800080 to 0x010000ff the original orion5x_winctrl_calcsize
returns 0x7f
For range 0x01000100 to 0x020001ff the original orion5x_winctrl_calcsize
returns 0xff
For range 0x02000200 to 0x040003ff the original orion5x_winctrl_calcsize
returns 0x1ff
For range 0x04000400 to 0x080007ff the original orion5x_winctrl_calcsize
returns 0x3ff
For range 0x08000800 to 0x10000fff the original orion5x_winctrl_calcsize
returns 0x7ff
For range 0x10001000 to 0x20001fff the original orion5x_winctrl_calcsize
returns 0xfff
For range 0x20002000 to 0x40003fff the original orion5x_winctrl_calcsize
returns 0x1fff
For range 0x40004000 to 0x80007fff the original orion5x_winctrl_calcsize
returns 0x3fff
For range 0x80008000 to 0xffffffff the original orion5x_winctrl_calcsize
returns 0x7fff
For range 0x00000000 to 0x0001ffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x0
For range 0x00020000 to 0x0003ffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x1
For range 0x00040000 to 0x0007ffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x3
For range 0x00080000 to 0x000fffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x7
For range 0x00100000 to 0x001fffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0xf
For range 0x00200000 to 0x003fffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x1f
For range 0x00400000 to 0x007fffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x3f
For range 0x00800000 to 0x00ffffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x7f
For range 0x01000000 to 0x01ffffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0xff
For range 0x02000000 to 0x03ffffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x1ff
For range 0x04000000 to 0x07ffffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x3ff
For range 0x08000000 to 0x0fffffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x7ff
For range 0x10000000 to 0x1fffffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0xfff
For range 0x20000000 to 0x3fffffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x1fff
For range 0x40000000 to 0x7fffffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x3fff
For range 0x80000000 to 0xffffffff Albert Aribaud's
orion5x_winctrl_calcsize returns 0x7fff
For range 0x00000000 to 0x00000000 Chris Moore's
orion5x_winctrl_calcsize returns 0xffff
For range 0x00000001 to 0x00010000 Chris Moore's
orion5x_winctrl_calcsize returns 0x0
For range 0x00010001 to 0x00020000 Chris Moore's
orion5x_winctrl_calcsize returns 0x1
For range 0x00020001 to 0x00040000 Chris Moore's
orion5x_winctrl_calcsize returns 0x3
For range 0x00040001 to 0x00080000 Chris Moore's
orion5x_winctrl_calcsize returns 0x7
For range 0x00080001 to 0x00100000 Chris Moore's
orion5x_winctrl_calcsize returns 0xf
For range 0x00100001 to 0x00200000 Chris Moore's
orion5x_winctrl_calcsize returns 0x1f
For range 0x00200001 to 0x00400000 Chris Moore's
orion5x_winctrl_calcsize returns 0x3f
For range 0x00400001 to 0x00800000 Chris Moore's
orion5x_winctrl_calcsize returns 0x7f
For range 0x00800001 to 0x01000000 Chris Moore's
orion5x_winctrl_calcsize returns 0xff
For range 0x01000001 to 0x02000000 Chris Moore's
orion5x_winctrl_calcsize returns 0x1ff
For range 0x02000001 to 0x04000000 Chris Moore's
orion5x_winctrl_calcsize returns 0x3ff
For range 0x04000001 to 0x08000000 Chris Moore's
orion5x_winctrl_calcsize returns 0x7ff
For range 0x08000001 to 0x10000000 Chris Moore's
orion5x_winctrl_calcsize returns 0xfff
For range 0x10000001 to 0x20000000 Chris Moore's
orion5x_winctrl_calcsize returns 0x1fff
For range 0x20000001 to 0x40000000 Chris Moore's
orion5x_winctrl_calcsize returns 0x3fff
For range 0x40000001 to 0x80000000 Chris Moore's
orion5x_winctrl_calcsize returns 0x7fff
For range 0x80000001 to 0xffffffff Chris Moore's
orion5x_winctrl_calcsize returns 0xffff
For range 0x00000000 to 0x00000000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0xffff
For range 0x00000001 to 0x00010000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x0
For range 0x00010001 to 0x00020000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x1
For range 0x00020001 to 0x00040000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x3
For range 0x00040001 to 0x00080000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x7
For range 0x00080001 to 0x00100000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0xf
For range 0x00100001 to 0x00200000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x1f
For range 0x00200001 to 0x00400000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x3f
For range 0x00400001 to 0x00800000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x7f
For range 0x00800001 to 0x01000000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0xff
For range 0x01000001 to 0x02000000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x1ff
For range 0x02000001 to 0x04000000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x3ff
For range 0x04000001 to 0x08000000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x7ff
For range 0x08000001 to 0x10000000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0xfff
For range 0x10000001 to 0x20000000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x1fff
For range 0x20000001 to 0x40000000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x3fff
For range 0x40000001 to 0x80000000 Chris Moore's fast
orion5x_winctrl_calcsize returns 0x7fff
For range 0x80000001 to 0xffffffff Chris Moore's fast
orion5x_winctrl_calcsize returns 0xffff
AIUI (apart from the question of how best to handle a size of 0) this is
the required result.
If I have misunderstood, please tell me and I'll rewrite.
HTH.
Cheers,
Chris
More information about the U-Boot
mailing list