Hi Alex,
Please see below for some (delayed) review comments.
On Mon, 17 Mar 2008 11:31:42 +0100 Alex mailinglist@miromico.ch wrote:
I corrected the patch. I tried to run MAKEALL, but it fails, as I don't have all those cross-compilers installed. Am I missing something? Do I need to install them, or what is the correct procedure?
You can run MAKEALL for one particular architecture by doing
CROSS_COMPILE=<target>- ./MAKEALL <arch>
where <arch> can be a major architecture (e.g. ppc, arm, etc.) or a subarchitecture/platform (e.g. 4xx, ARM9, etc.)
diff -Naur old/u-boot-avr32/board/miromico/hammerhead/eth.c new/u-boot-avr32/board/miromico/hammerhead/eth.c --- old/u-boot-avr32/board/miromico/hammerhead/eth.c 1970-01-01 01:00:00.000000000 +0100 +++ new/u-boot-avr32/board/miromico/hammerhead/eth.c 2008-03-14 16:06:35.000000000 +0100
+extern int macb_eth_initialize(int id, void *regs, unsigned int phy_addr);
Hrm...we should really move this to a separate header, but let's wait until the current discussions reach some sort of conclusion.
+#ifdef CONFIG_CMD_NET +void board_eth_initialize(bd_t *bi) +{
- macb_eth_initialize(0, (void *)MACB0_BASE, bi->bi_phy_id[0]);
+} +#endif diff -Naur old/u-boot-avr32/board/miromico/hammerhead/hammerhead.c new/u-boot-avr32/board/miromico/hammerhead/hammerhead.c --- old/u-boot-avr32/board/miromico/hammerhead/hammerhead.c 1970-01-01 01:00:00.000000000 +0100 +++ new/u-boot-avr32/board/miromico/hammerhead/hammerhead.c 2008-03-14 15:34:22.000000000 +0100
- /* Select GCLK3 peripheral function. We'll need it as clock output
* for ethernet PHY. */
There's a bit of whitespace damage here and there, as others have pointed out.
+void board_init_info(void) +{
- gd->bd->bi_phy_id[0] = 0x01;
+}
We should probably get rid of this function. Don't worry about it for now though.
diff -Naur old/u-boot-avr32/board/miromico/hammerhead/u-boot.lds new/u-boot-avr32/board/miromico/hammerhead/u-boot.lds --- old/u-boot-avr32/board/miromico/hammerhead/u-boot.lds 1970-01-01 01:00:00.000000000 +0100 +++ new/u-boot-avr32/board/miromico/hammerhead/u-boot.lds 2008-03-14 14:57:37.000000000 +0100
- . = ALIGN(32);
- __flashprog_start = .;
- .flashprog : {
*(.flashprog)- }
- . = ALIGN(32);
- __flashprog_end = .;
I don't think this block is needed since you're using the CFI flash driver. It should probably be culled from the ATNGW100 linker script too.
diff -Naur old/u-boot-avr32/cpu/at32ap/at32ap700x/gpio.c new/u-boot-avr32/cpu/at32ap/at32ap700x/gpio.c --- old/u-boot-avr32/cpu/at32ap/at32ap700x/gpio.c 2008-03-17 10:23:49.000000000 +0100 +++ new/u-boot-avr32/cpu/at32ap/at32ap700x/gpio.c 2008-03-17 10:10:37.000000000 +0100 @@ -142,3 +142,14 @@ gpio_select_periph_A(GPIO_PIN_PA15, 0); /* DATA3 */ } #endif
+#ifdef CONFIG_HAMMERHEAD +/*
- Hammerhead board uses GCLK3 (Periph A on PB29) as 25MHz clock output
- for ethernet PHY.
- */
+void gpio_enable_gclk3(void) +{
- gpio_select_periph_A(GPIO_PIN_PB29, 0); /* GCLK3 */
+} +#endif
Please drop the #ifdef. --gc-sections will take care of it.
diff -Naur old/u-boot-avr32/cpu/at32ap/cpu.c new/u-boot-avr32/cpu/at32ap/cpu.c --- old/u-boot-avr32/cpu/at32ap/cpu.c 2008-03-17 10:23:49.000000000 +0100 +++ new/u-boot-avr32/cpu/at32ap/cpu.c 2008-03-14 15:43:09.000000000 +0100 @@ -81,6 +81,16 @@ #endif }
+#ifdef CONFIG_HAMMERHEAD +static void gclk_init(void) +{
- /* Hammerhead boards uses GCLK3 as 25MHz output to ethernet PHY */
- /* Enable GCLK3 with no input divider, from OSC0 (crystal) */
- sm_writel( PM_GCCTRL3, SM_BIT(CEN) );
+} +#endif
Hmm...could you turn it into something more generic, like
void gclk_enable(unsigned int id)
and call it from the board code? Then you could get rid of the #ifdefs and let --gc-sections remove it if it turns out to be unused.
@@ -105,6 +115,10 @@ p += CFG_ICACHE_LINESZ) asm volatile("cache %0, 0x02" : "=m"(*p) :: "memory");
Hmm...another piece of code we can get rid of. This is unrelated to your patch though.
+#ifdef CONFIG_HAMMERHEAD
- gclk_init();
+#endif
Please remove this if you follow my suggestion above.
diff -Naur old/u-boot-avr32/cpu/at32ap/sm.h new/u-boot-avr32/cpu/at32ap/sm.h --- old/u-boot-avr32/cpu/at32ap/sm.h 2008-03-17 10:23:49.000000000 +0100 +++ new/u-boot-avr32/cpu/at32ap/sm.h 2008-03-14 15:51:10.000000000 +0100 @@ -21,7 +21,16 @@ #define SM_PM_IMR 0x0048 #define SM_PM_ISR 0x004c #define SM_PM_ICR 0x0050 -#define SM_PM_GCCTRL 0x0060
+#define SM_PM_GCCTRL0 0x0060 +#define SM_PM_GCCTRL1 0x0064 +#define SM_PM_GCCTRL2 0x0068 +#define SM_PM_GCCTRL3 0x006c +#define SM_PM_GCCTRL4 0x0070 +#define SM_PM_GCCTRL5 0x0074 +#define SM_PM_GCCTRL6 0x0078 +#define SM_PM_GCCTRL7 0x007c
Why not
#define SM_PM_GCCTRL(x) (0x0060 + 4 * (x))
perhaps as a separate patch?
diff -Naur old/u-boot-avr32/net/eth.c new/u-boot-avr32/net/eth.c --- old/u-boot-avr32/net/eth.c 2008-03-17 10:23:50.000000000 +0100 +++ new/u-boot-avr32/net/eth.c 2008-03-17 10:11:33.000000000 +0100 @@ -64,6 +64,7 @@ extern int mcffec_initialize(bd_t*); extern int mcdmafec_initialize(bd_t*); extern int at91cap9_eth_initialize(bd_t *); +extern int board_eth_initialize(bd_t *);
#ifdef CONFIG_API extern void (*push_packet)(volatile void *, int); @@ -287,6 +288,9 @@ #if defined(CONFIG_AT91CAP9) at91cap9_eth_initialize(bis); #endif +#if defined(CONFIG_HAMMERHEAD)
- board_eth_initialize(bis);
+#endif
We probably want something more generic-sounding like CONFIG_HAS_BOARD_ETH_INITIALIZE so that boards can migrate over to the new API one by one. But I'll leave it to Ben to decide how to do this.
Haavard