Hello Christian,
Christian Riesch wrote:
> Hello Wolfgang,
>
> On Wed, Nov 9, 2011 at 11:44 AM, Wolfgang Denk <w...@denx.de> wrote:
>> In message
>> <cabklobpuzibreef+8ekkqtgyycytec2qmx2um1gqkzx--k3...@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?
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.
> 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.
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:
diff --git a/arch/arm/cpu/arm926ejs/start.S b/arch/arm/cpu/arm926ejs/start.S
index 339c5ed..73ceb30 100644
--- a/arch/arm/cpu/arm926ejs/start.S
+++ b/arch/arm/cpu/arm926ejs/start.S
@@ -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?
>
> 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
added this defines to arch/arm/include/asm/arch-davinci/hardware.h
> 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.
Changed this. You need now a CONFIG_SYS_DA850_SYSCFG_SUSPSRC
in your board config.
>
> 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!).
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.
> 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...
Maybe this is a way to prevent the weak function, yes.
> 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.
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.
> 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.
> 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?
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.
> 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)?
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
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
