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

Darius Augulis
2009-08-03T08:02:34Z

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.

>> 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
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 9379c7e..ade43ed 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -671,6 +671,10 @@ Sergey Lapin<slapin@ossfans.org>
>>
>> afeb9260 ARM926EJS (AT91SAM9260 SoC)
>>
>> +Darius Augulis<augulis.darius@gmail.com>
>> +
>> + nas4220 CS3516
>> +
> <snip>

fixed alphabetical order.

>> diff --git a/board/nas4220/config.mk b/board/nas4220/config.mk
>> new file mode 100644
>> index 0000000..5b418ba
>> --- /dev/null
>> +++ b/board/nas4220/config.mk
>> @@ -0,0 +1,14 @@
>> +#
>> +# (c) Copyright 2009
>> +# Linkodas, Inc.
>> +# http://www.linkodas.com
>> +#
>> +# Author: Darius Augulis<daugulis@linkodas.com> <augulis.darius@gmail.com>
>> +#
>> +# 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.
>> +
>> +TEXT_BASE = 0x10400000
>> +LDSCRIPT := $(SRCTREE)/board/$(BOARDDIR)/u-boot.lds
>> diff --git a/board/nas4220/lowlevel_init.S b/board/nas4220/lowlevel_init.S
>> new file mode 100644
>> index 0000000..d51a5a2
>> --- /dev/null
>> +++ b/board/nas4220/lowlevel_init.S
>> @@ -0,0 +1,96 @@
>> +/*
>> + * (c) Copyright 2009
>> + * Linkodas, Inc.
>> + * http://www.linkodas.com
>> + *
>> + * Author: Darius Augulis<daugulis@linkodas.com> <augulis.darius@gmail.com>
>> + *
>> + * 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.
>> + */
>> +
>> +#include<config.h>
>> +#include<version.h>
>> +#include<asm/arch/gemini.h>
>> +
>> +#define DRAM_SET_MODE GEMINI_SET_MODE | GEMINI_MODE_DDRAM | \
>> + GEMINI_CAS_3 | GEMINI_BL_4
>> +#define DRAM_SET_TYPE GEMINI_BUS_32b | GEMINI_RAM_15x10_16x10
>> +
>> +#define DRAM_IOCAP GEMINI_IOCAP_DRAM_CLOCK | GEMINI_IOCAP_DRAM_DATA | \
>> + GEMINI_IOCAP_DRAM_CTRL
>> +#define DRAM_TIMING GEMINI_RRATE_AREF8 | GEMINI_RTIMER(195) | \
>> + GEMINI_TRFC(11) | GEMINI_TRAS(6) | GEMINI_TWR(3) | \
>> + GEMINI_TRP(3) | GEMINI_TRCD(3) | GEMINI_TCAS(3)
>> +#define DRAM_READ_DLL GEMINI_RDLL_BYTE3(8) | GEMINI_RDLL_BYTE2(8) | \
>> + GEMINI_RDLL_BYTE1(8) | GEMINI_RDLL_BYTE0(8)
>> +#define DRAM_WRITE_DLL GEMINI_WDLL(26)
>> +#define DRAM_MEM_CTRL GEMINI_TRAINING_MODE | GEMINI_DQS_N_FALLING | \
>> + GEMINI_BUF_IN_4rd | (1<< 6)
> please move this to config header and add CONFIG_SYS_ in the name

ok.

>> +
>> +.globl lowlevel_init
>> +lowlevel_init:
>> +
>> + /* DRAM init */
>> + ldr r0, =GEMINI_DRAM_TYPE /* DRAM set type */
>> + ldr r1, =DRAM_SET_TYPE /* 32bit, 64 Mbytes total */
>> + str r1, [r0]
> please use write32

ok.

>> +
>> + ldr r0, =GEMINI_DRAM_MODE /* DRAM set mode */
>> + ldr r1, =DRAM_SET_MODE /* DDRAM, CAS 3, Burst 4 */
>> + str r1, [r0]
> 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?

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

