Make the at91sam9263ek compile again.

PATCH New Uli Raich 5 participants 10 messages Feb 21, 2011 – Feb 22, 2011 1 message not in the archive

Subsystems (7)

Arm (32-bit) USB Kbuild GPIO +3 more subsystems

lore

Share & tools

lore

Uli Raich

author

2011-02-21 09:41 UTC #0 lore raw

PATCH 18/18 Make the at91sam9263ek compile again.

This version has been tested on an
armputer-vmax board, which is similar to the at91sam9263ek
but not on the at91sam9263ek board itself.
A new configuration "armputer-vmax_config" has been added. This configuration has been tested on the hardware and is known to work. Further hardware tests for individual options are needed. The LCD screen was not tested.

---
 Makefile                                          |    1 -
 arch/arm/cpu/arm926ejs/at91/at91sam9263_devices.c |    3 +-
 arch/arm/cpu/arm926ejs/at91/clock.c               |    2 +-
 arch/arm/cpu/arm926ejs/at91/cpu.c                 |    2 +-
 arch/arm/cpu/arm926ejs/at91/reset.c               |    2 +-
 arch/arm/cpu/arm926ejs/at91/timer.c               |   10 +-
 arch/arm/include/asm/arch-at91/at91_pio.h         |    6 +-
 arch/arm/include/asm/arch-at91/at91sam9263.h      |  170 +++++++++++----------
 arch/arm/lib/board.c                              |    1 +
 board/atmel/at91sam9263ek/at91sam9263ek.c         |   13 +-
 board/atmel/at91sam9263ek/led.c                   |   19 +--
 boards.cfg                                        |    2 +
 drivers/gpio/at91_gpio.c                          |   44 +++---
 drivers/serial/atmel_usart.c                      |   12 +-
 drivers/spi/atmel_dataflash_spi.c                 |   78 ++++------
 drivers/usb/host/ohci-at91.c                      |    6 +-
 include/configs/at91sam9263ek.h                   |  122 ++++++++-------
 17 files changed, 251 insertions(+), 242 deletions(-)

Makefile +0 -1

index 6133160..0c653fa 100644
@@ -827,7 +827,6 @@ at91sam9263ek_norflash_config \
 at91sam9263ek_norflash_boot_config \
 at91sam9263ek_nandflash_config \
 at91sam9263ek_dataflash_config \
-at91sam9263ek_dataflash_cs0_config \
 at91sam9263ek_config	:	unconfig
 	@mkdir -p $(obj)include
 	@if [ "$(findstring _nandflash,$@)" ] ; then \

arch/arm/cpu/arm926ejs/at91/at91sam9263_devices.c +2 -1

index deda3e5..d58f8e4 100644
@@ -26,9 +26,10 @@
  * MA 02111-1307 USA
  */
 
+#include <config.h>
 #include <common.h>
 #include <asm/arch/hardware.h>
-#include <asm/arch/io.h>
+#include <asm/io.h>
 #include <asm/arch/at91_common.h>
 #include <asm/arch/at91_pmc.h>
 #include <asm/arch/at91_pio.h>

arch/arm/cpu/arm926ejs/at91/clock.c +1 -1

