[U-Boot] [RFC PATCH] Add support for Raidsonic ICYBOX NAS4220 board

Tue Aug 4 21:41:55 CEST 2009

On 19:55 Mon 03 Aug     , Darius Augulis wrote:
> On 07/08/2009 01:29 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 22:03 Tue 30 Jun     , Darius Augulis wrote:
> >> This board is based on Cortina Systems networking processor
> >> CS3516. It has FA526 core, which is ARMv4 compatible.
> >> Many SoC specific definitions may be used for similar
> >> processors CS3512 and dual-core CS3518. This processor
> >> family has Gemini name.
> > do you boot linux?
> 
> yes.
so why don't you define a machine id?
on arm you need to pass it to linux
> 
> >> Signed-off-by: Darius Augulis<augulis.darius at gmail.com>
> >> ---
> >>
> >>   MAINTAINERS                          |    4 +
> >>   MAKEALL                              |    1
> >>   Makefile                             |    3
> >>   board/nas4220/Makefile               |   43 +++++
> >>   board/nas4220/config.mk              |   14 ++
> >>   board/nas4220/lowlevel_init.S        |   96 ++++++++++++
> >>   board/nas4220/nas4220.c              |   75 +++++++++
> >>   board/nas4220/u-boot.lds             |   48 ++++++
> >>   cpu/arm920t/gemini/Makefile          |   38 +++++
> >>   cpu/arm920t/gemini/timer.c           |   93 ++++++++++++
> >>   cpu/arm920t/start.S                  |   11 +
> >>   include/asm-arm/arch-gemini/gemini.h |  271 ++++++++++++++++++++++++++++++++++
> >>   include/configs/nas4220.h            |  116 +++++++++++++++
> >>   13 files changed, 811 insertions(+), 2 deletions(-)
> >>   create mode 100644 board/nas4220/Makefile
> >>   create mode 100644 board/nas4220/config.mk
> >>   create mode 100644 board/nas4220/lowlevel_init.S
> >>   create mode 100644 board/nas4220/nas4220.c
> >>   create mode 100644 board/nas4220/u-boot.lds
> >>   create mode 100644 cpu/arm920t/gemini/Makefile
> >>   create mode 100644 cpu/arm920t/gemini/timer.c
> >>   create mode 100644 include/asm-arm/arch-gemini/gemini.h
> >>   create mode 100644 include/configs/nas4220.h
> >>
> > ditto etc...
> >> +
> >> +	ldr	r3, =GEMINI_GLOBAL_ID		/* Global ID reg */
> >> +	ldr	r4, [r3]
> >> +	ldr	r5, =0xFF			/* Chip revision mask */
> >> +	and	r4, r4, r5
> > please create a function for this and call it
> 
> it is used only single time. why do you recommend to create function?
because it will interesting to have it in the board info and a lot's of time
you will need it in drivers
> 
> >> +	cmp	r4, #0xc0			/* Test if chip rev. is 'c0' */
> >> +	bne	end_prefetch
> >> +
> >> +	/* Fix for rev. 'c0' chip */
> >> +	ldr	r0, =GEMINI_DRAM_AHB_CTRL	/* AHB control */
> >> +	ldr	r5, =GEMINI_WRITE_FLUSH_READ
> >> +	str	r5, [r0]
> >> +
> > <snip>
> >> +
> >> +	ldr	r2, =GEMINI_DRAM_BASE
> >> +	mov	r4, #0xa0
> >> +
> >> +read_loop:
> >> +	ldr	r3, [r2]			/* Read data */
> >> +	subs	r4, r4, #1			/* Decrement loop count */
> >> +	bge	read_loop
> >> +
> >> +	bic	r1, r1, #GEMINI_TRAINING_MODE	/* Disable train mode */
> > what is train mode?
> 
> this is not documented. This piece of code is revers-engineered 
> proprietary Storlink boot loader.
so please put a comment in the code
> 
> >> +	str	r1, [r0]
> >> +
> >> +	mov	pc, lr
> 
> >
> > please create an api such as at91, davinci and other to manage this kind of
> > config
> 
> what api are you talking about? please give some reference.
take a look an at91 in cpu/arm926ejs/at91/
for each soc you have hw init
> 

