[U-Boot] [PATCH] mpc86xx: adjust the DDR BATs after calculating true DDR size

[Timur Tabi]

On Mon, Mar 29, 2010 at 5:09 PM, Becky Bruce <beckyb at kernel.crashing.org> wrote:

>> --- a/board/freescale/mpc8641hpcn/mpc8641hpcn.c
>> +++ b/board/freescale/mpc8641hpcn/mpc8641hpcn.c
>> @@ -72,6 +72,8 @@ initdram(int board_type)
>>       dram_size = fixed_sdram();
>> #endif
>>
>> +     adjust_ddr_bat(dram_size);
>> +
>
> By doing this here, you still have a (small) window where the problem
> could occur.

U-Boot hasn't relocated yet, so it's not using any of the DDR.  I
don't see how there could be a problem.

> It's highly unlikely, but we might still have problems
> going forward.  I think we might need to:
>
> 1) remove the write of BAT0 from setup_bats (add a comment).  This
> way, there is no BAT setup for the DDR until right after we configure
> the controller

Ok, I get it now.  It will catch anyone trying to write to DDR before
it's really set up.  I can do that.

> 2) change the name "adjust_ddr_bat" to "setup_ddr_bat" or something
> similar.  I haven't dug around too much to see if this causes
> problems, but I am able to boot my 8641 this way.

Ok.

> 3) Change setup_ddr_bat so that it just does a write (remove the BL
> from the #defined values for the default BAT0 and or them in here
> instead, and add a comment to the config file that says BL is
> determined dynamically

Ok.

> I think you need to adjust how much usable ram u-boot thinks it has if
> you can't map it all.

I tried that.  The problem is that U-Boot uses this number to tell
Linux how much DDR there is.  The code doesn't really support U-Boot
and Linux seeing different amounts of DDR.

> If you have one BAT, and you have an amount of
> memory that is ! a power of 2, then you're going to leave a chunk
> unmapped, which can cause problems later.

AFAIK, we always have only one BAT for DDR.  I wasn't planning on
expanding the scope of this patch to add support for multiple BATs.

I don't know how to handle !2^X sizes of DDR.

>> +#define KB_TO_BATU(x) ((((x)/128) - 1) * 4) /* Convert KBs to BATU
>> value */
>
> It seems somewhat arbitrary that you decided to use take KB here as an
> arg when the BATU_SIZE macro returns bytes.   I'd prefer to see
> symmetry here.

I used KB to keep the sizes of numbers small.  The smallest value is
128KB, so it's not *that* arbitrary.

-- 
Timur Tabi
Linux kernel developer at Freescale