[U-Boot] [PATCH 1/3] ARM: Add support for EP93XX SoCs

[Tom <Tom.Rix@windriver.com>]

Matthias Kaehlcke wrote:
> Add support for the Cirrus EP93XX platform
> 
> Signed-off-by: Matthias Kaehlcke <matthias@kaehlcke.net>
> ---
>  cpu/arm920t/ep93xx/Makefile          |   56 ++++
>  cpu/arm920t/ep93xx/cpu.c             |   51 +++
>  cpu/arm920t/ep93xx/led.c             |   63 ++++
>  cpu/arm920t/ep93xx/led.h             |   26 ++
>  cpu/arm920t/ep93xx/lowlevel_init.S   |   65 ++++
>  cpu/arm920t/ep93xx/speed.c           |  109 ++++++
>  cpu/arm920t/ep93xx/timer.c           |  157 +++++++++
>  cpu/arm920t/ep93xx/u-boot.lds        |   59 ++++
>  include/asm-arm/arch-ep93xx/ep93xx.h |  596 ++++++++++++++++++++++++++++++++++
>  9 files changed, 1182 insertions(+), 0 deletions(-)
>  create mode 100644 cpu/arm920t/ep93xx/Makefile
>  create mode 100644 cpu/arm920t/ep93xx/cpu.c
>  create mode 100644 cpu/arm920t/ep93xx/led.c
>  create mode 100644 cpu/arm920t/ep93xx/led.h
>  create mode 100644 cpu/arm920t/ep93xx/lowlevel_init.S
>  create mode 100644 cpu/arm920t/ep93xx/speed.c
>  create mode 100644 cpu/arm920t/ep93xx/timer.c
>  create mode 100644 cpu/arm920t/ep93xx/u-boot.lds
>  create mode 100644 include/asm-arm/arch-ep93xx/ep93xx.h

There are build errors that cut this review short.
The review of this patch is fairly complete.
I will review the others on the next revision.

For the ep93xx boards, I saw this error on MAKEALL and building normally

src/u-boot-arm/drivers/net/ep93xx.c:513: undefined reference to `unlikely'
drivers/net/libnet.a(ep93xx.o): In function `ep93xx_miiphy_write'

On regression testing with
MAKEALL arm

There are multiple new warnings about 'ERROR' being redefined
/u-boot-arm/include/dataflash.h:160:1: warning: "ERROR" redefined
In file included from at45.c:22:

> 
> diff --git a/cpu/arm920t/ep93xx/Makefile b/cpu/arm920t/ep93xx/Makefile
> new file mode 100644
> index 0000000..30e12af
> --- /dev/null
> +++ b/cpu/arm920t/ep93xx/Makefile
> @@ -0,0 +1,56 @@
> +#

<snip>

> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <asm/io.h>
> +#include <asm/arch/ep93xx.h>
> +
> +#define GREEN_LED_POS		0x01
> +#define RED_LED_POS		0x02
> +
> +
ws
remove extra space