>> + str r1, [r0]
>> +
>> + mov pc, lr
>> diff --git a/board/nas4220/nas4220.c b/board/nas4220/nas4220.c
>> new file mode 100644
>> index 0000000..784a249
>> --- /dev/null
>> +++ b/board/nas4220/nas4220.c
>> @@ -0,0 +1,75 @@
>> +/*
>> + * (c) Copyright 2009
>> + * Linkodas, Inc.
>> + * http://www.linkodas.com
>> + *
>> + * Author: Darius Augulis<daugulis@linkodas.com> <augulis.darius@gmail.com>
>> + *
>> + * 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.
>> + */
>> +
>> +#include<common.h>
>> +#include<asm/arch/gemini.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#define UART_PINS (GEMINI_PIN_UART_RX | GEMINI_PIN_UART_TX)
>> +#define MISC_CTRL (GEMINI_PAD_SFLASH_DIS | GEMINI_PAD_NAND_DIS)
>> +
>> +int board_init(void)
>> +{
>> + /* Bypass UART pins */
>> + GEMINI_GPIO_BYPASS(0) |= UART_PINS;
>> +
>> + /* Enable: parallel flash pins, disable: serial, nand pins */
>> + GEMINI_GLOBAL_MISC = MISC_CTRL;
>> +
>> + /* Enable parallel flash direct write mode */
>> + GEMINI_FLASH_PAR_ACCESS |= GEMINI_PFLASH_DWR;
> please use proper acess : writex

ok.

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

>> +
>> + return 0;
>> +}
>> +
>> +int board_late_init(void)
>> +{
>> + int cpu_id, cpu_rev, cpu_clk, ahb_clk;
>> +
>> + cpu_id = GEMINI_CHIP_ID;
>> + cpu_rev = GEMINI_CHIP_REV;
>> + ahb_clk = GEMINI_AHB_CLK;
>> +
>> + switch (GEMINI_CPU_AHB_RATIO) {
>> + case 0:
>> + cpu_clk = ahb_clk;
>> + break;
>> + case 1:
>> + cpu_clk = (ahb_clk * 3) / 2;
>> + break;
>> + case 2:
>> + cpu_clk = (ahb_clk * 24) / 13;
>> + break;
>> + case 3:
>> + cpu_clk = ahb_clk * 2;
>> + break;
>> + default:
>> + cpu_clk = 0;
>> + break;
>> + }
> please create a clk api such as at91
> please take a look on
> cpu/arm926ejs/at91/clock.c
> include/asm-arm/arch-at91/clk.h

ok.

>> +
>> + printf("\nRaidsonic ICYBOX NAS4220 board\n");
>> + printf("CPU: Gemini CS%X, REV: %X\n", cpu_id, cpu_rev);
>> + printf("CPU Speed: %d MHz, AHB Speed: %d MHz, APB Speed: %d MHz\n\n", cpu_clk / 1000000, ahb_clk / 1000000, ahb_clk / 6000000);
> please implement print_cpuinfo
> and checkboard
> and for clock string conversion please use strmhz
>> +
>> + return 0;
>> +}
>> +
>> +int dram_init(void)
>> +{
>> + gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
>> + gd->bd->bi_dram[0].size = PHYS_SDRAM_1_SIZE;
>> +
>> + return 0;
>> +}
>> 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.

