Hello Wolfgang,
On Wed, Nov 9, 2011 at 11:44 AM, Wolfgang Denk wd@denx.de wrote:
In message CABkLObpuzibrEef+8EKKqTgyYcyTeC2qMx2UM1GqkzX--K3jLQ@mail.gmail.com you wrote:
All this needs a _thorough_ cleanup before I'm willing to accept this for mainline.
This is already in mainline, see
Indeed. What a pity.
Anyway. We should clean it up first, before attempting any other changes.
I don't understand yet what your exact requirements are - can you confirm that my assumption is correct that you can do without the "weak" if the hardwired constants in this fle get replaced by symbolic names that can be set from the board config file?
I'll comment on the code that is currently in u-boot-arm:arch /arm/cpu/arm926ejs/davinci/da850_lowlevel.c
263 #if defined(CONFIG_NAND_SPL)
I guess this will become obsolete soon, in the new SPL framework this should be done in another way, right?
264 void board_init_f(ulong bootflag) 265 #else 266 int arch_cpu_init(void) 267 #endif 268 { 269 /* 270 * copied from arch/arm/cpu/arm926ejs/start.S 271 * 272 * flush v4 I/D caches 273 */ 274 asm("mov r0, #0"); 275 asm("mcr p15, 0, r0, c7, c7, 0"); /* flush v3/v4 cache */ 276 asm("mcr p15, 0, r0, c8, c7, 0"); /* flush v4 TLB */ 277 278 /* 279 * disable MMU stuff and caches 280 */ 281 asm("mrc p15, 0, r0, c1, c0, 0"); 282 /* clear bits 13, 9:8 (--V- --RS) */ 283 asm("bic r0, r0, #0x00002300"); 284 /* clear bits 7, 2:0 (B--- -CAM) */ 285 asm("bic r0, r0, #0x00000087"); 286 /* set bit 2 (A) Align */ 287 asm("orr r0, r0, #0x00000002"); 288 /* set bit 12 (I) I-Cache */ 289 asm("orr r0, r0, #0x00001000"); 290 asm("mcr p15, 0, r0, c1, c0, 0"); 291
Heiko, why do we need this? I noticed, that u-boot takes longer to start when I remove this code.
292 /* Unlock kick registers */ 293 writel(0x83e70b13, &davinci_syscfg_regs->kick0); 294 writel(0x95a4f1e0, &davinci_syscfg_regs->kick1); 295
hawkboard has two defines HAWKBOARD_KICK0_UNLOCK and HAWKBOARD_KICK1_UNLOCK for these magic numbers. Maybe we should rename them because they are not HAWKBOARD specific and use them? see board/davinci/da8xxevm/hawkboard.c
296 dv_maskbits(&davinci_syscfg_regs->suspsrc, 297 ((1 << 27) | (1 << 22) | (1 << 20) | (1 << 5) | (1 << 16))); 298
This is done in a nicer way in board/davinci/da8xxevm/da850evm.c I wonder if these settings work for all boards or if any boards wound need different settings here.
299 /* Setup Pinmux */ 300 da850_pinmux_ctl(0, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX0); 301 da850_pinmux_ctl(1, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX1); 302 da850_pinmux_ctl(2, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX2); 303 da850_pinmux_ctl(3, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX3); 304 da850_pinmux_ctl(4, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX4); 305 da850_pinmux_ctl(5, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX5); 306 da850_pinmux_ctl(6, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX6); 307 da850_pinmux_ctl(7, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX7); 308 da850_pinmux_ctl(8, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX8); 309 da850_pinmux_ctl(9, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX9); 310 da850_pinmux_ctl(10, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX10); 311 da850_pinmux_ctl(11, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX11); 312 da850_pinmux_ctl(12, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX12); 313 da850_pinmux_ctl(13, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX13); 314 da850_pinmux_ctl(14, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX14); 315 da850_pinmux_ctl(15, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX15); 316 da850_pinmux_ctl(16, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX16); 317 da850_pinmux_ctl(17, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX17); 318 da850_pinmux_ctl(18, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX18); 319 da850_pinmux_ctl(19, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX19); 320
I could do with this code and setting my custom PINMUX constants. However, the da850evm uses a different way of configuring pinmux, so we have a duplication of code here. I'd prefer the da850evm way because the code is still readable when you don't use the TI tool mentioned by Heiko in [1] (I was not aware of this tool, thanks for the hint, Heiko!).
321 /* PLL setup */ 322 da850_pll_init(davinci_pllc0_regs, CONFIG_SYS_DA850_PLL0_PLLM); 323 da850_pll_init(davinci_pllc1_regs, CONFIG_SYS_DA850_PLL1_PLLM);
On my board I need to determine the hardware revision first and then set the values of CONFIG_SYS_DA850_PLL0_PLLM and CONFIG_SYS_DA850_PLL1_PLLM accordingly. But maybe I could uses something like
#define CONFIG_SYS_DA850_PLL1_PLLM board_get_pllm1()
and write a function that reads the board revision...
324 325 /* GPIO setup */ 326 board_gpio_init();
Why is this done here? For SPL? Will this change when the code moves to the new framework? I don't need GPIOs here.
327 328 /* setup CSn config */ 329 writel(CONFIG_SYS_DA850_CS2CFG, &davinci_emif_regs->ab1cr); 330 writel(CONFIG_SYS_DA850_CS3CFG, &davinci_emif_regs->ab2cr); 331
Ok for NAND, but how about a board that uses SPI flash? It should be possible to chose whether to initialize CSn.
332 lpsc_on(DAVINCI_LPSC_UART2); 333 NS16550_init((NS16550_t)(CONFIG_SYS_NS16550_COM1), 334 CONFIG_SYS_NS16550_CLK / 16 / CONFIG_BAUDRATE); 335 336 /* 337 * Fix Power and Emulation Management Register 338 * see sprufw3a.pdf page 37 Table 24 339 */ 340 writel(readl((CONFIG_SYS_NS16550_COM1 + 0x30)) | 0x00006001, 341 (CONFIG_SYS_NS16550_COM1 + 0x30));
I guess this is only needed for debugging the SPL?
342 #if defined(CONFIG_NAND_SPL) 343 puts("ddr init\n"); 344 da850_ddr_setup(132); 345 346 puts("boot u-boot ...\n"); 347 348 nand_boot(); 349 #else 350 da850_ddr_setup(132);
Ok for my board. But how about boards that use SDRAM on EMIFA instead (if there are any)?
351 return 0; 352 #endif 353 }
Regards, Christian
[1] http://lists.denx.de/pipermail/u-boot/2011-November/109104.html