[U-Boot] [PATCH 1/2 V3] at91sam9261ek: make operational again

Reinhard Meyer
2011-02-18T06:51:42Z

Dear Andreas Bie?mann, Remy Bohmer,
>
> I saw your request for review and have some (little) comments which may be pointed out by Reinhard too.

Thank you for helping me out. I have a few further "recommendations":

>
> Am 05.02.2011 um 13:43 schrieb Remy Bohmer:
>
>> Since U-boot v2010.12 the support for the at91sam9261ek board is broken.
>> This patch solves this issue. This change has been tested on this board.
>>
>> Note: It requires that the 1st stage bootloader (like Atmel at91-bootstrap)
>> to load U-boot at a different address compared to previous releases of
>> U-boot due to conflicts in the BSS area during relocation.
>> (0x23f00000 -> -0x20a00000)

See comment below.

>>
>> Signed-off-by: Remy Bohmer<linux@bohmer.net>
>> ---
>> V3: This patch applied to either mainstream or the u-boot-atmel branch
>> It contains all changes required to make mainstream work again for
>> this board.
>>
>> This patch can be pulled from:
>> git://git.denx.de/u-boot-usb.git -> fix-at91sam9261ek
>>
>> board/atmel/at91sam9261ek/Makefile | 6 +-
>> board/atmel/at91sam9261ek/at91sam9261ek.c | 7 +-
>> board/atmel/at91sam9261ek/config.mk | 1 -
>> include/configs/at91sam9261ek.h | 129 ++++++++++++++++------------
>> 4 files changed, 81 insertions(+), 62 deletions(-)
>> delete mode 100644 board/atmel/at91sam9261ek/config.mk
>>
>> diff --git a/board/atmel/at91sam9261ek/Makefile b/board/atmel/at91sam9261ek/Makefile
>> index 9d20ba0..3351493 100644
>> --- a/board/atmel/at91sam9261ek/Makefile
>> +++ b/board/atmel/at91sam9261ek/Makefile
>
> is OK
>
>> diff --git a/board/atmel/at91sam9261ek/at91sam9261ek.c b/board/atmel/at91sam9261ek/at91sam9261ek.c
>> index de5cfae..25c181e 100644
>> --- a/board/atmel/at91sam9261ek/at91sam9261ek.c
>> +++ b/board/atmel/at91sam9261ek/at91sam9261ek.c
>
> is OK
>
>> diff --git a/board/atmel/at91sam9261ek/config.mk b/board/atmel/at91sam9261ek/config.mk
>> deleted file mode 100644
>> index e554a45..0000000
>
>> diff --git a/include/configs/at91sam9261ek.h b/include/configs/at91sam9261ek.h
>> index 401478b..d601dad 100644
>> --- a/include/configs/at91sam9261ek.h
>> +++ b/include/configs/at91sam9261ek.h
>> @@ -29,34 +29,42 @@
>>
>> #define CONFIG_AT91_LEGACY
>
> will be removed in Patch 2/2, therefore OK
>
>> +/*
>> + * WARNING:
>> + * The initial boot program needs to be adapted such that it loads U-boot
>> + * at the provided TEXT_BASE below. Note that the Atmel AT91-bootstrap loader
>> + * might be configured such that it loads U-boot at 0x23f00000. But since
>> + * U-boot is now being relocated to the end of RAM, this will result in a
>> + * lockup during boot due to an overlap in the BSS segment. So, we choose a
>> + * safe load adress to begin with, namely 0x20a00000
>> + */
>> +#define CONFIG_SYS_TEXT_BASE 0x20a00000

I'd recommend setting this to the very beginning of SDRAM(0x20000000),
note that initial SP and GD would be put into SRAM further down.

>> +
>> /* ARM asynchronous clock */
>> -#define CONFIG_SYS_AT91_MAIN_CLOCK 18432000 /* 18.432 MHz crystal */
>> -#define CONFIG_SYS_HZ 1000
>> +#define CONFIG_SYS_AT91_SLOW_CLOCK 32768 /* slow clock xtal */
>> +#define CONFIG_SYS_AT91_MAIN_CLOCK 18432000 /* main clock xtal */
>> +#define CONFIG_SYS_HZ 1000
>>
>> #define CONFIG_ARM926EJS 1 /* This is an ARM926EJS Core */
>> +/* Define actual evaluation board type from used processor type */
>> #ifdef CONFIG_AT91SAM9G10EK
>> #define CONFIG_AT91SAM9G10 1 /* It's an Atmel AT91SAM9G10 SoC*/
>> #else
>> #define CONFIG_AT91SAM9261 1 /* It's an Atmel AT91SAM9261 SoC*/

