[U-Boot] [PATCH v4] [U-BOOT] Zoom3: Add support for OMAP3630 Zoom3 board.

[Wolfgang Denk]

Dear Aldo Cedillo,

In message <1279231344-16448-1-git-send-email-aldo.cedillo at ti.com> you wrote:
> From: Aldo Brett Cedillo Martinez <aldo.cedillo at ti.com>
> 
> This patch gives basic functionality to OMAP3630 Zoom3 board.
> 
> This version has been rebased againts u-boot-ti
> 
> Signed-off-by: Aldo Brett Cedillo Martinez <aldo.cedillo at ti.com>
> ---
>  MAINTAINERS                        |    4 +
>  MAKEALL                            |    1 +
>  board/logicpd/zoom3/Makefile       |   54 +++++++
>  board/logicpd/zoom3/config.mk      |   33 +++++
>  board/logicpd/zoom3/debug_board.c  |   66 +++++++++
>  board/logicpd/zoom3/led.c          |  133 +++++++++++++++++
>  board/logicpd/zoom3/zoom3.c        |  201 ++++++++++++++++++++++++++
>  board/logicpd/zoom3/zoom3.h        |  164 +++++++++++++++++++++
>  board/logicpd/zoom3/zoom3_serial.c |  132 +++++++++++++++++
>  board/logicpd/zoom3/zoom3_serial.h |   76 ++++++++++
>  boards.cfg                         |    1 +
>  common/serial.c                    |    2 +
>  include/configs/omap3_zoom3.h      |  274 ++++++++++++++++++++++++++++++++++++
>  include/serial.h                   |    7 +
>  14 files changed, 1148 insertions(+), 0 deletions(-)
>  create mode 100644 board/logicpd/zoom3/Makefile
>  create mode 100644 board/logicpd/zoom3/config.mk
>  create mode 100644 board/logicpd/zoom3/debug_board.c
>  create mode 100644 board/logicpd/zoom3/led.c
>  create mode 100644 board/logicpd/zoom3/zoom3.c
>  create mode 100644 board/logicpd/zoom3/zoom3.h
>  create mode 100644 board/logicpd/zoom3/zoom3_serial.c
>  create mode 100644 board/logicpd/zoom3/zoom3_serial.h
>  create mode 100644 include/configs/omap3_zoom3.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1520312..260fda3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -813,6 +813,10 @@ Alex Z
>  	lart		SA1100
>  	dnp1110		SA1110
>  
> +Aldo Brett Cedillo Martinez <aldo.cedillo at ti.com>
> +
> +	omap3_zoom3	ARM CORTEX-A8 (OMAP3xx SoC)
> +

Please keep list sorted. "Martinez" goes between "Lapin" and "Menon",
it seems.


> +static void zoom3_debug_board_detect(void)
> +{
> +	int val = 0;
> +
> +	/*
> +	 * TODO gpio 158 is UART1_RX, should we make a macro of this
> +	 * instead of the raw number?
> +	 */

What a question - yes, you should!

...
> +#include <common.h>
> +#include <netdev.h>
> +#ifdef CONFIG_STATUS_LED
> +#include <status_led.h>
> +#endif

Would it hurt to include <status_led.h> unconditionally?


> +int misc_init_r(void)
> +{
> +	zoom3_identify();
> +	#if (CONFIG_TWL4030_POWER)
> +	twl4030_power_init();
> +	twl4030_led_init(TWL4030_LED_LEDEN_LEDAON | TWL4030_LED_LEDEN_LEDBON);
> +	#endif

Please unindent the preprocessor statements.  Check and fix globally,
please.

...
> +#define CONFIG_SYS_TIMERBASE		(OMAP36XX_GPT2)
> +#define CONFIG_SYS_PTV			7	/* 2^(PTV+1) */
> +#define CONFIG_SYS_HZ			((V_SCLK) / (2 << CONFIG_SYS_PTV))

NAK!!!

CONFIG_SYS_HZ is a constant and must always be 1000.


...
> diff --git a/include/serial.h b/include/serial.h
> index 6513d8e..236e5d1 100644
> --- a/include/serial.h
> +++ b/include/serial.h
> @@ -66,6 +66,13 @@ extern struct serial_device zoom2_serial_device2;
>  extern struct serial_device zoom2_serial_device3;
>  #endif
>  
> +#if defined(CONFIG_OMAP3_ZOOM3)
> +extern struct serial_device zoom3_serial_device0;
> +extern struct serial_device zoom3_serial_device1;
> +extern struct serial_device zoom3_serial_device2;
> +extern struct serial_device zoom3_serial_device3;
> +#endif
> +
>  extern struct serial_device serial_ffuart_device;
>  extern struct serial_device serial_btuart_device;
>  extern struct serial_device serial_stuart_device;

This is becoming a serious mess. Isn't there a way to avoid this?

Best regards,

Wolfgang Denk

-- 
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
People are always a lot more complicated than you  think.  It's  very
important to remember that.             - Terry Pratchett, _Truckers_