[U-Boot] [PATCH V6 1/3] Initial support for Marvell Orion5x SoC

[Albert ARIBAUD]

(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.

Amicalement,
-- 
Albert.