That looks to me the other way: its defining the processor type from the
board type. If both boards are identical but for the processor, the processor
should be defined in boards.cfg, like it is for 9260/9xe-ek.

>
> do not define to '1' here
>
>> #endif
>> +#define CONFIG_AT91FAMILY
>> +
>> +/* Misc CPU related */
>> #define CONFIG_ARCH_CPU_INIT
>> #undef CONFIG_USE_IRQ /* we don't need IRQ/FIQ stuff */
>> -
>> -#define CONFIG_CMDLINE_TAG 1 /* enable passing of ATAGs */
>> -#define CONFIG_SETUP_MEMORY_TAGS 1
>> -#define CONFIG_INITRD_TAG 1
>> +#define CONFIG_CMDLINE_TAG /* enable passing of ATAGs */
>> +#define CONFIG_SETUP_MEMORY_TAGS
>> +#define CONFIG_INITRD_TAG
>>
>> #define CONFIG_SKIP_LOWLEVEL_INIT
>>
>> -/*
>> - * Hardware drivers
>> - */
>> -#define CONFIG_AT91_GPIO 1
>> -#define CONFIG_ATMEL_USART 1
>> -#undef CONFIG_USART0
>> -#undef CONFIG_USART1
>> -#undef CONFIG_USART2
>> -#define CONFIG_USART3 1 /* USART 3 is DBGU */
>> +/* general purpose I/O */
>> +#define CONFIG_AT91_GPIO
>>
>> /* LCD */
>> #define CONFIG_LCD 1
>
> no '1' here
>
>> @@ -65,22 +73,31 @@
>> #undef LCD_TEST_PATTERN
>> #define CONFIG_LCD_INFO 1
>
> no '1' here ... fix globally, only define to an numerical value if that value is used in code, see Reinhard's mail '[STATUS: AT91/AVR32]' point '1.'
>
>> #define CONFIG_LCD_INFO_BELOW_LOGO 1
>> -#define CONFIG_SYS_WHITE_ON_BLACK 1
>> +#define CONFIG_SYS_WHITE_ON_BLACK 1
>> #define CONFIG_ATMEL_LCD 1
>> #ifdef CONFIG_AT91SAM9261EK
>> #define CONFIG_ATMEL_LCD_BGR555 1
>> #else
>> -#define CONFIG_AT91SAM9G10_LCD_BASE 0x23E00000 /* LCD is no more in SRAM */
>> +#define CONFIG_AT91SAM9G10_LCD_BASE 0x23E00000 /* LCD is no more in SRAM */
>> #endif
>> #define CONFIG_SYS_CONSOLE_IS_IN_ENV 1
>>
>> +/* serial console */
>> +#define CONFIG_ATMEL_USART
>> +#undef CONFIG_USART0
>> +#undef CONFIG_USART1
>> +#undef CONFIG_USART2
>> +#define CONFIG_USART3 1 /* USART 3 is DBGU */

See my "squash" notice below.

>
> do not undef not defiend values, fix '1'
>
>> +#define CONFIG_BAUDRATE 115200
>> +#define CONFIG_SYS_BAUDRATE_TABLE {115200 , 19200, 38400, 57600, 9600 }
>> +
>> /* LED */
>> #define CONFIG_AT91_LED
>> -#define CONFIG_RED_LED AT91_PIN_PA23 /* this is the power led */
>> -#define CONFIG_GREEN_LED AT91_PIN_PA13 /* this is the user1 led */
>> -#define CONFIG_YELLOW_LED AT91_PIN_PA14 /* this is the user2 led */
>> +#define CONFIG_RED_LED AT91_PIN_PA23 /* the power led */
>> +#define CONFIG_GREEN_LED AT91_PIN_PA13 /* the user1 led */
>> +#define CONFIG_YELLOW_LED AT91_PIN_PA14 /* the user2 led */
>>
>> -#define CONFIG_BOOTDELAY 3
>> +#define CONFIG_BOOTDELAY 3
>>
>> /*
>> * BOOTP options
>> @@ -101,21 +118,28 @@
>> #undef CONFIG_CMD_LOADS
>> #undef CONFIG_CMD_SOURCE
>>
>> -#define CONFIG_CMD_PING 1
>> -#define CONFIG_CMD_DHCP 1
>> -#define CONFIG_CMD_NAND 1
>> -#define CONFIG_CMD_USB 1
>> +#define CONFIG_CMD_PING 1
>> +#define CONFIG_CMD_DHCP 1
>> +#define CONFIG_CMD_NAND 1
>> +#define CONFIG_CMD_USB 1
>> +#define CONFIG_CMD_CACHE 1
>>
>> -/* SDRAM */
>> +/*
>> + * SDRAM: 1 bank, 64 MB
>> + * Initialized before u-boot gets started.
>> + */
>> #define CONFIG_NR_DRAM_BANKS 1
>> -#define PHYS_SDRAM 0x20000000
>> -#define PHYS_SDRAM_SIZE 0x04000000 /* 64 megs */
>> +#define CONFIG_SYS_SDRAM_BASE 0x20000000
>> +#define CONFIG_SYS_SDRAM_SIZE 0x04000000 /* 64 megs */

