On 07/02/2013 01:39:09 AM, Po Liu wrote:
+int board_early_init_r(void) +{
- const unsigned long flashbase = CONFIG_SYS_FLASH_BASE;
- const u8 flash_esel = find_tlb_idx((void *)flashbase, 1);
- /*
* Remap Boot flash region to caching-inhibited* so that flash can be erased properly.*/- /* Flush d-cache and invalidate i-cache of any FLASH data */
- flush_dcache();
- invalidate_icache();
- /* invalidate existing TLB entry for flash */
- disable_tlb(flash_esel);
- set_tlb(1, flashbase, CONFIG_SYS_FLASH_BASE_PHYS,
MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,0, flash_esel, BOOKE_PAGESZ_64M, 1);
Don't set MAS3_SX on I/O regions.
+#if defined(CONFIG_OF_BOARD_SETUP) +void fdt_del_sec(void *blob, int offset) +{
- int nodeoff = 0;
- while ((nodeoff = fdt_node_offset_by_compat_reg(blob,
"fsl,sec-v6.0",
CONFIG_SYS_CCSRBAR_PHYS +CONFIG_SYS_FSL_SEC_OFFSET
+ offset * 0x20000)) >= 0) {fdt_del_node(blob, nodeoff);offset++;}+}
Whitespace
+static u8 __cpld_read(unsigned int reg) +{
- void *p = (void *)CONFIG_SYS_CPLD_BASE;
- return in_8(p + reg);
+}
+u8 cpld_read(unsigned int reg) __attribute__((weak, alias("__cpld_read")));
+static void __cpld_write(unsigned int reg, u8 value) +{
- void *p = (void *)CONFIG_SYS_CPLD_BASE;
- out_8(p + reg, value);
+}
+void cpld_write(unsigned int reg, u8 value)
- __attribute__((weak, alias("__cpld_write")));
Why does this need to be weak? Where do you expect to call cpld_read/write that isn't specific to this board? What sort of behavior could a non-board-specific caller possibly expect?
+/**
- Set the boot bank to the alternate bank
- */
+void __cpld_set_altbank(u8 banksel) +{
- u8 reg11 = CPLD_READ(flhcsr);
- switch (banksel) {
- case 1:
CPLD_WRITE(flhcsr, (reg11 & CPLD_BANKSEL_MASK)| CPLD_BANKSEL_EN | CPLD_SELECT_BANK1);break;- case 2:
CPLD_WRITE(flhcsr, (reg11 & CPLD_BANKSEL_MASK)| CPLD_BANKSEL_EN | CPLD_SELECT_BANK2);break;- case 3:
CPLD_WRITE(flhcsr, (reg11 & CPLD_BANKSEL_MASK)| CPLD_BANKSEL_EN | CPLD_SELECT_BANK3);break;- case 4:
CPLD_WRITE(flhcsr, (reg11 & CPLD_BANKSEL_MASK)| CPLD_BANKSEL_EN | CPLD_SELECT_BANK4);break;
So there's a __cpld_write, a cpld_write, *and* a CPLD_WRITE?
+#define CPLD_READ(reg) cpld_read(offsetof(struct cpld_data, reg)) +#define CPLD_WRITE(reg, value) cpld_write(offsetof(struct cpld_data, reg), \
value)
Ugh. Why not just initialize a pointer to CONFIG_SYS_CPLD_BASE, and use normal I/O accessors on that, just like every other driver that isn't the p2041rdb CPLD code you copied this from?
+struct law_entry law_table[] = {
- SET_LAW(CONFIG_SYS_FLASH_BASE_PHYS, LAW_SIZE_64M,
LAW_TRGT_IF_IFC),
- SET_LAW(CONFIG_SYS_CPLD_BASE_PHYS, LAW_SIZE_4K,
LAW_TRGT_IF_IFC),
- SET_LAW(CONFIG_SYS_NAND_BASE_PHYS, LAW_SIZE_1M,
LAW_TRGT_IF_IFC),
The NAND SRAM window is 16K. There is no reason to make the LAW 1M.
- /* TLB 1 */
- /* *I*** - Covers boot page */
- SET_TLB_ENTRY(1, 0xfffff000, 0xfffff000,
MAS3_SX|MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,0, 0, BOOKE_PAGESZ_4K, 1),
Comment says *I*** but code says *I*G*.
Do we still use this mapping at all?
- /* *I*G* - CCSRBAR */
- SET_TLB_ENTRY(1, CONFIG_SYS_CCSRBAR, CONFIG_SYS_CCSRBAR_PHYS,
MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,0, 1, BOOKE_PAGESZ_1M, 1),- SET_TLB_ENTRY(1, CONFIG_SYS_FLASH_BASE,
CONFIG_SYS_FLASH_BASE_PHYS,
MAS3_SX|MAS3_SR, MAS2_W|MAS2_G,0, 2, BOOKE_PAGESZ_64M, 1),
Why do most of these entries have a comment but some (like this flash entry) don't? Be consistent. Probably better to remove all of these comments since they don't say anything that isn't clear from the code, and are often wrong.
- SET_TLB_ENTRY(1, CONFIG_SYS_NAND_BASE,
CONFIG_SYS_NAND_BASE_PHYS,
MAS3_SW|MAS3_SR, MAS2_I|MAS2_G,0, 6, BOOKE_PAGESZ_1M, 1),
Again, this can be 16K.
- /* *I*G - platform SRAM */
- SET_TLB_ENTRY(1, CONFIG_SYS_PLATFORM_SRAM_BASE,
CONFIG_SYS_PLATFORM_SRAM_BASE_PHYS,MAS3_SX|MAS3_SW|MAS3_SR, 0,0, 7, BOOKE_PAGESZ_256K, 1),- SET_TLB_ENTRY(1, CONFIG_SYS_PLATFORM_SRAM_BASE + 0x40000,
CONFIG_SYS_PLATFORM_SRAM_BASE_PHYS + 0x40000,MAS3_SX|MAS3_SW|MAS3_SR, 0,0, 8, BOOKE_PAGESZ_256K, 1),
Comment says *I*G but it isn't.
+/* Use the HUSH parser */ +#define CONFIG_SYS_HUSH_PARSER +#ifdef CONFIG_SYS_HUSH_PARSER +#define CONFIG_SYS_PROMPT_HUSH_PS2 "> " +#endif
Again, "Please go through this file and only retain things which actually continue to make sense."
This is a pointless ifdef, and a pointless define of CONFIG_SYS_PROMPT_HUSH_PS2 since that's the same as the default.
+/*
- Internal Definitions
- Boot Flags
- */
+#define BOOTFLAG_COLD 0x01 /* Normal Power-On: Boot from FLASH */ +#define BOOTFLAG_WARM 0x02 /* Software reboot */
Sigh.
-Scott