> >> diff --git a/board/nas4220/u-boot.lds b/board/nas4220/u-boot.lds
> >> new file mode 100644
> >> index 0000000..7d249c3
> >> --- /dev/null
> >> +++ b/board/nas4220/u-boot.lds
> > I do not understand why you need a specific lds?
> 
> because of specific and not software controlled boot procedure of 
> Gemini. Please read my comments below.
this is steal not clear why you need
this
> >> +	{
> >> +		cpu/arm920t/start.o		(.text)
> >> +		board/nas4220/libnas4220.a	(.text)
> >> +		lib_arm/libarm.a		(.text)
> >> +		*(.text)
> >> +	}
and not
	{
		cpu/arm926ejs/start.o	(.text)
		*(.text)
	}
> >> +
> >> +	. = ALIGN(4);
> >> +	.rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
> >> +
<snip>
> 
> >> +
> >> +#endif /* defined (CONFIG_GEMINI) */
> >> diff --git a/cpu/arm920t/start.S b/cpu/arm920t/start.S
> >> index 475cdaf..761753e 100644
> >> --- a/cpu/arm920t/start.S
> >> +++ b/cpu/arm920t/start.S
> >> @@ -115,8 +115,10 @@ start_code:
> >>   	orr	r0,r0,#0xd3
> >>   	msr	cpsr,r0
> >>
> >> -	bl coloured_LED_init
> >> -	bl red_LED_on
> >> +#ifndef CONFIG_GEMINI
> >> +	bl	coloured_LED_init
> >> +	bl	red_LED_on
> >> +#endif
> > no need please remove
> 
> these are linked below (TEXT_BASE + 0x800) address. if called before 
> reallocation, they are not accessible because of Gemini specific boot 
> features.
> 
> >>
> >>   #if	defined(CONFIG_AT91RM9200DK) || defined(CONFIG_AT91RM9200EK)
> >>   	/*
> >> @@ -189,6 +191,11 @@ relocate:				/* relocate U-Boot to RAM	    */
> >>   	sub	r2, r3, r2		/* r2<- size of armboot            */
> >>   	add	r2, r0, r2		/* r2<- source end address         */
> >>
> >> +#ifdef CONFIG_GEMINI
> >> +	orr	r0, r0, #CONFIG_SYS_FLASH_BASE
> >> +	orr	r2, r2, #CONFIG_SYS_FLASH_BASE
> >> +#endif
> > why?
> > and NACK the start.S MUST be generic
> > I'll rework the RM9200 code for this too
> 
> start.S is not generic already.
but it must not become more board specific with ifdef

> and answer is: Gemini does NOT execute code from flash after start up.
> Some hardware copies 2048 bytes of code from flash to internal SRAM memory.
> After that it starts execute code from SRAM. We must use position 
> independent code before reallocation. But start.S calls some functions, 
> linked below (TEXT_BASE + 0x800) address.
> This copy loop copies code from 0x0 base address, which in case of 
> Gemini is not Flash, but SRAM. I must add additional offset to copy code 
> from Flash. Flash physical base is 0x30000000.
> Because of this specific Gemini feature, I must change start.S and 
> create custom linker script.
> NOTE: this is not Faraday FA526 core specific, this is Gemini SoC 
> specific. Other SoCs with FA526 core may use different boot approach.
so it will be better to use a PRELOADER such as we do for nand or onenand
which will boot u-boot

> >> +
> >> +/* NS16550 Configuration */
> >> +#define CONFIG_SYS_NS16550		1
> >> +#define CONFIG_SYS_NS16550_SERIAL	1
> >> +#define CONFIG_SYS_NS16550_REG_SIZE	(-4)
> >> +#define CONFIG_SYS_NS16550_CLK		GEMINI_UART_CLK
> >> +#define CONFIG_SYS_NS16550_COM1		GEMINI_UART_BASE
> >> +#define CONFIG_CONS_INDEX		1
> >> +
> >> +/* Select serial console configuration */
> >> +#define CONFIG_BAUDRATE			115200
> >> +#define CONFIG_SYS_BAUDRATE_TABLE	{ 9600, 19200, 38400, 57600, 115200 }
> >> +
> > please include config_cmd_default
> 
> I don't have networking implemented yet and some of these commands need it.
so just include the config_cmd_default and undef what you do not support

Best Regards,
J.