-----Original Message----- From: Wolfgang Denk [mailto:wd@denx.de] Sent: Wednesday, September 23, 2009 3:16 AM To: Hu Mingkai-B21284 Cc: u-boot@lists.denx.de; Wood Scott-B07421; Gala Kumar-B11780 Subject: Re: [U-Boot] [PATCH v3 3/5] NAND boot: MPC8536DS support
- /* set up CCSR if we want it moved */ #if
+(CONFIG_SYS_CCSRBAR_DEFAULT != CONFIG_SYS_CCSRBAR_PHYS)
- {
volatile u32 *ccsr_virt =(volatile u32 *)(CONFIG_SYS_CCSRBAR + 0x1000);Please do not declare vaiables righjt in the middle of the code. Consider moving this code into a separate function instead.
OK.
diff --git a/cpu/mpc85xx/u-boot-nand.lds
b/cpu/mpc85xx/u-boot-nand.lds
index c14e946..a0fc8f1 100644 --- a/cpu/mpc85xx/u-boot-nand.lds +++ b/cpu/mpc85xx/u-boot-nand.lds @@ -65,10 +65,8 @@ SECTIONS PROVIDE (etext = .); .rodata : {
- *(.rodata)
- *(.rodata1)
- *(.rodata.str1.4) *(.eh_frame)
- *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*)))
Please rebase your patch against current code.
OK.
...
/*
- Config the L2 Cache as L2 SRAM
- */
+#define CONFIG_SYS_INIT_L2_ADDR 0xf8f80000 +#ifdef CONFIG_PHYS_64BIT +#define CONFIG_SYS_INIT_L2_ADDR_PHYS 0xff8f80000ull +#else +#define CONFIG_SYS_INIT_L2_ADDR_PHYS CONFIG_SYS_INIT_L2_ADDR +#endif +#define CONFIG_SYS_L2_SIZE (512 << 10) +#define CONFIG_SYS_INIT_L2_END
(CONFIG_SYS_INIT_L2_ADDR + CONFIG_SYS_L2_SIZE)
Line too long, please fix globally.
OK.
diff --git a/nand_spl/board/freescale/mpc8536ds/Makefile b/nand_spl/board/freescale/mpc8536ds/Makefile new file mode 100644 index 0000000..c9104d3 --- /dev/null +++ b/nand_spl/board/freescale/mpc8536ds/Makefile
...
+$(obj)tlb.c:
- @rm -f $(obj)tlb.c
- ln -sf $(SRCTREE)/cpu/mpc85xx/tlb.c $(obj)tlb.c
+$(obj)tlb_table.c:
- @rm -f $(obj)tlb_table.c
- ln -sf $(SRCTREE)/board/$(BOARDDIR)/tlb.c $(obj)tlb_table.c
Consider using the same file name here; it will simplify the Makefile rules.
+$(obj)law.c:
- @rm -f $(obj)law.c
- ln -sf $(SRCTREE)/drivers/misc/fsl_law.c $(obj)law.c
Ditto.
+$(obj)law_table.c:
- @rm -f $(obj)law_table.c
- ln -sf $(SRCTREE)/board/$(BOARDDIR)/law.c $(obj)law_table.c
Ditto.
Please sort list.
And why do you need a separate rule everywhere? They look all the same to me (except for the inconsistent file names) ?
Ok, I'll change it. But for some files, the name is same for different files in the different directory, for example, cpu/mpc85xx/tlb.c and board/freescale/mpc8536/tlb.c, so I changed the linked name.
- /* initialize selected port with appropriate baud rate */
- sysclk_ratio = *((volatile unsigned char *)(PIXIS_BASE +
+PIXIS_SPD));
Please use I/O accessors.
Thanks, I've modified this, and prepared a new version patch.
...
- NS16550_init((NS16550_t)(CONFIG_SYS_CCSRBAR + 0x4500),
bus_clk / 16 / CONFIG_BAUDRATE);Smells like I/O accessors should be used here (and further down below), too?
Ditto.
Thanks, Mingkai