Hi Alex,
Please see below for some (delayed) review comments.
On Mon, 17 Mar 2008 11:31:42 +0100
Alex <[EMAIL PROTECTED]> 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
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users
