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

Darius Augulis
2009-08-04T20:22:52Z

On 08/04/2009 10:41 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> 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

oh yes, I know that. it's only RFC patch, I will finish all similar
stuff later.

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

ok.

>>>> + 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

ok, I've already implemented this.

>
>>>> 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)
> }

Gemini has arm920t (FA526) core.
I need libarm linked above 0x800.

>>>> +
>>>> + . = 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

what could you suggest instead this?

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

you suggest to use two separate boot loaders? this is bad idea.
it's not better for users. they will have problems with firmware upgrade
and similar.
Problem is solved with several code lines in u-boot, and it is working.
Additional package (preloader) increases system complexity and decreases
reliability.
btw, It is not attractive to use u-boot if I need additional loader.

>
>>>> +
>>>> +/* 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.