Hi Prafulla, Prafulla Wadaskar a écrit : > > >> -----Original Message----- >> From: Albert ARIBAUD [mailto:albert.arib...@free.fr] >> Sent: Monday, April 12, 2010 8:33 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 >> >> (apparently one of Prafulla's answers did not reach me; >> copy-pasting it >> and replying even though it's slightly out of place within the thread) >> >> Prafulla Wadaskar a écrit : >>>> -----Original Message----- >>>> From: u-boot-bounces at lists.denx.de >>>> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of >> Prafulla Wadaskar >>>> Sent: Friday, April 09, 2010 3:08 PM >>>> To: Albert Aribaud; U-Boot at lists.denx.de >>>> Subject: Re: [U-Boot] [PATCH V6 3/3] Add support for the >>>> LaCie ED Mini V2 board >>>> >>>> >>>> >>>>> -----Original Message----- >>>>> From: u-boot-bounces at lists.denx.de >>>>> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of >> Albert Aribaud >>>>> Sent: Friday, April 09, 2010 1:59 PM >>>>> To: U-Boot at lists.denx.de >>>>> Subject: [U-Boot] [PATCH V6 3/3] Add support for the LaCie ED >>>>> Mini V2 board >>>>> >>>>> This patch adds support for the LaCie ED Mini V2 product >>>>> which is based on the Marvell Orion5x SoC. >>>>> --- >>>>> MAINTAINERS | 4 + >>>>> MAKEALL | 1 + >>>>> Makefile | 3 + >>>>> board/LaCie/edminiv2/Makefile | 58 +++++++++++ >>>>> board/LaCie/edminiv2/board_lowlevel_init.S | 59 +++++++++++ >>>>> board/LaCie/edminiv2/config.mk | 27 +++++ >>>>> board/LaCie/edminiv2/edminiv2.c | 93 >>>> ++++++++++++++++++ >>>>> board/LaCie/edminiv2/edminiv2.h | 54 ++++++++++ >>>>> include/configs/edminiv2.h | 147 >>>>> ++++++++++++++++++++++++++++ >>>>> 9 files changed, 446 insertions(+), 0 deletions(-) >>>>> create mode 100644 board/LaCie/edminiv2/Makefile >>>>> create mode 100644 board/LaCie/edminiv2/board_lowlevel_init.S >>>>> create mode 100644 board/LaCie/edminiv2/config.mk >>>>> create mode 100644 board/LaCie/edminiv2/edminiv2.c >>>>> create mode 100644 board/LaCie/edminiv2/edminiv2.h >>>>> create mode 100644 include/configs/edminiv2.h >>>>> >>>> ..snip.. >>>>> diff --git a/board/LaCie/edminiv2/board_lowlevel_init.S >>>>> b/board/LaCie/edminiv2/board_lowlevel_init.S >>>>> new file mode 100644 >>>>> index 0000000..00e68e9 >>>>> --- /dev/null >>>>> +++ b/board/LaCie/edminiv2/board_lowlevel_init.S >>>>> @@ -0,0 +1,59 @@ >>>>> +/* >>>>> + * Copyright (C) 2010 Albert ARIBAUD <albert.aribaud at free.fr> >>>>> + * >>>>> + * (C) Copyright 2009 >>>>> + * Marvell Semiconductor <www.marvell.com> >>>>> + * Written-by: Prafulla Wadaskar <prafulla at marvell.com> >>>>> + * >>>>> + * See file CREDITS for list of people who contributed to this >>>>> + * project. >>>>> + * >>>>> + * This program is free software; you can redistribute it and/or >>>>> + * modify it under the terms of the GNU General Public >> License as >>>>> + * published by the Free Software Foundation; either >> version 2 of >>>>> + * the License, or (at your option) any later version. >>>>> + * >>>>> + * This program is distributed in the hope that it will >> be useful, >>>>> + * but WITHOUT ANY WARRANTY; without even the implied >> warranty of >>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>>>> + * GNU General Public License for more details. >>>>> + * >>>>> + * You should have received a copy of the GNU General >>>> Public License >>>>> + * along with this program; if not, write to the Free Software >>>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, >>>>> + * MA 02110-1301 USA >>>>> + */ >>>>> + >>>>> +#include "edminiv2.h" >>>>> + >>>>> +/* >>>>> + * Low-level init happens right after start.S has switched >>>> to SVC32, >>>>> + * flushed and disabled caches and disabled MMU. We're >>>> still running >>>>> + * from the boot chip select, so the first thing we should >>>> do is set >>>>> + * up RAM for us to relocate into. >>>>> + * >>>>> + * board_low_level_init is called by the Orion5x >>>> lowlevel_init code, >>>>> + * and sets up board-specifics such as MPPs and GPIOs. >>>>> + */ >>>>> + >>>>> +.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, not board specific. Even the guidelines are variant-specific, not board-specific. > 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. > 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. > Regards.. > Prafulla . . Amicalement, -- Albert. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