> +inline void switch_LED_on(uint32_t bit_pos)
> +{
> +	register struct gpio_regs *gpio = (struct gpio_regs *)GPIO_BASE;
> +
> +	writel(readl(&gpio->pedr) | bit_pos, &gpio->pedr);
> +
> +}
> +
> +inline void switch_LED_off(uint32_t bit_pos)
> +{
> +	register struct gpio_regs *gpio = (struct gpio_regs *)GPIO_BASE;
> +

<snip>

> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +extern void red_LED_on(void);
> +extern void red_LED_off(void);
> +extern void green_LED_on(void);
> +extern void green_LED_off(void);

These are defined in include/status_led.h
Please use these and insure that your led implementation is
complaint with status_led.h

> diff --git a/cpu/arm920t/ep93xx/lowlevel_init.S b/cpu/arm920t/ep93xx/lowlevel_init.S
> new file mode 100644
> index 0000000..a20ec89
> --- /dev/null
> +++ b/cpu/arm920t/ep93xx/lowlevel_init.S
> @@ -0,0 +1,65 @@
> +/*
> + * Low-level initialization for EP93xx
> + *
> + * Copyright (C) 2009 Matthias Kaehlcke <matthias@kaehlcke.net>

<snip>

> index 0000000..7e0c26d
> --- /dev/null
> +++ b/cpu/arm920t/ep93xx/speed.c
> @@ -0,0 +1,109 @@
> +/*
> + * Cirrus Logic EP93xx PLL support.
> + *
> + * Copyright (C) 2009 Matthias Kaehlcke <matthias@kaehlcke.net>
> + *
> + * See file CREDITS for list of people who contributed to this project.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE.	See the GNU General Public License
> + * for more details.

ws
remove tab after PURPOSE.

> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 675 Mass Ave, Cambridge, MA 02139, USA.
> + */
> +#include <common.h>
> +#include <asm/arch/ep93xx.h>
> +#include <asm/io.h>
> +#include <div64.h>
> +
> +/*
> + * NOTE: This describes the proper use of this file.
> + *
> + * CONFIG_SYS_CLK_FREQ should be defined as the input frequency of the PLL.
> + *
> + * get_FCLK(), get_HCLK(), get_PCLK() and get_UCLK() return the clock of
> + * the specified bus in HZ.
> + */
> +
> +static char fclk_divisors[] = { 1, 2, 4, 8, 16, 1, 1, 1 };

This is only used in get_FCLK()
The definition should move there.
Also the type in get_FLCK is uint8_t.
The type of the table could be improbed to be uint8_t

> +static char hclk_divisors[] = { 1, 2, 4, 5, 6, 8, 16, 32 };

Similar.

> +static char pclk_divisors[] = { 1, 2, 4, 8 };


> +
> +/*
> + * return the PLL output frequency
> + *

<snip>

> +	return CONFIG_SYS_HZ;
> +}
> diff --git a/cpu/arm920t/ep93xx/u-boot.lds b/cpu/arm920t/ep93xx/u-boot.lds
> new file mode 100644
> index 0000000..76caef3
> --- /dev/null
> +++ b/cpu/arm920t/ep93xx/u-boot.lds
> @@ -0,0 +1,59 @@
> +/*
> + * (C) Copyright 2002
> + * Gary Jennejohn, DENX Software Engineering, <gj@denx.de>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +OUTPUT_FORMAT("elf32-littlearm", "elf32-littlearm", "elf32-littlearm")
> +/*OUTPUT_FORMAT("elf32-arm", "elf32-arm", "elf32-arm")*/

Remove if not needed

> +OUTPUT_ARCH(arm)
> +ENTRY(_start)
> +SECTIONS
> +{
> +	. = 0x00000000;
> +
> +	. = ALIGN(4);
> +	.text      :
> +	{
> +	  cpu/arm920t/start.o	(.text)
> +	  . = 0x1000;
> +	  LONG(0x53555243)
> +	  *(.text)
> +	}

Please add a comment on this magic number
+	  LONG(0x53555243)
And on the offset 0x1000

> +
> +	. = ALIGN(4);
> +	.rodata : { *(.rodata) }
> +
> +	. = ALIGN(4);
> +	.data : { *(.data) }
> +
> +	. = ALIGN(4);
> +	.got : { *(.got) }
> +
> +	. = .;
> +	__u_boot_cmd_start = .;
> +	.u_boot_cmd : { *(.u_boot_cmd) }
> +	__u_boot_cmd_end = .;
> +
> +	. = ALIGN(4);
> +	__bss_start = .;
> +	.bss : { *(.bss) }
> +	_end = .;
> +}
> diff --git a/include/asm-arm/arch-ep93xx/ep93xx.h b/include/asm-arm/arch-ep93xx/ep93xx.h
> new file mode 100644
> index 0000000..74c79f6
> --- /dev/null
> +++ b/include/asm-arm/arch-ep93xx/ep93xx.h
> @@ -0,0 +1,596 @@
> +/* -----------------------------------------------------------------------------
> + * Cirrus Logic EP93xx register definitions.
> + *
> + * Copyright (C) 2009
> + * Matthias Kaehlcke <matthias@kaehlcke.net>
> + *
> + * Copyright (C) 2006
> + * Dominic Rath <Dominic.Rath@gmx.de>
> + *
> + * Copyright (C) 2004, 2005
> + * Cory T. Tusar, Videon Central, Inc., <ctusar@videon-central.com>
> + *
> + * Based in large part on linux/include/asm-arm/arch-ep93xx/regmap.h, which is
> + *
> + * Copyright (C) 2004 Ray Lehtiniemi
> + * Copyright (C) 2003 Cirrus Logic, Inc
> + * Copyright (C) 1999 ARM Limited.
> + *
> + * See file CREDITS for list of people who contributed to this project.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
> + * or FITNESS FOR A PARTICULAR PURPOSE.	 See the GNU General Public License
> + * for more details.
> + *
ws
remove tab after PURPOSE.

> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 675 Mass Ave, Cambridge, MA 02139, USA.
> + */

Tom