> -----Original Message----- > From: Albert ARIBAUD [mailto:albert.arib...@free.fr] > Sent: Tuesday, April 13, 2010 4:39 PM > To: Prafulla Wadaskar > Cc: Wolfgang Denk; U-Boot@lists.denx.de > Subject: Re: [U-Boot] [PATCH V6 1/3] Initial support for > Marvell Orion5x SoC > ...snip... > >>>>> +.globl board_lowlevel_init > >>>>> + > >>>>> +board_lowlevel_init: > >>>>> + > >>>>> + /* Use R3 as the base for Device Bus registers */ > >>>>> + add r3, r4, #0x10000 > >>>>> + > >>>>> + /* init MPPs */ > >>>>> + ldr r6, =EDMINIV2_MPP0_7 > >>>>> + str r6, [r3, #0x000] > >>>>> + ldr r6, =EDMINIV2_MPP8_15 > >>>>> + str r6, [r3, #0x004] > >>>>> + ldr r6, =EDMINIV2_MPP16_23 > >>>>> + str r6, [r3, #0x050] > >>>>> + > >>>>> + /* init GPIOs */ > >>>>> + ldr r6, =EDMINIV2_GPIO_OUT_ENABLE > >>>>> + str r6, [r3, #0x104] > >>>>> + > >>>>> + /* Return to lowlevel_init via saved link register */ > >>>>> + mov pc, lr > >>>> Dear Albert > >>>> > >>>> You are just doing mpp and gpio settings here, those are IO > >>>> specific only > >>>> you can have mpp and gpio configs as in case of Kirkwood (c > >>>> functions) and call them from your bard_init. > >>>> Please remove this file. > >>>> and dependency of this code with lowlevel_init.S in patch 1/2 > >>>> > >>>> That's it. > >>> Dear Albert > >>> > >>> Can you pls do the needfull for above and post V7 for this patch? > >> I can, but IMO that would doing C code for the sake of C code. > >> > >> The ED Mini board is a product, not a dev board, and has a > completely > >> static and predefined MPP and GPIO configuration; there will > >> not even be > >> an API to modify them from within U-boot (this was discussed > >> in earlier > >> iterations of the patch) and thus their whole handling > throughout the > >> whole patch takes no more than these eight ASM statements. > >> > >> Unless there is a pressing reason to switch to C, I would > >> prefer to keep > >> MPP+GPIO inits as they are. C code here would not provide > any benefit. > > This is how C evolved :-) > > Hmm... It only did because/when assembly became impractical. :) > > > 1. Using C itself is benefit over assembly. Stragically > prefer C wherever possible. > > 2. Converting mpp and gpio init in c function and putting > them in board_init completely removes this file. patch > becomes simple and smaller. > > 3. cpu/arm926ejs/orion5x/lowlevel_init.S in patch 1/3 will > become independent and simple. > > All right. However: > > > Actually you can pull it to board/LaCie/edminiv2/ since it > has DRAM configuration and that is board specific. > > I have put this as review comment earlier > > And I'd answered it I believe. :) The DRAM-setting code is > SoC specific,
You are right, the code is SoC specific, but the values to be programmed are board specific. if you are interested, Look at cpu/arm926ejs/at91/lowlevel_init.s how it programs SMRDATA. Follow similar arch here too. -Define similar data segment for holding soc_reg_addr and data array -Define macros for SOC register addresses in soc specific header file -Define macros for SOC register values in board specific header file This way we can retain lowlevel_init.S in soc specific folder, whereas you can pass data through board specific macros. > not board specific. Even the guidelines are variant-specific, not > board-specific. To me: Type of DRAM used, how it is interfaced, clock speed- all are board specific. If removed mpp and gpio inits from current file, Lowlevel_init.S mostly represents DRAM configuration here. For a particular board we know what static configuration is required. Those should be configured through lowlevel_init.S I am mapping this with Kirkwood kwbimage.cfg implementation. > > > It is not necessary to use Assembly here. > > Well there it is necessary, because you can't use a C stack yet for > instance, since you don't have RAM initialized yet. That's why lowlevel_init should do basic static initialization in asm (i.e. mostly DRAM configuration) And other initialization in either arch_cpu_init() or arch_misc_init() or board_init(). > > > Our common objective is to add functional, clean, readable > and smaller code to the repository. > > > > I hope you will agree with me. > > Partly: the DRAM init part has to stay in asm and in the SoC code I'm > afraid. Sure, DRAM init should go in lowlevel_init i.e. asm code, that's need of Orion5X Either its location or its architecture need to change to address board specific issues. Regards.. Prafulla . . _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
