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

[Reinhard Meyer <u-boot@emk-elektronik.de>]

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