[U-Boot] [PATCH 0/3] powerpc/85xx: Fix TQM8560 DDR boot issue

Series to fix the TQM85xx issue reported by Wolfgang.... the problem turned out to be that those boards use get_ram_size(), which requires that a TLB entry be set up for the DDR region. My earlier patches changed the way this was done and broke this.
I've created a new function, clear_ddr_tlbs(), so that there is now a setup_ddr_tlbs() and a matching clear_ddr_tlbs() that boards can use if they need to temporarily set up a TLB entry for this purpose. The tqm85xx ddr init code now uses this and the board is able to boot.
There is also a cleanup patch to the TQM header file to make it easier to read - I added code that is dependent on this so it's included in this series.
Cheers, Becky
arch/powerpc/cpu/mpc85xx/cpu.c | 14 +++----------- arch/powerpc/cpu/mpc85xx/tlb.c | 29 +++++++++++++++++++++++++++++ arch/powerpc/include/asm/mmu.h | 1 + board/tqc/tqm85xx/sdram.c | 7 +++++++ include/configs/TQM85xx.h | 9 ++++++++- 5 files changed, 48 insertions(+), 12 deletions(-)

This is useful when we just want to wipe out the TLBs. There's currently a function that resets the ddr tlbs to a different value; it is changed to utilize this function. The new function can be used in conjunction with setup_ddr_tlbs() for a board to temporarily map/unmap the DDR address range as needed.
Signed-off-by: Becky Bruce beckyb@kernel.crashing.org --- arch/powerpc/cpu/mpc85xx/cpu.c | 14 +++----------- arch/powerpc/cpu/mpc85xx/tlb.c | 29 +++++++++++++++++++++++++++++ arch/powerpc/include/asm/mmu.h | 1 + 3 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/cpu/mpc85xx/cpu.c b/arch/powerpc/cpu/mpc85xx/cpu.c index 53f0887..ce59c25 100644 --- a/arch/powerpc/cpu/mpc85xx/cpu.c +++ b/arch/powerpc/cpu/mpc85xx/cpu.c @@ -374,6 +374,8 @@ void read_tlbcam_entry(int idx, u32 *valid, u32 *tsize, unsigned long *epn, unsigned int setup_ddr_tlbs_phys(phys_addr_t p_addr, unsigned int memsize_in_meg);
+void clear_ddr_tlbs_phys(phys_addr_t p_addr, unsigned int memsize_in_meg); + static void dump_spd_ddr_reg(void) { int i, j, k, m; @@ -460,19 +462,9 @@ static int reset_tlb(phys_addr_t p_addr, u32 size, phys_addr_t *phys_offset) u32 vstart = CONFIG_SYS_DDR_SDRAM_BASE; unsigned long epn; u32 tsize, valid, ptr; - phys_addr_t rpn = 0; int ddr_esel;
- ptr = vstart; - - while (ptr < (vstart + size)) { - ddr_esel = find_tlb_idx((void *)ptr, 1); - if (ddr_esel != -1) { - read_tlbcam_entry(ddr_esel, &valid, &tsize, &epn, &rpn); - disable_tlb(ddr_esel); - } - ptr += TSIZE_TO_BYTES(tsize); - } + clear_ddr_tlbs_phys(p_addr, size>>20);
/* Setup new tlb to cover the physical address */ setup_ddr_tlbs_phys(p_addr, size>>20); diff --git a/arch/powerpc/cpu/mpc85xx/tlb.c b/arch/powerpc/cpu/mpc85xx/tlb.c index 295f175..01a3561 100644 --- a/arch/powerpc/cpu/mpc85xx/tlb.c +++ b/arch/powerpc/cpu/mpc85xx/tlb.c @@ -300,4 +300,33 @@ unsigned int setup_ddr_tlbs(unsigned int memsize_in_meg) return setup_ddr_tlbs_phys(CONFIG_SYS_DDR_SDRAM_BASE, memsize_in_meg); } + +/* Invalidate the DDR TLBs for the requested size */ +void clear_ddr_tlbs_phys(phys_addr_t p_addr, unsigned int memsize_in_meg) +{ + u32 vstart = CONFIG_SYS_DDR_SDRAM_BASE; + unsigned long epn; + u32 tsize, valid, ptr; + phys_addr_t rpn = 0; + int ddr_esel; + u64 memsize = (u64)memsize_in_meg << 20; + + ptr = vstart; + + while (ptr < (vstart + memsize)) { + ddr_esel = find_tlb_idx((void *)ptr, 1); + if (ddr_esel != -1) { + read_tlbcam_entry(ddr_esel, &valid, &tsize, &epn, &rpn); + disable_tlb(ddr_esel); + } + ptr += TSIZE_TO_BYTES(tsize); + } +} + +void clear_ddr_tlbs(unsigned int memsize_in_meg) +{ + clear_ddr_tlbs_phys(CONFIG_SYS_DDR_SDRAM_BASE, memsize_in_meg); +} + + #endif /* !CONFIG_NAND_SPL */ diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h index c01c85f..ef5076b 100644 --- a/arch/powerpc/include/asm/mmu.h +++ b/arch/powerpc/include/asm/mmu.h @@ -489,6 +489,7 @@ extern int find_free_tlbcam(void); extern void print_tlbcam(void);
extern unsigned int setup_ddr_tlbs(unsigned int memsize_in_meg); +extern void clear_ddr_tlbs(unsigned int memsize_in_meg);
extern void write_tlb(u32 _mas0, u32 _mas1, u32 _mas2, u32 _mas3, u32 _mas7);

The way I wrote this in the first time was based on the original code, but I used a karnaugh map to make it a bit easier to read..... There should be no functional change.
Signed-off-by: Becky Bruce beckyb@kernel.crashing.org --- include/configs/TQM85xx.h | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/include/configs/TQM85xx.h b/include/configs/TQM85xx.h index 79a958d..b4eef28 100644 --- a/include/configs/TQM85xx.h +++ b/include/configs/TQM85xx.h @@ -147,10 +147,15 @@ * DDR Setup */ #define CONFIG_SYS_DDR_SDRAM_BASE 0x00000000 /* DDR is system memory */ + #if defined(CONFIG_TQM_BIGFLASH) || \ - (!defined(CONFIG_TQM8548_AG) && !defined(CONFIG_TQM8548_BE)) + defined(CONFIG_TQM8548_AG) || \ + defined(CONFIG_TQM8548_BE) #define CONFIG_SYS_PPC_DDR_WIMGE (MAS2_I | MAS2_G) +#else +#define CONFIG_SYS_PPC_DDR_WIMGE (0) #endif + #define CONFIG_SYS_SDRAM_BASE CONFIG_SYS_DDR_SDRAM_BASE #ifdef CONFIG_TQM8548_AG #define CONFIG_VERY_BIG_RAM

We need a TLB entry to call get_ram_size(); the common code doesn't create one until *after* fixed_sdram() has determined the size. So we set up tlbs for the max possible size and tear them down once we're done with get_ram_size(); the common 85xx code will then set up a final set of tlb entries for the *actual* detected size of ddr.
This prevents us from having TLB entries that are larger than DDR sitting around for very long, which is not a recommended scenario.
Signed-off-by: Becky Bruce beckyb@kernel.crashing.org --- board/tqc/tqm85xx/sdram.c | 7 +++++++ include/configs/TQM85xx.h | 2 ++ 2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/board/tqc/tqm85xx/sdram.c b/board/tqc/tqm85xx/sdram.c index 39a9e21..baf073e 100644 --- a/board/tqc/tqm85xx/sdram.c +++ b/board/tqc/tqm85xx/sdram.c @@ -363,6 +363,12 @@ static phys_size_t sdram_setup(int casl) udelay (1000); #endif /* CONFIG_TQM8548 */
+ /* + * get_ram_size() depends on having tlbs for the DDR, but they are + * not yet setup because we don't know the size. Set up a temp + * mapping and delete it when done. + */ + setup_ddr_tlbs(CONFIG_SYS_DDR_EARLY_SIZE_MB); for (i = 0; i < N_DDR_CS_CONF; i++) { ddr->cs0_config = ddr_cs_conf[i].reg;
@@ -376,6 +382,7 @@ static phys_size_t sdram_setup(int casl) break; } } + clear_ddr_tlbs(CONFIG_SYS_DDR_EARLY_SIZE_MB);
#ifdef CONFIG_TQM8548 if (i < N_DDR_CS_CONF) { diff --git a/include/configs/TQM85xx.h b/include/configs/TQM85xx.h index b4eef28..0eadcd2 100644 --- a/include/configs/TQM85xx.h +++ b/include/configs/TQM85xx.h @@ -152,8 +152,10 @@ defined(CONFIG_TQM8548_AG) || \ defined(CONFIG_TQM8548_BE) #define CONFIG_SYS_PPC_DDR_WIMGE (MAS2_I | MAS2_G) +#define CONFIG_SYS_DDR_EARLY_SIZE_MB (512) #else #define CONFIG_SYS_PPC_DDR_WIMGE (0) +#define CONFIG_SYS_DDR_EARLY_SIZE_MB (2 * 1024) #endif
#define CONFIG_SYS_SDRAM_BASE CONFIG_SYS_DDR_SDRAM_BASE

Dear Becky Bruce,
In message 13107687401917-git-send-email-beckyb@kernel.crashing.org you wrote:
The way I wrote this in the first time was based on the original code, but I used a karnaugh map to make it a bit easier to read..... There should be no functional change.
Ummm...
#if defined(CONFIG_TQM_BIGFLASH) || \
- (!defined(CONFIG_TQM8548_AG) && !defined(CONFIG_TQM8548_BE))
- defined(CONFIG_TQM8548_AG) || \
- defined(CONFIG_TQM8548_BE)
Sorry, but this looks wrong to me.
Isn't the rule "foo & bar == !(!foo || !bar)", so that
(!defined(CONFIG_TQM8548_AG) && !defined(CONFIG_TQM8548_BE))
becomes
!(defined(CONFIG_TQM8548_AG) || defined(CONFIG_TQM8548_BE))
?
I think you are missing the "not" part...
Best regards,
Wolfgang Denk
participants (2)
-
Becky Bruce
-
Wolfgang Denk