[U-Boot] [PATCH V6 1/3] Initial support for Marvell Orion5x SoC
Albert ARIBAUD
albert.aribaud at free.fr
Tue Apr 13 13:08:44 CEST 2010
Hi Prafulla,
Prafulla Wadaskar a écrit :
>
>
>> -----Original Message-----
>> From: Albert ARIBAUD [mailto:albert.aribaud at free.fr]
>> Sent: Monday, April 12, 2010 8:33 PM
>> To: Prafulla Wadaskar
>> Cc: Wolfgang Denk; U-Boot at 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.
More information about the U-Boot
mailing list