Dear Wolfgang Denk Thanks for your review. See my comment below:
On 3/18/2013 2:58 PM, Wolfgang Denk wrote:
Dear Josh Wu, In message <1363342624-2939-1-git-send-email-josh...@atmel.com> you wrote:This patch adds at91sam9n12ek support, it enables: - dbgu - nand with pmecc - spi flash - mmc - lcdIt appears you are adding support for a new board - in this case the Subject: is misleading, plrase fix. The missing entry in MAINTAINERS has already been pointed out. Please also make sure to run your patch through checkpatch - it complains for example "WARNING: line over 80 characters". Please fix these, too. Please also make sure to keep all lists sorted. Bo shen already mentioned this for the Makefile, but this also applies to lists as here: #elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) \ - || defined(CONFIG_AT91SAM9X5) + || defined(CONFIG_AT91SAM9X5) || defined(CONFIG_AT91SAM9N12)
I'll fix them in next version.
+++ b/arch/arm/include/asm/arch-at91/at91sam9n12.h @@ -0,0 +1,126 @@...+/* + * Peripheral identifiers/interrupts. + */ +#define ATMEL_ID_FIQ 0 /* Advanced Interrupt Controller (FIQ) */ +#define ATMEL_ID_SYS 1 /* System Controller Interrupt */ +#define ATMEL_ID_PIOAB 2 /* Parallel I/O Controller A and B */ +#define ATMEL_ID_PIOCD 3 /* Parallel I/O Controller C and D */ +#define ATMEL_ID_FUSE 4 /* FUSE Controller */ +#define ATMEL_ID_USART0 5 /* USART 0 */ +#define ATMEL_ID_USART1 6 /* USART 1 */ +#define ATMEL_ID_USART2 7 /* USART 2 */ +#define ATMEL_ID_USART3 8 /* USART 3 */ +#define ATMEL_ID_TWI0 9 /* Two-Wire Interface 0 */ +#define ATMEL_ID_TWI1 10 /* Two-Wire Interface 1 */ +#define ATMEL_ID_HSMCI 12 /* High Speed Multimedia Card Interface */ +#define ATMEL_ID_SPI0 13 /* Serial Peripheral Interface 0 */ +#define ATMEL_ID_SPI1 14 /* Serial Peripheral Interface 1 */ +#define ATMEL_ID_UART0 15 /* UART 0 */ +#define ATMEL_ID_UART1 16 /* UART 1 */ +#define ATMEL_ID_TC01 17 /* Timer Counter 0, 1, 2, 3, 4 and 5 */ +#define ATMEL_ID_PWM 18 /* Pulse Width Modulation Controller */ +#define ATMEL_ID_ADC 19 /* ADC Controller */ +#define ATMEL_ID_DMAC 20 /* DMA Controller */ +#define ATMEL_ID_UHP 22 /* USB Host */ +#define ATMEL_ID_UDP 23 /* USB Device */ +#define ATMEL_ID_LCDC 25 /* LCD Controller */ +#define ATMEL_ID_SSC 28 /* Synchronous Serial Controller */ +#define ATMEL_ID_TRNG 30 /* True Random Number Generator */ +#define ATMEL_ID_IRQ 31 /* Advanced Interrupt Controller */We already have the very same data in "arch/arm/include/asm/arch-at91/at91sam9x5.h".+/* + * User Peripherals physical base addresses. + */ +#define ATMEL_BASE_SPI0 0xf0000000 +#define ATMEL_BASE_SPI1 0xf0004000 +#define ATMEL_BASE_HSMCI 0xf0008000 +#define ATMEL_BASE_SSC 0xf0010000 +#define ATMEL_BASE_TC012 0xf8008000 +#define ATMEL_BASE_TC345 0xf800c000 +#define ATMEL_BASE_TWI0 0xf8010000 +#define ATMEL_BASE_TWI1 0xf8014000 +#define ATMEL_BASE_USART0 0xf801c000 +#define ATMEL_BASE_USART1 0xf8020000 +#define ATMEL_BASE_USART2 0xf8024000 +#define ATMEL_BASE_USART3 0xf8028000 +#define ATMEL_BASE_PWM 0xf8034000 +#define ATMEL_BASE_LCDC 0xf8038000 +#define ATMEL_BASE_UDP 0xf803c000 +#define ATMEL_BASE_UART0 0xf8040000 +#define ATMEL_BASE_UART1 0xf8044000 +#define ATMEL_BASE_TRNG 0xf8048000 +#define ATMEL_BASE_ADC 0xf804c000Same here.+/* + * System Peripherals physical base addresses. + */ +#define ATMEL_BASE_FUSE 0xffffdc00 +#define ATMEL_BASE_MATRIX 0xffffde00 +#define ATMEL_BASE_PMECC 0xffffe000 +#define ATMEL_BASE_PMERRLOC 0xffffe600 +#define ATMEL_BASE_DDRSDRC 0xffffe800 +#define ATMEL_BASE_SMC 0xffffea00 +#define ATMEL_BASE_DMAC 0xffffec00 +#define ATMEL_BASE_AIC 0xfffff000 +#define ATMEL_BASE_DBGU 0xfffff200 +#define ATMEL_BASE_PIOA 0xfffff400 +#define ATMEL_BASE_PIOB 0xfffff600 +#define ATMEL_BASE_PIOC 0xfffff800 +#define ATMEL_BASE_PIOD 0xfffffa00 +#define ATMEL_BASE_PMC 0xfffffc00 +#define ATMEL_BASE_RSTC 0xfffffe00 +#define ATMEL_BASE_SHDC 0xfffffe10 +#define ATMEL_BASE_PIT 0xfffffe30 +#define ATMEL_BASE_WDT 0xfffffe40 +#define ATMEL_BASE_SCKCR 0xfffffe50 +#define ATMEL_BASE_BSCR 0xfffffe54 +#define ATMEL_BASE_GPBR 0xfffffe60 +#define ATMEL_BASE_RTC 0xfffffeb0And here (except for the newly added ATMEL_BASE_FUSE).
yes, the at91sam9n12 is a subset of 9x5 with little modification. I will reuse the at91sam9x5.h for at91sam9n12.
Oh, these tables are repeated in arch/arm/include/asm/arch-at91/at91sam9x5.h, arch/arm/include/asm/arch-at91/at91sam9rl.h and arch/arm/include/asm/arch-at91/at91sam9rl.h ?
The at91sam9rl.h file are very different with at91sam9x5/9n12.only a few peripherals has same id and offset(fiq, aic, pmc, dbgu, pio), all others are different.
so I think I will just keep the at91sam9rl.h not touched.
And you would add yet another copy of the same? This is not acceptable. Please factor these out into a common header file, so we have only a single copy of these defintiions.
like said in above, I will reuse the at91sam9x5.h for 9n12 chip and keep at91sam9rl.h untouched.
--- a/arch/arm/include/asm/arch-at91/at91sam9x5_matrix.h +++ b/arch/arm/include/asm/arch-at91/at91sam9x5_matrix.h @@ -1,10 +1,10 @@ /* * Matrix-centric header file for the AT91SAM9X5 family * - * Copyright (C) 2012 Atmel Corporation. + * Copyright (C) 2013 Atmel Corporation.You cannot and you must not revert existing copyrights. This must NEVER be done. Instead, update it like that: Copyright (C) 2012-2013 Atmel Corporation.
thank you for the reminding. Now I got it.
--- a/arch/arm/include/asm/arch-at91/hardware.h +++ b/arch/arm/include/asm/arch-at91/hardware.h @@ -39,6 +39,8 @@ # include <asm/arch/at91sam9g45.h> #elif defined(CONFIG_AT91SAM9X5) # include <asm/arch/at91sam9x5.h> +#elif defined(CONFIG_AT91SAM9N12) +# include <asm/arch/at91sam9n12.h> #elif defined(CONFIG_AT91CAP9) # include <asm/arch/at91cap9.h> #elif defined(CONFIG_AT91X40)Another place where lists should be sorted.
sure.
+ lcd_printf("%s\n", U_BOOT_VERSION); + lcd_printf("(C) 2013 ATMEL Corp\n");U-Boot itself is not (C) ATMEL.
I will remove the copyright (C) 2013, and just keep 'ATMEL Corp' to indicate that is ATMEL chips and boards, what do you think?
+ lcd_printf("ATMEL Corp\n");
+#undef CONFIG_CMD_IMI +#undef CONFIG_CMD_LOADSIs there any good reason to disable these?
I will keep these two commands.
Best regards, Wolfgang Denk
Thanks again. Best Regards, Josh Wu _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
