arm, davinci: make arch_cpu_init() in da850_lowlevel.c weak
Subsystems (2)
PATCH 1/1 arm, davinci: make arch_cpu_init() in da850_lowlevel.c weak
This patch allows replacing arch_cpu_init() if a custom initialization is required.
--- arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c +1 -0
@@ -263,6 +263,7 @@ void nand_boot(void)#if defined(CONFIG_NAND_SPL )void board_init_f (ulong bootflag) #else +__attribute__ ((weak)) intarch_cpu_init (void) #endif {
Signature · 1 line
1.7.0.4
PATCH 1/1 arm, davinci: make arch_cpu_init() in da850_lowlevel.c weak
Dear Christian Riesch,
Quoted 15 lines · In message <[email protected]> you…
This patch allows replacing
arch_cpu_init()if a custom initialization is required.Signed-off-by Christian Riesch <[email protected]>Cc Sandeep Paulraj <[email protected]>Cc Heiko Schocher <[email protected]>--- arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c +0
-0
arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c void nand_boot(void) · line 268
Quoted 6 lines
arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c +1
-0@@ -263,6 +263,7 @@ void nand_boot(void)#if defined(CONFIG_NAND_SPL ) void board_init_f(ulong bootflag) #else +__attribute__ ((weak)) int arch_cpu_init(void)#endif
Stop please.
This whole code is crap and needs to be changed.
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");What is #0x00002300? #0x00000087? #0x00000002? #0x00001000?
293 writel(0x83e70b13, &davinci_syscfg_regs->kick0);
294 writel(0x95a4f1e0, &davinci_syscfg_regs->kick1);
What is 0x83e70b13? 0x95a4f1e0?
296 dv_maskbits(&davinci_syscfg_regs->suspsrc,
297 ((1 << 27) | (1 << 22) | (1 << 20) | (1 << 5) | (1 << 16)));
What is ((1 << 27) | (1 << 22) | (1 << 20) | (1 << 5) | (1 << 16))) ?
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));
What is CONFIG_SYS_NS16550_COM1 + 0x30 ???? don't we have proper NS16550 register definitions? And what is 0x00006001 ??
All this needs a _thorough_ cleanup before I'm willing to accept this for mainline.
Heiko, Christian: please negotiate who performs which part of the cleanup. But I expect that with proper symbolic names instead of the hardwired constants the need for a wek function will go away.
Albert, please make sure to block this current code. I do not want to see this in mainline as is.
Thanks.
Best regards,
Wolfgang Denk
Signature · 4 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [email protected] Another megabytes the dust.
PATCH 1/1 arm, davinci: make arch_cpu_init() in da850_lowlevel.c weak
Hi Wolfgang,
Quoted 58 lines · Wolfgang Denk
Quoted 17 lines · In message <[email protected]> you…
This patch allows replacing
arch_cpu_init()if a custom initialization is required.Signed-off-by Christian Riesch <[email protected]>Cc Sandeep Paulraj <[email protected]>Cc Heiko Schocher <[email protected]>---
?arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c | ? ?1 + ?1 files changed, 1 insertions(+), 0 deletions(-)arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c +0
-0@@ -263,6 +263,7 @@ void nand_boot(void) ?#if defined(CONFIG_NAND_SPL) ?void board_init_f(ulong bootflag)Quoted 4 lines
?#else
+__attribute__ ((weak))
?intarch_cpu_init(void)
?#endifStop please.
This whole code is crap and needs to be changed.
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");What is #0x00002300? #0x00000087? #0x00000002? #0x00001000?
293 ? ? ? ?
writel(0x83e70b13, &davinci_syscfg_regs->kick0); 294 ? ? ? ?writel(0x95a4f1e0, &davinci_syscfg_regs->kick1);What is
0x83e70b13?0x95a4f1e0?296 ? ? ? ? dv_maskbits(&davinci_syscfg_regs->suspsrc,
297 ? ? ? ? ? ? ? ? ((1 << 27) | (1 << 22) | (1 << 20) | (1 << 5) | (1 << 16)));What is ((1 << 27) | (1 << 22) | (1 << 20) | (1 << 5) | (1 << 16))) ?
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));
Quoted 6 lines
What is
CONFIG_SYS_NS16550_COM1+0x30???? don't we have proper NS16550 register definitions? ?And what is0x00006001??All this needs a _thorough_ cleanup before I'm willing to accept this for mainline.
This is already in mainline, see
commit 310ae55efe14aa7923b16c718cbdb22ec364b18b
Author: Heiko Schocher <[email protected]> Date: Wed Sep 14 19:59:38 2011 +0000
arm, davinci, am1808: add lowlevel functions for booting from NOR
Regards, Christian
Quoted 26 lines
Heiko, Christian: please negotiate who performs which part of the cleanup. But I expect that with proper symbolic names instead of the hardwired constants the need for a wek function will go away.
Albert, please make sure to block this current code. I do not want to see this in mainline as is.
Thanks.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Another megabytes the dust.
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH 1/1 arm, davinci: make arch_cpu_init() in da850_lowlevel.c weak
Dear Christian Riesch,
Quoted 5 lines · In message <CABkLObpuzibrEef+8EKKqTgyYcyTeC2qMx2UM1GqkzX--K3jLQ@mail.gmail.com>…
Quoted 3 lines
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?
Best regards,
Wolfgang Denk
Signature · 6 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de Humanity has the stars in its future, and that future is too important to be lost under the burden of juvenile folly and ignorant superstition. - Isaac Asimov
PATCH 1/1 arm, davinci: make arch_cpu_init() in da850_lowlevel.c weak
Hello Wolfgang,
Quoted 9 lines · Wolfgang Denk
Quoted 5 lines · In message <CABkLObpuzibrEef+8EKKqTgyYcyTeC2qMx2UM1GqkzX--K3jLQ@mail.gmail.com>…
Quoted 3 lines
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.
Quoted 8 lines
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"); /* flushv3/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
PATCH 1/1 arm, davinci: make arch_cpu_init() in da850_lowlevel.c weak
Hello Christian,
Quoted 18 lines · Christian Riesch
Hello Wolfgang,
Quoted 6 lines · Wolfgang Denk
Quoted 3 lines · In message <CABkLObpuzibrEef+8EKKqTgyYcyTeC2qMx2UM1GqkzX--K3jLQ@mail.gmail.com>…
Quoted 2 lines
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.
Quoted 8 lines
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?
Quoted 7 lines
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?
Yes. I prefer actually to remove the old NAND_SPL style complete in this cleanup step, because nobody is using it yet. And if someone use this code for SPL, it must be adapted.
Quoted 28 lines
264 void
board_init_f(ulong bootflag)
265 #else
266 intarch_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"); /* flushv3/v4 cache */
276asm("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");
Quoted 4 lines
291
Heiko, why do we need this? I noticed, that u-boot takes longer to start when I remove this code.
We need it, because we have defined CONFIG_SKIP_LOWLEVEL_INIT (at least for the enbw_cmc board), and this is a copy from arch/arm/cpu/arm926ejs/start.S ... I tend to change arch/arm/cpu/arm926ejs/start.S, so we always execute this code from arch/arm/cpu/arm926ejs/start.S and don't need it here anymore:
arch/arm/cpu/arm926ejs/start.S +2 -2
@@ -353,7 +353,6 @@ _dynsym_start_ofs: * ************************************************************************* */ -#ifndef CONFIG_SKIP_LOWLEVEL_INIT cpu_init_crit: /* * flush v4 I/D caches
@@ -372,14 +371,15 @@ cpu_init_crit: orr r0, r0, #0x00001000 /* set bit 12 (I) I-Cache */ mcr p15, 0, r0, c1, c0, 0 +#ifndef CONFIG_SKIP_LOWLEVEL_INIT /* * Go setup Memory and board specific bits prior to relocation. */ mov ip, lr /* perserve link reg across call */ bl lowlevel_init /* go setup pll,mux,memory */ mov lr, ip /* restore link */ - mov pc, lr /* back to my caller */ #endif /* CONFIG_SKIP_LOWLEVEL_INIT */ + mov pc, lr /* back to my caller */ #ifndef CONFIG_SPL_BUILD /*
Albert, what do you think? Do you see some problems against this?
Quoted 10 lines
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_UNLOCKandHAWKBOARD_KICK1_UNLOCKfor these magic numbers. Maybe we should rename them because they are not HAWKBOARD specific and use them? seeboard/davinci/da8xxevm/hawkboard.c
added this defines to arch/arm/include/asm/arch-davinci/hardware.h
Quoted 8 lines
296 dv_maskbits(&davinci_syscfg_regs->suspsrc,
297 ((1 << 27) | (1 << 22) | (1 << 20) | (1 << 5) |
(1 << 16)));
298This is done in a nicer way in
board/davinci/da8xxevm/da850evm.cI wonder if these settings work for all boards or if any boards wound need different settings here.
Changed this. You need now a CONFIG_SYS_DA850_SYSCFG_SUSPSRC in your board config.
Quoted 22 lines
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);
Quoted 8 lines
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!).
I prefer it this way, but as I said, if we come to the result to use it like the da850evm way, it is ok with me.
Quoted 4 lines
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);
Quoted 8 lines
On my board I need to determine the hardware revision first and then set the values of
CONFIG_SYS_DA850_PLL0_PLLMandCONFIG_SYS_DA850_PLL1_PLLMaccordingly. But maybe I could uses something like#define
CONFIG_SYS_DA850_PLL1_PLLMboard_get_pllm1()and write a function that reads the board revision...
Maybe this is a way to prevent the weak function, yes.
Quoted 6 lines
324
325 /* GPIO setup */
326board_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.
If you don't need this code, do nothing. The weak function for this is a dummy function. Maybe boards will use gpios early, as I use this on the enbw_cmc board.
Quoted 7 lines
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.
We could check if CONFIG_SYS_DA850_CS2CFG/CONFIG_SYS_DA850_CS3CFG is defined, and if not, dont init the register.
Quoted 5 lines
332
lpsc_on(DAVINCI_LPSC_UART2);
333 NS16550_init((NS16550_t)(CONFIG_SYS_NS16550_COM1),
334CONFIG_SYS_NS16550_CLK/ 16 /CONFIG_BAUDRATE);
335
336 /*
Quoted 7 lines
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?
This must be done before using the uart as a console, and I think this should be done in common code as early as possible.
Quoted 10 lines
342 #if
defined(CONFIG_NAND_SPL)
343puts("ddr init\n");
344da850_ddr_setup(132);
345
346puts("boot u-boot ...\n");
347
348nand_boot();
349 #else
350da850_ddr_setup(132);
Quoted 2 lines
Ok for my board. But how about boards that use SDRAM on EMIFA instead (if there are any)?
As no boards use this code except the new enbw_cmc port, there are no such boards ... if somebody want to use this feature, it must be adapted here, yes.
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 arm, davinci: make arch_cpu_init() in da850_lowlevel.c weak
Hello Heiko,
Quoted 12 lines · Heiko Schocher
Quoted 11 lines · Christian Riesch
?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));
Quoted 4 lines
Quoted 1 line
I guess this is only needed for debugging the SPL?
This must be done before using the uart as a console, and I think this should be done in common code as early as possible.
Yes, but it will also be done later in serial_init() called from board_init_f(), right? So this is only a hack to allow debug messages at this stage before it is done properly in serial_init(). Am I missing something here?
Regards, Christian
PATCH 1/1 arm, davinci: make arch_cpu_init() in da850_lowlevel.c weak
In reply to Wolfgang DenkHello Wolfgang, Christian,
Quoted 28 lines · Wolfgang Denk
Dear Christian Riesch,
Quoted 15 lines · In message <[email protected]> you…
This patch allows replacing
arch_cpu_init()if a custom initialization is required.Signed-off-by Christian Riesch <[email protected]>Cc Sandeep Paulraj <[email protected]>Cc Heiko Schocher <[email protected]>--- arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c +0
-0arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c void nand_boot(void) · line 268
Quoted 6 lines
arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c +1
-0@@ -263,6 +263,7 @@ void nand_boot(void)#if defined(CONFIG_NAND_SPL ) void board_init_f(ulong bootflag) #else +__attribute__ ((weak)) int arch_cpu_init(void)#endif Stop please.
This whole code is crap and needs to be changed.
[...]
Quoted 3 lines
Heiko, Christian: please negotiate who performs which part of the cleanup. But I expect that with proper symbolic names instead of the hardwired constants the need for a wek function will go away.
I rework this.
Quoted 2 lines
Albert, please make sure to block this current code. I do not want to see this in mainline as is.
Is it OK, if I send a cleanup patch against current u-boot-arm.git?
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 arm, davinci: make arch_cpu_init() in da850_lowlevel.c weak
Hello Heiko,
Quoted 5 lines · Heiko Schocher
Quoted 3 lines
Heiko, Christian: please negotiate who performs which part of the cleanup. But I expect that with proper symbolic names instead of the hardwired constants the need for a wek function will go away.
I rework this.
I'd like to help but since we have different custom boards it's a bit hard to test. Therefore I'd like to get this lowlevel initialization working on the da850evm board. Since it boots from SPI I guess I'll need an SPL for SPI flash, right? Could we try to clean it up in a way so it nicely integrates with your SPL code in arch/arm/cpu/arm926ejs/davinci/spl.c?
Regards, Christian
PATCH 1/1 arm, davinci: make arch_cpu_init() in da850_lowlevel.c weak
Hello Christian,
Quoted 10 lines · Christian Riesch
Hello Heiko,
Quoted 4 lines · Heiko Schocher
Quoted 3 lines
Heiko, Christian: please negotiate who performs which part of the cleanup. But I expect that with proper symbolic names instead of the hardwired constants the need for a wek function will go away.
I rework this.
I'd like to help but since we have different custom boards it's a bit hard to test. Therefore I'd like to get this lowlevel initialization
Do you plan to post your patches for your custom board to the ML?
Quoted 2 lines
working on the da850evm board. Since it boots from SPI I guess I'll need an SPL for SPI flash, right? Could we try to clean it up in a way
Yes.
Quoted 2 lines
so it nicely integrates with your SPL code in
arch/arm/cpu/arm926ejs/davinci/spl.c?
I soon post the cleanup for the da850_lowlevel.c file, so you can do this based on that patch. You only have to adapt board_init_f() in arch/arm/cpu/arm926ejs/davinci/spl.c. There for the da850 case, arch_cpu_init() or for the spl case better called da850_lowlevel_init() from arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c must be called.
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 arm, davinci: make arch_cpu_init() in da850_lowlevel.c weak
Hello Heiko,
Quoted 11 lines · Heiko Schocher
Quoted 8 lines · Christian Riesch
Quoted 4 lines · Heiko Schocher
Quoted 3 lines
Heiko, Christian: please negotiate who performs which part of the cleanup. But I expect that with proper symbolic names instead of the hardwired constants the need for a wek function will go away.
I rework this.
I'd like to help but since we have different custom boards it's a bit hard to test. Therefore I'd like to get this lowlevel initialization
Do you plan to post your patches for your custom board to the ML?
I'm not sure yet if it makes sense... Of course we will publish the code to comply with GPL once the product development is finished. But I would rather provide the features that I develop for my custom board for the da850evm, so everyone with an evaluation board can test them and adapt them for their own boards.
Quoted 6 lines
Quoted 3 lines
working on the da850evm board. Since it boots from SPI I guess I'll need an SPL for SPI flash, right? Could we try to clean it up in a way
Yes.
Quoted 8 lines
Quoted 2 lines
so it nicely integrates with your SPL code in
arch/arm/cpu/arm926ejs/davinci/spl.c?I soon post the cleanup for the
da850_lowlevel.cfile, so you can do this based on that patch. You only have to adaptboard_init_f()inarch/arm/cpu/arm926ejs/davinci/spl.c. There for the da850 case,arch_cpu_init()or for the spl case better calledda850_lowlevel_init()fromarch/arm/cpu/arm926ejs/davinci/da850_lowlevel.cmust be called.
Great! I'll try to do that.
Thanks for your comments!
Christian