>> @@ -0,0 +1,48 @@
>> +/*
>> + * (c) Copyright 2009
>> + * Linkodas, Inc.
>> + * http://www.linkodas.com
>> + *
>> + * Author: Darius Augulis<daugulis@linkodas.com> <augulis.darius@gmail.com>
>> + *
>> + * 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.
>> + */
>> +
>> +OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
>> +OUTPUT_ARCH(arm)
>> +ENTRY(_start)
>> +SECTIONS
>> +{
>> + . = 0x00000000;
>> +
>> + . = ALIGN(4);
>> + .text :
>> + {
>> + cpu/arm920t/start.o (.text)
>> + board/nas4220/libnas4220.a (.text)
>> + lib_arm/libarm.a (.text)
>> + *(.text)
>> + }
>> +
>> + . = ALIGN(4);
>> + .rodata : { *(SORT_BY_ALIGNMENT(SORT_BY_NAME(.rodata*))) }
>> +
>> + . = ALIGN(4);
>> + .data : { *(.data) }
>> +
>> + . = ALIGN(4);
>> + .got : { *(.got) }
>> +
>> + . = .;
>> + __u_boot_cmd_start = .;
>> + .u_boot_cmd : { *(.u_boot_cmd) }
>> + __u_boot_cmd_end = .;
>> +
>> + . = ALIGN(4);
>> + __bss_start = .;
>> + .bss : { *(.bss) . = ALIGN(4); }
>> + _end = .;
>> +}
> <snip>
>> +++ b/cpu/arm920t/gemini/timer.c
>> @@ -0,0 +1,93 @@
>> +/*
>> + * (c) Copyright 2009
>> + * Linkodas, Inc.
>> + * http://www.linkodas.com
>> + *
>> + * Author: Darius Augulis<daugulis@linkodas.com> <augulis.darius@gmail.com>
>> + *
>> + * 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.
>> + */
>> +
>> +#include<common.h>
>> +
>> +#ifdef CONFIG_GEMINI
> no need please remove
>> +
>> +#include<asm/arch/gemini.h>
>> +
>> +static ulong gemini_usec;
>> +static ulong timestamp;
>> +static ulong extraticks;
>> +
>> +int timer_init(void)
>> +{
>> + gemini_usec = GEMINI_APB_CLK / 1000000;
> please clk framework as at91
>> + timestamp = extraticks = 0;
>> +
>> + GEMINI_TIMER_COUNT(0) = 0;
> please use proprer accessor : writex/readx
> etc...

ok.

>> + GEMINI_TIMER_LOAD(0) = 0;
>> + GEMINI_TIMER_CR = TIMER_CR_UP(0) | TIMER_CR_ENABLE(0);
>> +
>> + return 0;
>> +}
>> +
>> +void reset_timer(void)
>> +{
>> + GEMINI_TIMER_CR&= ~TIMER_CR_ENABLE(0);
>> + GEMINI_TIMER_COUNT(0) = 0x0;
>> + GEMINI_TIMER_LOAD(0) = 0x0;
>> + GEMINI_TIMER_CR = TIMER_CR_UP(0) | TIMER_CR_ENABLE(0);
>> +
>> + timestamp = extraticks = 0;
>> +}
>> +
>> +unsigned long long get_usecs(void)
>> +{
>> + ulong timenow = GEMINI_TIMER_COUNT(0);
>> +
>> + if (timenow>= timestamp)
>> + timestamp += timenow - extraticks;
>> + else
>> + timestamp += 0xFFFFFFFF - extraticks + timenow;
>> + extraticks = timenow;
>> +
>> + return timestamp / gemini_usec;
>> +}
>> +
>> +ulong get_timer(ulong base)
>> +{
>> + return (get_usecs() / 1000) - base;
>> +}
>> +
>> +void set_timer(ulong t)
>> +{
>> +}
>> +
>> +void udelay(unsigned long usec)
>> +{
>> + signed long elapsed;
>> + ulong timestart = get_usecs();
>> +
>> + do {
>> + ulong timenow = get_usecs();
>> + elapsed = timenow - timestart;
>> + } while (elapsed< usec);
>> +}
>> +
>> +ulong get_tbclk(void)
>> +{
>> + ulong tbclk;
>> +
>> + tbclk = CONFIG_SYS_HZ;
>> +
>> + return tbclk;
> please retunr CONFIG_SYS_HZ directly

ok.

>> +}
>> +
>> +void reset_cpu(ulong ignored)
>> +{
>> + GEMINI_GLOBAL_RESET = GEMINI_SOFT_RESET | GEMINI_CPU1_RESET;
>> +}
> please move in reset.c or cpu.c