It is up to you, but (64 << 20) might look "nicer".

>>
>> +/* size in bytes reserved for initial data */
>> +#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_SDRAM_BASE + 0x1000 \
>> + - GENERATED_GBL_DATA_SIZE)

I've put that into SRAM, that facilitates future NOR flash boot variants where SDRAM
would be initialized by low level it...

>> /* DataFlash */
>> #define CONFIG_ATMEL_DATAFLASH_SPI
>> #define CONFIG_HAS_DATAFLASH 1
>> -#define CONFIG_SYS_SPI_WRITE_TOUT (5*CONFIG_SYS_HZ)
>> -#define CONFIG_SYS_MAX_DATAFLASH_BANKS 2
>> +#define CONFIG_SYS_SPI_WRITE_TOUT (5*CONFIG_SYS_HZ)
>> +#define CONFIG_SYS_MAX_DATAFLASH_BANKS 2
>> #define CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS0 0xC0000000 /* CS0 */
>> #define CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS3 0xD0000000 /* CS3 */
>> #define AT91_SPI_CLK 15000000
>> @@ -125,20 +149,18 @@
>> /* NAND flash */
>> #ifdef CONFIG_CMD_NAND
>> #define CONFIG_NAND_ATMEL
>> -#define CONFIG_SYS_MAX_NAND_DEVICE 1
>> -#define CONFIG_SYS_NAND_BASE 0x40000000
>> -#define CONFIG_SYS_NAND_DBW_8 1
>> -/* our ALE is AD22 */
>> -#define CONFIG_SYS_NAND_MASK_ALE (1<< 22)
>> -/* our CLE is AD21 */
>> -#define CONFIG_SYS_NAND_MASK_CLE (1<< 21)
>> -#define CONFIG_SYS_NAND_ENABLE_PIN AT91_PIN_PC14
>> -#define CONFIG_SYS_NAND_READY_PIN AT91_PIN_PC15
>> +#define CONFIG_SYS_MAX_NAND_DEVICE 1
>> +#define CONFIG_SYS_NAND_BASE 0x40000000
>> +#define CONFIG_SYS_NAND_DBW_8
>> +#define CONFIG_SYS_NAND_MASK_ALE (1<< 22)
>> +#define CONFIG_SYS_NAND_MASK_CLE (1<< 21)
>> +#define CONFIG_SYS_NAND_ENABLE_PIN AT91_PIN_PC14
>> +#define CONFIG_SYS_NAND_READY_PIN AT91_PIN_PC15
>>
>> #endif
>>
>> /* NOR flash - no real flash on this board */
>> -#define CONFIG_SYS_NO_FLASH 1
>> +#define CONFIG_SYS_NO_FLASH 1
>>
>> /* Ethernet */
>> #define CONFIG_NET_MULTI 1
>> @@ -155,21 +177,21 @@
>> #define CONFIG_USB_ATMEL
>> #define CONFIG_USB_OHCI_NEW 1
>> #define CONFIG_DOS_PARTITION 1
>> -#define CONFIG_SYS_USB_OHCI_CPU_INIT 1
>> -#define CONFIG_SYS_USB_OHCI_REGS_BASE 0x00500000 /* AT91SAM9261_UHP_BASE */
>> +#define CONFIG_SYS_USB_OHCI_CPU_INIT 1
>> +#define CONFIG_SYS_USB_OHCI_REGS_BASE 0x00500000 /* AT91SAM9261_UHP_BASE */
>> #ifdef CONFIG_AT91SAM9G10EK
>> -#define CONFIG_SYS_USB_OHCI_SLOT_NAME "at91sam9g10"
>> +#define CONFIG_SYS_USB_OHCI_SLOT_NAME "at91sam9g10"
>> #else
>> -#define CONFIG_SYS_USB_OHCI_SLOT_NAME "at91sam9261"
>> +#define CONFIG_SYS_USB_OHCI_SLOT_NAME "at91sam9261"
>> #endif

