[U-Boot] [PATCH v2] mpc86xx: set the DDR BATs after calculating true DDR size
Becky Bruce
beckyb at kernel.crashing.org
Tue Mar 30 19:05:51 CEST 2010
On Mar 30, 2010, at 12:02 PM, Becky Bruce wrote:
> Did I miss something here? Why did you change the coherence attribute
> in the BAT? Please add an explanation to the comment, assuming you
> have a good reason for this (I'm not awake enough to think about this
> right now :)
>
Ah, my brain comes online - I see you only did this for 8610. Carry
on :)
-B
> Otherwise, a couple of minor nits below.
>
> On Mar 30, 2010, at 9:25 AM, Kumar Gala wrote:
>
>> From: Timur Tabi <timur at freescale.com>
>>
>> After determining how much DDR is actually in the system, set DBAT0
>> and
>> IBAT0 accordingly. This ensures that the CPU won't attempt to access
>> (via speculation) addresses outside of actual memory.
>>
>> On 86xx systems, DBAT0 and IBAT0 (the BATs for DDR) are initialized
>> to 2GB
>> and kept that way. If the system has less than 2GB of memory
>> (typical for
>> an MPC8610 HPCD), the CPU may attempt to access this memory during
>> speculation. The zlib code is notorious for generating such memory
>> reads,
>> and indeed on the MPC8610, uncompressing the Linux kernel causes a
>> machine
>> check (without this patch).
>>
>> Signed-off-by: Timur Tabi <timur at freescale.com>
>> Signed-off-by: Kumar Gala <galak at kernel.crashing.org>
>> ---
>> * Fixed TO_BATU_BL to handle non-power of two
>> * Fixed write_bat to use computed 'bl' not always 2G size
>> * removed comment that wasn't need anymore
>>
>> board/freescale/mpc8610hpcd/mpc8610hpcd.c | 2 +
>> board/freescale/mpc8641hpcn/mpc8641hpcn.c | 2 +
>> cpu/mpc86xx/cpu.c | 30 ++++++++++++++++++++
>> +++++++++
>> cpu/mpc86xx/cpu_init.c | 4 +++
>> include/asm-ppc/mmu.h | 6 ++++-
>> include/configs/MPC8610HPCD.h | 6 +---
>> include/configs/MPC8641HPCN.h | 4 +--
>> include/mpc86xx.h | 2 +
>> 8 files changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/board/freescale/mpc8610hpcd/mpc8610hpcd.c b/board/
>> freescale/mpc8610hpcd/mpc8610hpcd.c
>> index 784a2ed..ab5f800 100644
>> --- a/board/freescale/mpc8610hpcd/mpc8610hpcd.c
>> +++ b/board/freescale/mpc8610hpcd/mpc8610hpcd.c
>> @@ -127,6 +127,8 @@ initdram(int board_type)
>> dram_size = fixed_sdram();
>> #endif
>>
>> + setup_ddr_bat(dram_size);
>> +
>> puts(" DDR: ");
>> return dram_size;
>> }
>> diff --git a/board/freescale/mpc8641hpcn/mpc8641hpcn.c b/board/
>> freescale/mpc8641hpcn/mpc8641hpcn.c
>> index c521527..443c9fd 100644
>> --- a/board/freescale/mpc8641hpcn/mpc8641hpcn.c
>> +++ b/board/freescale/mpc8641hpcn/mpc8641hpcn.c
>> @@ -74,6 +74,8 @@ initdram(int board_type)
>> dram_size = fixed_sdram();
>> #endif
>>
>> + setup_ddr_bat(dram_size);
>> +
>> puts(" DDR: ");
>> return dram_size;
>> }
>> diff --git a/cpu/mpc86xx/cpu.c b/cpu/mpc86xx/cpu.c
>> index f7e012d..ac171f5 100644
>> --- a/cpu/mpc86xx/cpu.c
>> +++ b/cpu/mpc86xx/cpu.c
>> @@ -197,3 +197,33 @@ void mpc86xx_reginfo(void)
>> printf("\tBR7\t0x%08X\tOR7\t0x%08X \n", in_be32(&lbc->br7),
>> in_be32(&lbc->or7));
>>
>> }
>> +
>> +/*
>> + * Set the DDR BATs to reflect the actual size of DDR.
>> + *
>> + * dram_size is the actual size of DDR, in bytes
>> + *
>> + * Note: we assume that CONFIG_MAX_MEM_MAPPED is 2G or smaller as
>> we only
>> + * are using a single BAT to cover DDR.
>> + *
>> + * If this is not true, (e.g. CONFIG_MAX_MEM_MAPPED is 2GB but
>> HID0_XBSEN
>> + * is not defined) then we might have a situation where U-Boot will
>> attempt
>> + * to relocated itself outside of the region mapped by DBAT0.
>> + * This will cause a machine check.
>
> At the very least, fix this comment to mention the problem with not
> being able to map all the RAM as well, if you're going to leave it
> that way. Can you test a board with a strange amount of RAM (1.5GB,
> or something), and see what happens with this patch? I really don't
> like leaving things this way.
>
>> + *
>> + */
>> +void setup_ddr_bat(phys_addr_t dram_size)
>> +{
>> + unsigned long batu, bl;
>> +
>> + bl = TO_BATU_BL(min(dram_size, CONFIG_MAX_MEM_MAPPED));
>> +
>> + if (BATU_SIZE(bl) != dram_size) {
>> + u64 sz = (u64)dram_size - BATU_SIZE(bl);
>> + print_size(sz, " left unmapped\n");
>> + }
>> +
>> + batu = bl | BATU_VS | BATU_VP;
>> + write_bat(DBAT0, batu, CONFIG_SYS_DBAT0L);
>> + write_bat(IBAT0, batu, CONFIG_SYS_IBAT0L);
>> +}
>> diff --git a/cpu/mpc86xx/cpu_init.c b/cpu/mpc86xx/cpu_init.c
>> index 5a78a9c..b4f047d 100644
>> --- a/cpu/mpc86xx/cpu_init.c
>> +++ b/cpu/mpc86xx/cpu_init.c
>> @@ -138,8 +138,12 @@ int cpu_init_r(void)
>> /* Set up BAT registers */
>> void setup_bats(void)
>> {
>> +#if defined(CONFIG_SYS_DBAT0U) && defined(CONFIG_SYS_DBAT0L)
>> write_bat(DBAT0, CONFIG_SYS_DBAT0U, CONFIG_SYS_DBAT0L);
>> +#endif
>> +#if defined(CONFIG_SYS_IBAT0U) && defined(CONFIG_SYS_IBAT0L)
>> write_bat(IBAT0, CONFIG_SYS_IBAT0U, CONFIG_SYS_IBAT0L);
>> +#endif
>> write_bat(DBAT1, CONFIG_SYS_DBAT1U, CONFIG_SYS_DBAT1L);
>> write_bat(IBAT1, CONFIG_SYS_IBAT1U, CONFIG_SYS_IBAT1L);
>> write_bat(DBAT2, CONFIG_SYS_DBAT2U, CONFIG_SYS_DBAT2L);
>> diff --git a/include/asm-ppc/mmu.h b/include/asm-ppc/mmu.h
>> index fd10249..ce7f081 100644
>> --- a/include/asm-ppc/mmu.h
>> +++ b/include/asm-ppc/mmu.h
>> @@ -213,7 +213,11 @@ extern void print_bats(void);
>> #define BATL_PADDR(x) ((phys_addr_t)((x & 0xfffe0000) \
>> | ((x & 0x0e00ULL) << 24) \
>> | ((x & 0x04ULL) << 30)))
>> -#define BATU_SIZE(x) (1UL << (fls((x & BATU_BL_MAX) >> 2) + 17))
>> +#define BATU_SIZE(x) (1ULL << (fls((x & BATU_BL_MAX) >> 2) + 17))
>> +
>> +/* bytes into BATU_BL */
>> +#define TO_BATU_BL(x) \
>> + (u32)((((1ull << __ilog2_u64((u64)x)) / (128 * 1024)) - 1) * 4)
>
> It's a nit, but can we change the *4 to << 2 ? I know most modern
> compilers should optimize this, but I think it makes the code easier
> to read and is logically more sensical, and if you've got to change
> the patch, anyway, we might as well clean this up.
>
> -B
>
>
>>
>> /* Used to set up SDR1 register */
>> #define HASH_TABLE_SIZE_64K 0x00010000
>> diff --git a/include/configs/MPC8610HPCD.h b/include/configs/
>> MPC8610HPCD.h
>> index 1d2d659..fed441e 100644
>> --- a/include/configs/MPC8610HPCD.h
>> +++ b/include/configs/MPC8610HPCD.h
>> @@ -341,10 +341,8 @@
>> * BAT0 2G Cacheable, non-guarded
>> * 0x0000_0000 2G DDR
>> */
>> -#define CONFIG_SYS_DBAT0L (BATL_PP_RW | BATL_MEMCOHERENCE)
>> -#define CONFIG_SYS_DBAT0U (BATU_BL_2G | BATU_VS | BATU_VP)
>> -#define CONFIG_SYS_IBAT0L (BATL_PP_RW | BATL_MEMCOHERENCE )
>> -#define CONFIG_SYS_IBAT0U CONFIG_SYS_DBAT0U
>> +#define CONFIG_SYS_DBAT0L (BATL_PP_RW)
>> +#define CONFIG_SYS_IBAT0L (BATL_PP_RW)
>>
>> /*
>> * BAT1 1G Cache-inhibited, guarded
>> diff --git a/include/configs/MPC8641HPCN.h b/include/configs/
>> MPC8641HPCN.h
>> index 12a8f60..94e4d24 100644
>> --- a/include/configs/MPC8641HPCN.h
>> +++ b/include/configs/MPC8641HPCN.h
>> @@ -482,9 +482,7 @@ extern unsigned long get_board_sys_clk(unsigned
>> long dummy);
>> * BAT0 DDR
>> */
>> #define CONFIG_SYS_DBAT0L (BATL_PP_RW | BATL_MEMCOHERENCE)
>> -#define CONFIG_SYS_DBAT0U (BATU_BL_2G | BATU_VS | BATU_VP)
>> -#define CONFIG_SYS_IBAT0L (BATL_PP_RW | BATL_MEMCOHERENCE )
>> -#define CONFIG_SYS_IBAT0U CONFIG_SYS_DBAT0U
>> +#define CONFIG_SYS_IBAT0L (BATL_PP_RW | BATL_MEMCOHERENCE)
>>
>> /*
>> * BAT1 LBC (PIXIS/CF)
>> diff --git a/include/mpc86xx.h b/include/mpc86xx.h
>> index c6f30f9..eb85d60 100644
>> --- a/include/mpc86xx.h
>> +++ b/include/mpc86xx.h
>> @@ -83,5 +83,7 @@ static __inline__ unsigned long get_l2cr (void)
>> return l2cr_val;
>> }
>>
>> +void setup_ddr_bat(phys_addr_t dram_size);
>> +
>> #endif /* _ASMLANGUAGE */
>> #endif /* __MPC86xx_H__ */
>> --
>> 1.6.0.6
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
More information about the U-Boot
mailing list