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

Jean-Christophe PLAGNIOL-VILLARD
2009-08-04T19:41:55Z

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