Could we not use "ATMEL_CPU_NAME" here? Its capitalized,
but I assume that would not matter?

>> #define CONFIG_SYS_USB_OHCI_MAX_ROOT_PORTS 2
>> #define CONFIG_USB_STORAGE 1
>> #define CONFIG_CMD_FAT 1
>>
>> -#define CONFIG_SYS_LOAD_ADDR 0x22000000 /* load address */
>> +#define CONFIG_SYS_LOAD_ADDR 0x22000000 /* load address */
>>
>> -#define CONFIG_SYS_MEMTEST_START PHYS_SDRAM
>> -#define CONFIG_SYS_MEMTEST_END 0x23e00000
>> +#define CONFIG_SYS_MEMTEST_START CONFIG_SYS_SDRAM_BASE
>> +#define CONFIG_SYS_MEMTEST_END 0x23e00000
>>
>> #ifdef CONFIG_SYS_USE_DATAFLASH_CS0
>>
>> @@ -177,7 +199,7 @@
>> #define CONFIG_ENV_IS_IN_DATAFLASH 1
>> #define CONFIG_SYS_MONITOR_BASE (CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS0 + 0x8400)
>> #define CONFIG_ENV_OFFSET 0x4200
>> -#define CONFIG_ENV_ADDR (CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS0 + CONFIG_ENV_OFFSET)
>> +#define CONFIG_ENV_ADDR (CONFIG_SYS_DATAFLASH_LOGIC_ADDR_CS0 + CONFIG_ENV_OFFSET)
>> #define CONFIG_ENV_SIZE 0x4200
>> #define CONFIG_BOOTCOMMAND "cp.b 0xC0042000 0x22000000 0x210000; bootm"
>> #define CONFIG_BOOTARGS "console=ttyS0,115200 " \
>> @@ -202,10 +224,10 @@
>> #else /* CONFIG_SYS_USE_NANDFLASH */
>>
>> /* bootstrap + u-boot + env + linux in nandflash */
>> -#define CONFIG_ENV_IS_IN_NAND 1
>> +#define CONFIG_ENV_IS_IN_NAND 1
>> #define CONFIG_ENV_OFFSET 0x60000
>> #define CONFIG_ENV_OFFSET_REDUND 0x80000
>> -#define CONFIG_ENV_SIZE 0x20000 /* 1 sector = 128 kB */
>> +#define CONFIG_ENV_SIZE 0x20000 /* 1 sector = 128 kB */
>> #define CONFIG_BOOTCOMMAND "nand read 0x22000000 0xA0000 0x200000; bootm"
>> #define CONFIG_BOOTARGS "console=ttyS0,115200 " \
>> "root=/dev/mtdblock5 " \
>> @@ -216,22 +238,19 @@
>>
>> #endif
>>
>> -#define CONFIG_BAUDRATE 115200
>> -#define CONFIG_SYS_BAUDRATE_TABLE {115200 , 19200, 38400, 57600, 9600 }
>> -
>> #define CONFIG_SYS_PROMPT "U-Boot> "
>> #define CONFIG_SYS_CBSIZE 256
>> #define CONFIG_SYS_MAXARGS 16
>> #define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE + sizeof(CONFIG_SYS_PROMPT) + 16)
>> #define CONFIG_SYS_LONGHELP 1
>> -#define CONFIG_CMDLINE_EDITING 1
>> +#define CONFIG_CMDLINE_EDITING 1
>>
>> /*
>> * Size of malloc() pool
>> */
>> #define CONFIG_SYS_MALLOC_LEN ROUND(3 * CONFIG_ENV_SIZE + 128*1024, 0x1000)
>>
>> -#define CONFIG_STACKSIZE (32*1024) /* regular stack */
>> +#define CONFIG_STACKSIZE (32*1024) /* regular stack */
>>
>> #ifdef CONFIG_USE_IRQ
>> #error CONFIG_USE_IRQ not supported
>
> please merge move of at91sam9261 to boards.cfg from Patch 2/2 here, this patch could IMHO still go into v2011.03; Patch 2/2 are changes for u-boot-atmel/rework2011xxx branch ... Reinhard do you agree?

In theory, yes, it could be considered a "bug fix". But whats the point of this double effort? Its easier when
both patches are merged together and applied to top of rework.

Best Regards, Reinhard