ok.

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

>> +
>> copy_loop:
>> ldmia r0!, {r3-r10} /* copy from source address [r0] */
>> stmia r1!, {r3-r10} /* copy to target address [r1] */
>> diff --git a/include/asm-arm/arch-gemini/gemini.h b/include/asm-arm/arch-gemini/gemini.h
>> new file mode 100644
>> index 0000000..e4fa91a
>> --- /dev/null
>> +++ b/include/asm-arm/arch-gemini/gemini.h
>> @@ -0,0 +1,271 @@
>> +/*
>> + * (c) Copyright 2009
>> + * Linkodas, Inc.
>> + * http://www.linkodas.com
>> + *
>> + * Author: Darius Augulis<daugulis@linkodas.com> <augulis.darius@gmail.com>
>> + *
>> + * 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.
>> + */
>> +
>> +#ifndef __ASSEMBLY__
>> +# define REG8(x) (*(volatile u8 *)(x))
>> +# define REG16(x) (*(volatile u16 *)(x))
>> +# define REG32(x) (*(volatile u32 *)(x))
>> +#else
>> +# define REG8(x) (x)
>> +# define REG16(x) (x)
>> +# define REG32(x) (x)
> NACK please use propoer accessor
> and please split this file by functionnality such as usb, timer etc...
> and keep it here only global information

ok.

>
>> diff --git a/include/configs/nas4220.h b/include/configs/nas4220.h
>> new file mode 100644
>> index 0000000..7f089c6
>> --- /dev/null
>> +++ b/include/configs/nas4220.h
>> @@ -0,0 +1,116 @@
>> +/*
>> + * (c) Copyright 2009
>> + * Linkodas, Inc.
>> + * http://www.linkodas.com
>> + *
>> + * Author: Darius Augulis<daugulis@linkodas.com> <augulis.darius@gmail.com>
>> + *
>> + * 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 is U-boot configuration for Raidsonic ICYBOX NAS4220 board.
>> + * More information about this device is available on websites:
>> + * http://en.nas-4220.org
>> + * http://wiki.gpl-devices.org/wiki/Raidsonic_ICY-BOX_IB-NAS4220-B
>> + */
>> +
>> +#ifndef __CONFIG_H
>> +#define __CONFIG_H
>> +
>> +#include<asm/arch/gemini.h>
>> +#include<asm-arm/sizes.h>
>> +
>> +/* High Level Configuration Options */
>> +#define CONFIG_ARM920T 1
>> +#define CONFIG_GEMINI 1
>> +#define CONFIG_NAS4220 1
>> +#undef CONFIG_USE_IRQ
>> +
>> +/* 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.

>> +/* Command line configuration */
>> +#define CONFIG_CMD_BDI /* bdinfo */
>> +#define CONFIG_CMD_BOOTD /* bootd */
>> +#define CONFIG_CMD_CONSOLE /* coninfo */
>> +#define CONFIG_CMD_ECHO /* echo arguments */
>> +#define CONFIG_CMD_SAVEENV /* saveenv */
>> +#define CONFIG_CMD_FLASH /* flinfo, erase, protect */
>> +#define CONFIG_CMD_IMLS /* List all found images */
>> +#define CONFIG_CMD_ITEST /* Integer (and string) test */
>> +#define CONFIG_CMD_LOADB /* loadb */
>> +#define CONFIG_CMD_LOADS /* loads */
>> +#define CONFIG_CMD_MEMORY /* md mm nm mw cp cmp crc base loop mtest */
>> +#define CONFIG_CMD_MISC /* Misc functions like sleep etc */
>> +#define CONFIG_CMD_RUN /* run command in env variable */
>> +#define CONFIG_CMD_SOURCE /* "source" command support */
>> +#define CONFIG_CMD_XIMG /* Load part of Multi Image */
>> +
> Best Regards,
> J.

Thanks for review.

Best regards,
Darius A.