[PATCH] CONFIG_LB_MEM_TOPK -> CONFIG_RAMTOP

[Myles Watson]

LB_MEM_TOPK is making it hard for me to modify linker scripts because
sometimes the shifting breaks it.

RAMTOP is much nicer:
- No need to shift it
- Matches with RAMBASE
- Shows up in the correct place in coreboot_ram.map

topk_defaults.diff & topk_shifts.diff are almost trivial, and were done 99%
by scripts.
topk_fixups.diff was done by hand and verified with meld, so I'm pretty sure
but I'd like another pair of eyes.

There was only one place where I had to do (RAMBASE >> 10)!

Abuild tested.

Signed-off-by: Myles Watson

Thanks,
Myles

[Peter Stuge]

Myles Watson wrote:
> There was only one place where I had to do (RAMBASE >> 10)!

Maybe change that one to / 1024 instead?

In a few places CONFIG_RAMTOP is used without (), but none of them
looked bad. Maybe also search for ",CONFIG_RAMTOP" and add a space
there.
> Abuild tested.
>
> Signed-off-by: Myles Watson

Acked-by: Peter Stuge

[Myles Watson]

On Fri, Oct 16, 2009 at 10:12 AM, Peter Stuge  wrote:
> Myles Watson wrote:
> > There was only one place where I had to do (RAMBASE >> 10)!
> Maybe change that one to / 1024 instead?

Do you think it's clearer to use /1024?  I just meant it was surprising to
multiply the value by 1024 and only have to put it back once.

In a few places CONFIG_RAMTOP is used without (), but none of them
> looked bad. Maybe also search for ",CONFIG_RAMTOP" and add a space
> there.

Fixed.

Thanks,
Myles

[ron minnich]

On Fri, Oct 16, 2009 at 9:20 AM, Myles Watson  wrote:
> Do you think it's clearer to use /1024?  I just meant it was surprising to
> multiply the value by 1024 and only have to put it back once.

I prefer >>10 because it more clearly expresses what you want. I've
been burned from time to time by people using
an arithmetic op where a bit op was intended. (+ instead of | for example)

ron

[Myles Watson]

On Fri, Oct 16, 2009 at 10:25 AM, ron minnich  wrote:
> On Fri, Oct 16, 2009 at 9:20 AM, Myles Watson  wrote:
> > Do you think it's clearer to use /1024?  I just meant it was surprising
>
> to
> > multiply the value by 1024 and only have to put it back once.
>
> I prefer >>10 because it more clearly expresses what you want. I've
> been burned from time to time by people using
> an arithmetic op where a bit op was intended. (+ instead of | for example)

OK.

Rev 4788.

Thanks,
Myles

[Peter Stuge]

Myles Watson wrote:
> > > There was only one place where I had to do (RAMBASE >> 10)!
> >
> > Maybe change that one to / 1024 instead?
>
> Do you think it's clearer to use /1024?

Yes, very much so.
> I just meant it was surprising to multiply the value by 1024 and
> only have to put it back once.

I agree with that.
> > In a few places CONFIG_RAMTOP is used without (), but none of them
> > looked bad. Maybe also search for ",CONFIG_RAMTOP" and add a space
> > there.
>
> Fixed.

Great!

ron minnich wrote:
> I prefer >>10 because it more clearly expresses what you want.

I strongly prefer to get kilobytes from bytes using 1024 rather than 10.

I don't think the bit operator is more clear - this isn't a bit
operation, it's arithmetic.

The particular arithmetic happens to have an efficient implementation
using a bit operator, but the compiler knows that, so that the
programmer doesn't have to care and can use arithmetic expression,
which is meant to be (I think it is) more clear.
> I've been burned from time to time by people using an arithmetic op
> where a bit op was intended. (+ instead of | for example)

That would be a bug, but it doesn't translate to "all use of
arithmetic operators should be changed to use bit operators".

Not in C. For assembly you could convince me, but not in C. :)

//Peter

[Uwe Hermann]

On Fri, Oct 16, 2009 at 06:42:22PM +0200, Peter Stuge wrote:
> > I prefer >>10 because it more clearly expresses what you want.
>
> I strongly prefer to get kilobytes from bytes using 1024 rather than 10.
>
> I don't think the bit operator is more clear - this isn't a bit
> operation, it's arithmetic.

Full ACK.

Uwe.