index 608af2c..37b0335 100644
@@ -145,7 +145,7 @@ static u32 at91_pll_rate(u32 freq, u32 reg)
 int at91_clock_init(unsigned long main_clock)
 {
 	unsigned freq, mckr;
-	at91_pmc_t *pmc = (at91_pmc_t *) ATMEL_BASE_PMC;
+	at91_pmc_t *pmc = (at91_pmc_t *) AT91_PMC_BASE;
 #ifndef CONFIG_SYS_AT91_MAIN_CLOCK
 	unsigned tmp;
 	/*

arch/arm/cpu/arm926ejs/at91/cpu.c +1 -1

index c47fb31..df77097 100644
@@ -43,7 +43,7 @@ int arch_cpu_init(void)
 void arch_preboot_os(void)
 {
 	ulong cpiv;
-	at91_pit_t *pit = (at91_pit_t *) ATMEL_BASE_PIT;
+	at91_pit_t *pit = (at91_pit_t *) AT91_PIT_BASE;
 
 	cpiv = AT91_PIT_MR_PIV_MASK(readl(&pit->piir));

arch/arm/cpu/arm926ejs/at91/reset.c +1 -1

index 023719a..22a8cb3 100644
@@ -30,7 +30,7 @@
 /* Reset the cpu by telling the reset controller to do so */
 void reset_cpu(ulong ignored)
 {
-	at91_rstc_t *rstc = (at91_rstc_t *) ATMEL_BASE_RSTC;
+	at91_rstc_t *rstc = (at91_rstc_t *) AT91_RSTC_BASE;
 
 	writel(AT91_RSTC_KEY
 		| AT91_RSTC_CR_PROCRST	/* Processor Reset */

arch/arm/cpu/arm926ejs/at91/timer.c +5 -5

index a087687..38be95a 100644
@@ -70,11 +70,11 @@ static inline unsigned long long usec_to_tick(unsigned long long usec)
  */
 int timer_init(void)
 {
-	at91_pmc_t *pmc = (at91_pmc_t *) ATMEL_BASE_PMC;
-	at91_pit_t *pit = (at91_pit_t *) ATMEL_BASE_PIT;
+        at91_pmc_t *pmc = (at91_pmc_t *) AT91_PMC_BASE;
+	at91_pit_t *pit = (at91_pit_t *) AT91_PIT_BASE;
 
 	/* Enable PITC Clock */
-	writel(1 << ATMEL_ID_SYS, &pmc->pcer);
+	writel(1 << AT91_ID_SYS, &pmc->pcer);
 
 	/* Enable PITC */
 	writel(TIMER_LOAD_VAL | AT91_PIT_MR_EN , &pit->mr);

@@ -90,7 +90,7 @@ int timer_init(void)
  */
 unsigned long long get_ticks(void)
 {
-	at91_pit_t *pit = (at91_pit_t *) ATMEL_BASE_PIT;
+	at91_pit_t *pit = (at91_pit_t *) AT91_PIT_BASE;
 
 	ulong now = readl(&pit->piir);

@@ -109,7 +109,7 @@ void __udelay(unsigned long usec)
 	start = get_ticks();		/* get current timestamp */
 	tmo = usec_to_tick(usec);	/* convert usecs to ticks */
 	while ((get_ticks() - start) < tmo)
-		;			/* loop till time has passed */
+		; 			/* loop till time has passed */
 }
 
 /*

arch/arm/include/asm/arch-at91/at91_pio.h +3 -3

index 0b5bd69..4e0a5cd 100644
@@ -81,14 +81,14 @@ typedef union at91_pio {
 		at91_port_t	pioa;
 		at91_port_t	piob;
 		at91_port_t	pioc;
-	#if (ATMEL_PIO_PORTS > 3)
+	#if (AT91_PIO_PORTS > 3)
 		at91_port_t	piod;
 	#endif
-	#if (ATMEL_PIO_PORTS > 4)
+	#if (AT91_PIO_PORTS > 4)
 		at91_port_t	pioe;
 	#endif
 	} ;
-	at91_port_t port[ATMEL_PIO_PORTS];
+	at91_port_t port[AT91_PIO_PORTS];
 } at91_pio_t;
 
 #ifdef CONFIG_AT91_GPIO

arch/arm/include/asm/arch-at91/at91sam9263.h +88 ~~-82~~

206 lines not on this page · raw message

arch/arm/lib/board.c +1 -0

index c620d2c..ea81378 100644
@@ -232,6 +232,7 @@ void __dram_init_banksize(void)
 {
 	gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
 	gd->bd->bi_dram[0].size =  gd->ram_size;
+
 }
 void dram_init_banksize(void)
 	__attribute__((weak, alias("__dram_init_banksize")));

board/atmel/at91sam9263ek/at91sam9263ek.c +9 -4

index 91efc07..7216022 100644
@@ -32,7 +32,7 @@
 #include <asm/arch/at91_matrix.h>
 #include <asm/arch/at91_pio.h>
 #include <asm/arch/clk.h>
-#include <asm/arch/io.h>
+#include <asm/io.h>
 #include <asm/arch/hardware.h>
 #include <lcd.h>
 #include <atmel_lcdc.h>

@@ -99,7 +99,7 @@ static void at91sam9263ek_macb_hw_init(void)
 {
 	unsigned long 	erstl;
 	at91_pmc_t	*pmc	= (at91_pmc_t *) AT91_PMC_BASE;
-	at91_pio_t	*pio	= (at91_pio_t *) AT91_PIO_BASE;
+	at91_pio_t	*pio	= (at91_pio_t *) AT91_PIOA_BASE;
 	at91_rstc_t	*rstc	= (at91_rstc_t *) AT91_RSTC_BASE;
 	/* Enable clock */
 	writel(1 << AT91SAM9263_ID_EMAC, &pmc->pcer);

@@ -225,8 +225,11 @@ void lcd_show_board_info(void)
 	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++)
 		dram_size += gd->bd->bi_dram[i].size;
 	nand_size = 0;
+
+#ifdef CONFIG_CMD_NAND
 	for (i = 0; i < CONFIG_SYS_MAX_NAND_DEVICE; i++)
 		nand_size += nand_info[i].size;
+#endif
 #ifndef CONFIG_SYS_NO_FLASH
 	flash_size = 0;
 	for (i = 0; i < CONFIG_SYS_MAX_FLASH_BANKS; i++)

@@ -255,6 +258,7 @@ int board_init(void)
 	gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
 
 	at91_serial_hw_init();
+	debug ("Ready for debugging messages\n");
 #ifdef CONFIG_CMD_NAND
 	at91sam9263ek_nand_hw_init();
 #endif

@@ -276,8 +280,9 @@ int board_init(void)
 
 int dram_init(void)
 {
-	gd->bd->bi_dram[0].start = PHYS_SDRAM;
-	gd->bd->bi_dram[0].size = PHYS_SDRAM_SIZE;
+	gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
+	gd->bd->bi_dram[0].size =  CONFIG_SYS_SDRAM_SIZE;
+	gd->ram_size = CONFIG_SYS_SDRAM_SIZE;    // U.R is this the way to do it?
 	return 0;
 }

board/atmel/at91sam9263ek/led.c +8 ~~-11~~

index fa1f05b..b3adc51 100644
@@ -23,25 +23,22 @@
  */
 
 #include <common.h>
-#include <asm/arch/hardware.h>
-#include <asm/arch/at91_pmc.h>
-#include <asm/arch/at91_pio.h>
 #include <asm/arch/gpio.h>
-#include <asm/arch/io.h>
+#include <asm/arch/at91_pmc.h>
 
 void coloured_LED_init(void)
 {
 	/* Enable clock */
 	at91_pmc_t	*pmc	= (at91_pmc_t *) AT91_PMC_BASE;
 
-	writel(1 << AT91SAM9263_ID_PIOB | 1 << AT91SAM9263_ID_PIOCDE,
+	writel(1 << AT91_ID_PIOB | 1 << AT91_ID_PIOCDE,
 		&pmc->pcer);
 
-	at91_set_pio_output(CONFIG_RED_LED, 1);
-	at91_set_pio_output(CONFIG_GREEN_LED, 1);
-	at91_set_pio_output(CONFIG_YELLOW_LED, 1);
+	//	at91_set_pio_output(CONFIG_RED_LED, 1);
+	at91_set_pio_output(CONFIG_GREEN_LED, 0);
+	at91_set_pio_output(CONFIG_YELLOW_LED, 0);
 
-	at91_set_pio_value(CONFIG_RED_LED, 0);
-	at91_set_pio_value(CONFIG_GREEN_LED, 1);
-	at91_set_pio_value(CONFIG_YELLOW_LED, 1);
+	//	at91_set_pio_value(CONFIG_RED_LED, 0);
+	at91_set_pio_value(CONFIG_GREEN_LED, 0);
+	at91_set_pio_value(CONFIG_YELLOW_LED, 0);
 }

boards.cfg +2 -0

index 45c3102..a9a8751 100644
@@ -106,6 +106,8 @@ magnesium                    arm         arm926ejs   imx27lite           logicpd
 omap5912osk                  arm         arm926ejs   -                   ti             omap
 edminiv2                     arm         arm926ejs   -                   LaCie          orion5x
 dkb			     arm         arm926ejs   -                   Marvell        pantheon
+at91sam9263ek_dataflash_cs0  arm	 arm926ejs   at91sam9263ek	 atmel		at91	     at91sam9263ek:AT91SAM9263,SYS_USE_DATAFLASH_CS0
+armputer-vmax		     arm	 arm926ejs   armputer-vmax	 atmel		at91	     armputer-vmax:AT91SAM9263,SYS_USE_DATAFLASH_CS0
 ca9x4_ct_vxp                 arm         armv7       vexpress            armltd
 efikamx                      arm         armv7       efikamx             -              mx5
 mx51evk                      arm         armv7       mx51evk             freescale      mx5

drivers/gpio/at91_gpio.c +22 ~~-22~~

130 lines not on this page · raw message

drivers/serial/atmel_usart.c +6 -6

index e326b2b..5ea2b23 100644
@@ -31,7 +31,7 @@ DECLARE_GLOBAL_DATA_PTR;
 
 void serial_setbrg(void)
 {
-	atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE;
+	atmel_usart3_t *usart = (atmel_usart3_t*)AT91_DBGU_BASE;
 	unsigned long divisor;
 	unsigned long usart_hz;

@@ -40,14 +40,14 @@ void serial_setbrg(void)
 	 * Baud Rate = --------------
 	 *                16 * CD
 	 */
-	usart_hz = get_usart_clk_rate(CONFIG_USART_ID);
+	usart_hz = get_usart_clk_rate(AT91_USART_ID);
 	divisor = (usart_hz / 16 + gd->baudrate / 2) / gd->baudrate;
 	writel(USART3_BF(CD, divisor), &usart->brgr);
 }
 
 int serial_init(void)
 {
-	atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE;
+	atmel_usart3_t *usart = (atmel_usart3_t*)AT91_DBGU_BASE;
 
 	writel(USART3_BIT(RSTRX) | USART3_BIT(RSTTX), &usart->cr);

@@ -66,7 +66,7 @@ int serial_init(void)
 
 void serial_putc(char c)
 {
-	atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE;
+	atmel_usart3_t *usart = (atmel_usart3_t*)AT91_DBGU_BASE;
 
 	if (c == '\n')
 		serial_putc('\r');

@@ -83,7 +83,7 @@ void serial_puts(const char *s)
 
 int serial_getc(void)
 {
-	atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE;
+	atmel_usart3_t *usart = (atmel_usart3_t*)AT91_DBGU_BASE;
 
 	while (!(readl(&usart->csr) & USART3_BIT(RXRDY)))
 		 WATCHDOG_RESET();

@@ -92,6 +92,6 @@ int serial_getc(void)
 
 int serial_tstc(void)
 {
-	atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE;
+	atmel_usart3_t *usart = (atmel_usart3_t*)AT91_DBGU_BASE;
 	return (readl(&usart->csr) & USART3_BIT(RXRDY)) != 0;
 }

drivers/spi/atmel_dataflash_spi.c +32 ~~-46~~

178 lines not on this page · raw message

drivers/usb/host/ohci-at91.c +3 -3

index 9532dd9..e1a3394 100644
@@ -32,7 +32,7 @@
 
 int usb_cpu_init(void)
 {
-	at91_pmc_t *pmc	= (at91_pmc_t *)ATMEL_BASE_PMC;
+	at91_pmc_t *pmc	= (at91_pmc_t *)AT91_PMC_BASE;
 
 #if defined(CONFIG_AT91CAP9) || defined(CONFIG_AT91SAM9260) || \
     defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \

@@ -55,7 +55,7 @@ int usb_cpu_init(void)
 	/* Enable USB host clock. */
 	writel(1 << ATMEL_ID_UHP, &pmc->pcer);
 #ifdef CONFIG_AT91SAM9261
-	writel(ATMEL_PMC_UHP | AT91_PMC_HCK0, &pmc->scer);
+	writel(ATMEl_PMC_UHP | AT91_PMC_HCK0, &pmc->scer);
 #else
 	writel(ATMEL_PMC_UHP, &pmc->scer);
 #endif

@@ -65,7 +65,7 @@ int usb_cpu_init(void)
 
 int usb_cpu_stop(void)
 {
-	at91_pmc_t *pmc	= (at91_pmc_t *)ATMEL_BASE_PMC;
+	at91_pmc_t *pmc	= (at91_pmc_t *)AT91_PMC_BASE;
 
 	/* Disable USB host clock. */
 	writel(1 << ATMEL_ID_UHP, &pmc->pcdr);

include/configs/at91sam9263ek.h +67 ~~-55~~

194 lines not on this page · raw message

Signature · 1 line

1.7.1

Reinhard Meyer

2011-02-21 10:56 UTC #1 lore raw

PATCH 18/18 Make the at91sam9263ek compile again.

Dear Uli Raich,

Quoted 6 lines

This version has been tested on an
armputer-vmax board, which is similar to the at91sam9263ek
but not on the at91sam9263ek board itself.
A new configuration "armputer-vmax_config" has been added. This configuration has been tested on the hardware and is known to work. Further hardware tests for individual options are needed. The LCD screen was not tested.

Sorry, this has to get a full NAK.

1. Please never hijack someone's thread! Your patch has nothing to do with someone's PATCH 18/18.

2. You are undoing all rework efforts with this patch!
The rework was, amongst others, to get rid of all AT91_*, AT91<SoC>_*, AVR* macros and unify them into ATMEL_* macros. And by that also remove the twofold re-wrapping of those defines.

Makefile at91sam9263ek_norflash_config \ · line 830

Quoted 10 lines

Makefile +0 -1

index 6133160..0c653fa 100644
@@ -827,7 +827,6 @@ at91sam9263ek_norflash_config \
  at91sam9263ek_norflash_boot_config \
  at91sam9263ek_nandflash_config \
  at91sam9263ek_dataflash_config \
-at91sam9263ek_dataflash_cs0_config \
  at91sam9263ek_config	:	unconfig

If, all entries have to be moved to boards.cfg

Makefile at91sam9263ek_norflash_config \ · line 831

Quoted 2 lines

Makefile +1 -1

-	at91_pmc_t *pmc = (at91_pmc_t *) ATMEL_BASE_PMC;
+	at91_pmc_t *pmc = (at91_pmc_t *) AT91_PMC_BASE;

ATMEL_BASE_*, ATMEL_ID_* are the new names that are the same names for all ATMEL SoCs, although they might hold different numerical values.

Makefile at91sam9263ek_norflash_config \ · line 832

Quoted 2 lines

Makefile +1 -1

-	at91_pit_t *pit = (at91_pit_t *) ATMEL_BASE_PIT;
+	at91_pit_t *pit = (at91_pit_t *) AT91_PIT_BASE;

and so on...

Makefile

Quoted 4 lines

Makefile +2 -2

-	at91_pmc_t *pmc = (at91_pmc_t *) ATMEL_BASE_PMC;
-	at91_pit_t *pit = (at91_pit_t *) ATMEL_BASE_PIT;
+        at91_pmc_t *pmc = (at91_pmc_t *) AT91_PMC_BASE;
+	at91_pit_t *pit = (at91_pit_t *) AT91_PIT_BASE;

You even re-add white-space errors..

...

Makefile

Quoted 2 lines

Makefile +1 -1

-	atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE;
+	atmel_usart3_t *usart = (atmel_usart3_t*)AT91_DBGU_BASE;

It seems you have not understood the concept of "CONFIG_" macros. They are to allow the designer to use different USARTs, not explicitly only the one in the debug unit...

3. And, please, run your patches through checkpatch.pl

Best Regards,
Reinhard

Uli Raich

author

2011-02-22 10:34 UTC #2 lore raw

PATCH 18/18 Make the at91sam9263ek compile again.

Dear Reinhard,
Yes, the problems were mainly due to my ignorance with respect to git: I thought that rework110218 was just a later version of rework110202, which is clearly not the case. I probably also did not create a new branch, which had the effect that you see 17 patches, which were not mine.
Ok, I think I am slowly improving. I started from scratch and first compiled your at91sam9260ek version, which compiles fine when using the nand flash version but fails on the dataflash version. Then I modified my files, always watching what you had done in the at91sam9260ek version. I think that now I exclusively touch files that are specific to the at91sam9263ek and nothing else. I will still take a few days to check everything before making another attempt to send the patches.
Thanks again for your patience with me.
Uli

Sorry, this has to get a full NAK.

1. Please never hijack someone's thread! Your patch has nothing to do with someone's PATCH 18/18.

It seems you have not understood the concept of "CONFIG_" macros. They are to allow the designer to use different USARTs, not explicitly only the one in the debug unit...

3. And, please, run your patches through checkpatch.pl

Best Regards,
Reinhard

Wolfgang Denk

2011-02-21 10:58 UTC #3 lore raw

PATCH 18/18 Make the at91sam9263ek compile again.

In reply to Uli Raich

Dear Uli Raich,

Quoted 9 lines · In message <[email protected]> you…

This version has been tested on an
armputer-vmax board, which is similar to the at91sam9263ek
but not on the at91sam9263ek board itself.
A new configuration "armputer-vmax_config" has been added. This configuration has been tested on the hardware and is known to work. Further hardware tests for individual options are needed. The LCD screen was not tested.
---

SoB line missing, as I already remaked twice before.

Also, please run your patch through checkpatch and fix the errors:

total: 57 errors, 26 warnings, 961 lines checked

ERROR: "(foo*)" should be "(foo *)"
ERROR: Macros with complex values should be enclosed in parenthesis
ERROR: Missing Signed-off-by: line(s)
ERROR: code indent should use tabs where possible
ERROR: do not use C99 // comments
ERROR: space required after that ',' (ctx:VxV)
ERROR: trailing statements should be on next line
ERROR: trailing whitespace
WARNING: line over 80 characters
WARNING: please, no space before tabs
WARNING: please, no spaces at the start of a line
WARNING: space prohibited between function name and open parenthesis '('

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Overdrawn?  But I still have checks left!

Remy Bohmer

2011-02-21 11:00 UTC #4 lore raw

PATCH 18/18 Make the at91sam9263ek compile again.

In reply to Uli Raich

Hi,

2011/2/21 Uli Raich <[email protected]>:

Quoted 25 lines

---

?Makefile ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| ? ?1 -
?arch/arm/cpu/arm926ejs/at91/at91sam9263_devices.c | ? ?3 +-
?arch/arm/cpu/arm926ejs/at91/clock.c ? ? ? ? ? ? ? | ? ?2 +-
?arch/arm/cpu/arm926ejs/at91/cpu.c ? ? ? ? ? ? ? ? | ? ?2 +-
?arch/arm/cpu/arm926ejs/at91/reset.c ? ? ? ? ? ? ? | ? ?2 +-
?arch/arm/cpu/arm926ejs/at91/timer.c ? ? ? ? ? ? ? | ? 10 +-
?arch/arm/include/asm/arch-at91/at91_pio.h ? ? ? ? | ? ?6 +-
?arch/arm/include/asm/arch-at91/at91sam9263.h ? ? ?| ?170 +++++++++++----------
?arch/arm/lib/board.c ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| ? ?1 +
?board/atmel/at91sam9263ek/at91sam9263ek.c ? ? ? ? | ? 13 +-
?board/atmel/at91sam9263ek/led.c ? ? ? ? ? ? ? ? ? | ? 19 +--
?boards.cfg ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?| ? ?2 +
?drivers/gpio/at91_gpio.c ? ? ? ? ? ? ? ? ? ? ? ? ?| ? 44 +++---
?drivers/serial/atmel_usart.c ? ? ? ? ? ? ? ? ? ? ?| ? 12 +-
?drivers/spi/atmel_dataflash_spi.c ? ? ? ? ? ? ? ? | ? 78 ++++------
?drivers/usb/host/ohci-at91.c ? ? ? ? ? ? ? ? ? ? ?| ? ?6 +-
?include/configs/at91sam9263ek.h ? ? ? ? ? ? ? ? ? | ?122 ++++++++-------

Quoted 1 line

?17 files changed, 251 insertions(+), 242 deletions(-)

I already make some remarks that Reinhard is also likely going to make...

Patch 18/18? Where are the first 17 patches?

You should not revert changes that where made within the rework110218 branch of the atmel tree.
I noticed that you replaced some ATMEL_ defines by the older AT91_ style defines.

You should use the 9260ek board as example, which is in the rework110202 branch of Reinhards tree.
This patch touches too many files and can be done which much less changes on less files.

Quoted 10 lines

Makefile +0 -1

index 6133160..0c653fa 100644
@@ -827,7 +827,6 @@ at91sam9263ek_norflash_config \
 ?at91sam9263ek_norflash_boot_config \
 ?at91sam9263ek_nandflash_config \
 ?at91sam9263ek_dataflash_config \
-at91sam9263ek_dataflash_cs0_config \

?at91sam9263ek_config ? : ? ? ? unconfig

I would expect you would remove all sam9263ek stuff from the Makefile.

arch/arm/cpu/arm926ejs/at91/clock.c static u32 at91_pll_rate(u32 freq, u32 reg) · line 148

Quoted 8 lines

arch/arm/cpu/arm926ejs/at91/clock.c +1 -1

@@ -145,7 +145,7 @@ static u32 at91_pll_rate(u32 freq, u32 reg)
 ?int at91_clock_init(unsigned long main_clock)
 ?{
 ? ? ? ?unsigned freq, mckr;
- ? ? ? at91_pmc_t *pmc = (at91_pmc_t *) ATMEL_BASE_PMC;
+ ? ? ? at91_pmc_t *pmc = (at91_pmc_t *) AT91_PMC_BASE;

ATMEL_BASE type defines are right, AT91_ type defines are wrong. Please fix globally

arch/arm/include/asm/arch-at91/at91sam9263.h

Quoted 8 lines

arch/arm/include/asm/arch-at91/at91sam9263.h +0 -0
@@ -20,118 +20,124 @@
?/*
?* defines to be used in other places
?*/
arch/arm/include/asm/arch-at91/at91sam9263.h +1 -1
-#define CONFIG_ARM926EJS ? ? ? /* ARM926EJS Core */
+//#define CONFIG_ARM926EJS ? ? /* ARM926EJS Core */

No C++ style comments and no dead code please.

arch/arm/lib/board.c void __dram_init_banksize(void) · line 235

Quoted 9 lines

arch/arm/lib/board.c +1 -0

index c620d2c..ea81378 100644
@@ -232,6 +232,7 @@ void __dram_init_banksize(void)
 ?{
 ? ? ? ?gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
 ? ? ? ?gd->bd->bi_dram[0].size = ?gd->ram_size;
+

Why adding an empty line here?

Quoted 8 lines

arch/arm/lib/board.c +0 -0
 }
 ?void dram_init_banksize(void)
 ? ? ? ?__attribute__((weak, alias("__dram_init_banksize")));
board/atmel/at91sam9263ek/at91sam9263ek.c +0 -0
index 91efc07..7216022 100644

board/atmel/at91sam9263ek/at91sam9263ek.c int board_init(void) · line 285

Quoted 8 lines

board/atmel/at91sam9263ek/at91sam9263ek.c +3 -2

@@ -276,8 +280,9 @@ int board_init(void)
 
 ?int dram_init(void)
 ?{
- ? ? ? gd->bd->bi_dram[0].start = PHYS_SDRAM;
- ? ? ? gd->bd->bi_dram[0].size = PHYS_SDRAM_SIZE;
+ ? ? ? gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
+ ? ? ? gd->bd->bi_dram[0].size = ?CONFIG_SYS_SDRAM_SIZE;
+ ? ? ? gd->ram_size = CONFIG_SYS_SDRAM_SIZE; ? ?// U.R is this the way to do it?

Filling gd->bd->bi_dram[0].start and size are not required here. It is likely enough to make dram_init() like this (Including auto-detection of available RAM size)

int dram_init(void)

 {
	/* dram_init must store complete ramsize in gd->ram_size */
	gd->ram_size = get_ram_size((volatile long *)CONFIG_SYS_SDRAM_BASE,
			CONFIG_SYS_SDRAM_SIZE);

return 0;
}

Quoted 15 lines

board/atmel/at91sam9263ek/led.c +0 -0
index fa1f05b..b3adc51 100644
@@ -23,25 +23,22 @@
?*/
?#include <common.h>
board/atmel/at91sam9263ek/led.c +0 -3
-#include <asm/arch/hardware.h>
-#include <asm/arch/at91_pmc.h>
-#include <asm/arch/at91_pio.h>
?#include <asm/arch/gpio.h>
board/atmel/at91sam9263ek/led.c +1 -1
-#include <asm/arch/io.h>
+#include <asm/arch/at91_pmc.h>

Quoted 8 lines

?void coloured_LED_init(void)
?{
? ? ? ?/* Enable clock */
? ? ? ?at91_pmc_t ? ? ?*pmc ? ?= (at91_pmc_t *) AT91_PMC_BASE;
- ? ? ? writel(1 << AT91SAM9263_ID_PIOB | 1 << AT91SAM9263_ID_PIOCDE,
+ ? ? ? writel(1 << AT91_ID_PIOB | 1 << AT91_ID_PIOCDE,
? ? ? ? ? ? ? ?&pmc->pcer);

Enable all the GPIO peripheral clocks in a board_early_init_f() routine. Not here.

Quoted 4 lines

- ? ? ? at91_set_pio_output(CONFIG_RED_LED, 1);
- ? ? ? at91_set_pio_output(CONFIG_GREEN_LED, 1);
- ? ? ? at91_set_pio_output(CONFIG_YELLOW_LED, 1);
+ ? ? ? // ? ? ?at91_set_pio_output(CONFIG_RED_LED, 1);

No C++ style comments please and NO dead code. Fix globally

boards.cfg

Quoted 10 lines

boards.cfg +0 -0
index 45c3102..a9a8751 100644
@@ -106,6 +106,8 @@ magnesium ? ? ? ? ? ? ? ? ? ?arm ? ? ? ? arm926ejs ? imx27lite ? ? ? ? ? logicpd
?omap5912osk ? ? ? ? ? ? ? ? ?arm ? ? ? ? arm926ejs ? - ? ? ? ? ? ? ? ? ? ti ? ? ? ? ? ? omap ?edminiv2 ? ? ? ? ? ? ? ? ? ? arm ? ? ? ? arm926ejs ? - ? ? ? ? ? ? ? ? ? LaCie ? ? ? ? ?orion5x ?dkb ? ? ? ? ? ? ? ? ? ? ? ? arm ? ? ? ? arm926ejs ? - ? ? ? ? ? ? ? ? ? Marvell ? ? ? ?pantheon
boards.cfg +2 -0
+at91sam9263ek_dataflash_cs0 ?arm ? ? ? ?arm926ejs ? at91sam9263ek ? ? ? atmel ? ? ? ? ?at91 ? ? ? ? at91sam9263ek:AT91SAM9263,SYS_USE_DATAFLASH_CS0
+armputer-vmax ? ? ? ? ? ? ? arm ? ? ? ? arm926ejs ? armputer-vmax ? ? ? atmel ? ? ? ? ?at91 ? ? ? ? armputer-vmax:AT91SAM9263,SYS_USE_DATAFLASH_CS0

I would expect more entries here.

drivers/usb/host/ohci-at91.c

Quoted 4 lines

drivers/usb/host/ohci-at91.c +0 -0
index 9532dd9..e1a3394 100644

Please, provide a separate patch for changes in the OHCI drivers. They need to go to the u-boot-usb tree.

drivers/usb/host/ohci-at91.c line 35

Quoted 6 lines

drivers/usb/host/ohci-at91.c +1 -1

@@ -32,7 +32,7 @@
 
 ?int usb_cpu_init(void)
 ?{
- ? ? ? at91_pmc_t *pmc = (at91_pmc_t *)ATMEL_BASE_PMC;
+ ? ? ? at91_pmc_t *pmc = (at91_pmc_t *)AT91_PMC_BASE;

You revert a change from Reinhard here.

Quoted 8 lines

drivers/usb/host/ohci-at91.c +0 -0
 ?#if defined(CONFIG_AT91CAP9) || defined(CONFIG_AT91SAM9260) || \
 ? ? defined(CONFIG_AT91SAM9263) || defined(CONFIG_AT91SAM9G20) || \
@@ -55,7 +55,7 @@ int usb_cpu_init(void)
? ? ? ?/* Enable USB host clock. */
? ? ? ?writel(1 << ATMEL_ID_UHP, &pmc->pcer); ?#ifdef CONFIG_AT91SAM9261
- ? ? ? writel(ATMEL_PMC_UHP | AT91_PMC_HCK0, &pmc->scer);
+ ? ? ? writel(ATMEl_PMC_UHP | AT91_PMC_HCK0, &pmc->scer);

Why are you breaking the AT91SAM9261 SoC here? You should be touching 9263 stuff only.

drivers/usb/host/ohci-at91.c int usb_cpu_init(void) · line 68

Quoted 9 lines

?#else
? ? ? ?writel(ATMEL_PMC_UHP, &pmc->scer);
?#endif

drivers/usb/host/ohci-at91.c +1 -1

@@ -65,7 +65,7 @@ int usb_cpu_init(void)
 
 ?int usb_cpu_stop(void)
 ?{
- ? ? ? at91_pmc_t *pmc = (at91_pmc_t *)ATMEL_BASE_PMC;
+ ? ? ? at91_pmc_t *pmc = (at91_pmc_t *)AT91_PMC_BASE;

Another change that is not needed. Please drop all changes to the ohci-at91.c files.
They do not need to be changed to fix your board.

Kind regards,

Remy

A message that is not in the archive (sent off-list or not yet received): [email protected]

Wolfgang Denk

2011-02-21 12:53 UTC #6 lore raw

[U-Boot] SoC specific driver changes through other custodian's trees?

Dear Reinhard Meyer,

Quoted 8 lines · In message <[email protected]> you wrote:

Quoted 3 lines
Please, provide a separate patch for changes in the OHCI drivers. They need to go to the u-boot-usb tree.
Apart from the fact that no change is required here..
I don't think it would make sense to let ATMEL specific driver patches (serial, net, usb, spi, etc.) go through different trees as long as they do not change common files there (Makefile, general header files etc.).

We split responsibilities (and repositories) by topics. Indeed USB stuff should go through the USB repository. Similar, network stuff should go through the network tree. I2C stuff goes through the I2C repo, etc.

Quoted 3 lines

For the rework effort, it would separate changes to files that inherently must be applied together, like changing header files in arch-at91/avr32 and the required adaptions to the atmel drivers.

Then your patches need to split in an orthogonal way.

Quoted 2 lines

It is natural, of course, that each custodian should have a look at the patch involving their architecture or field and possibly add an "Acked-By:" to it.

Just "having a look" is cartainly not sufficient.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Marriage is the triumph  of  imagination  over  intelligence.  Second
marriage is the triumph of hope over experience.

Reinhard Meyer

2011-02-21 13:35 UTC #7 lore raw

[U-Boot] SoC specific driver changes through other custodian's trees?

Dear Wolfgang Denk,

Quoted 17 lines

Dear Reinhard Meyer,
Quoted 8 lines · In message<[email protected]> you wrote:
Quoted 3 lines
Please, provide a separate patch for changes in the OHCI drivers. They need to go to the u-boot-usb tree.
Apart from the fact that no change is required here..
I don't think it would make sense to let ATMEL specific driver patches (serial, net, usb, spi, etc.) go through different trees as long as they do not change common files there (Makefile, general header files etc.).
We split responsibilities (and repositories) by topics. Indeed USB stuff should go through the USB repository. Similar, network stuff should go through the network tree. I2C stuff goes through the I2C repo, etc.

Quoted 5 lines

Quoted 3 lines
For the rework effort, it would separate changes to files that inherently must be applied together, like changing header files in arch-at91/avr32 and the required adaptions to the atmel drivers.
Then your patches need to split in an orthogonal way.

I'm not sure what that means, but the rework effort won't work when not all required changes are in one tree. And how would, for example, the NET custodian verify a patch when it would not even build when the rework changes to ATMEL header files are not in his tree?

On another note: If I am supposed to be responsible fot ATMEL stuff, that includes IMHO also the drivers that are purely ATMEL specific.

Quoted 6 lines

Quoted 3 lines
It is natural, of course, that each custodian should have a look at the patch involving their architecture or field and possibly add an "Acked-By:" to it.
Just "having a look" is cartainly not sufficient.

Best regards,

Reinhard

Wolfgang Denk

2011-02-21 13:52 UTC #8 lore raw

[U-Boot] SoC specific driver changes through other custodian's trees?

Dear Reinhard Meyer,

Quoted 5 lines · In message <[email protected]> you wrote:

I'm not sure what that means, but the rework effort won't work when not all required changes are in one tree. And how would, for example, the NET custodian verify a patch when it would not even build when the rework changes to ATMEL header files are not in his tree?

Normally you do the core first, so it builds, then add the network and USB and ...

On a patch series you can try Cc: each patch of a seris to a different list of people. You are supposed to include the respective custodian.

Quoted 2 lines

On another note: If I am supposed to be responsible fot ATMEL stuff, that includes IMHO also the drivers that are purely ATMEL specific.

Stuff that affects other areas as well need at least the ACK from the respective custodian. You can ask him for his ACK to take it through your tree, but this requires his explicit agreement and must not taken for granted.

Best regards,

Wolfgang Denk

Signature · 4 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Mistakes are often the stepping stones to utter failure.

Andreas Bießmann

2011-02-21 13:09 UTC #9 lore raw

PATCH 18/18 Make the at91sam9263ek compile again.

In reply to Remy Bohmer

Dear Remy Bohmer,

Am 21.02.2011 12:00, schrieb Remy Bohmer:

Quoted 20 lines

board/atmel/at91sam9263ek/led.c +1 -4

index fa1f05b..b3adc51 100644
@@ -23,25 +23,22 @@
 */
 
 #include <common.h>
-#include <asm/arch/hardware.h>
-#include <asm/arch/at91_pmc.h>
-#include <asm/arch/at91_pio.h>
 #include <asm/arch/gpio.h>
-#include <asm/arch/io.h>
+#include <asm/arch/at91_pmc.h>
 
 void coloured_LED_init(void)
 {
       /* Enable clock */
       at91_pmc_t      *pmc    = (at91_pmc_t *) AT91_PMC_BASE;

Quoted 6 lines

board/atmel/at91sam9263ek/led.c line 35
Quoted 3 lines
board/atmel/at91sam9263ek/led.c +1 -1
-       writel(1 << AT91SAM9263_ID_PIOB | 1 << AT91SAM9263_ID_PIOCDE,
+       writel(1 << AT91_ID_PIOB | 1 << AT91_ID_PIOCDE,
               &pmc->pcer);
Enable all the GPIO peripheral clocks in a board_early_init_f() routine. Not here.

No, that is wrong! board_early_init_f() is too late! coloured_LED stuff is used (currently) in early init stage for debugging. Therefore we need to initialize the clocks here. Nevertheless it should preserve old settings in PCER!

If we need these coloured_LED stuff beside the other LED implementation's is another question.

regards

Andreas Bie?mann

Andreas Bießmann

2011-02-21 13:17 UTC #10 lore raw

PATCH 18/18 Make the at91sam9263ek compile again.

Dear Remy Bohmer,

Am 21.02.2011 14:09, schrieb Andreas Bie?mann:

Quoted 8 lines

Quoted 3 lines
Enable all the GPIO peripheral clocks in a board_early_init_f() routine. Not here.
No, that is wrong! board_early_init_f() is too late! coloured_LED stuff is used (currently) in early init stage for debugging. Therefore we need to initialize the clocks here. Nevertheless it should preserve old settings in PCER!

That was wrong ... I know we do not 'disable' other peripherial clocks here.

regards

Andreas Bie?mann