[U-Boot] [PATCH 4/4] Added initial support for PRTLVT2-based boards.

[Stefano Babic <sbabic@denx.de>]

David Jander wrote:
> Signed-off-by: David Jander <david@protonic.nl>
> ---

Hi David,

in addition to Wolfgang's comments:

> +static u32 system_rev;
> +struct io_board_ctrl *mx51_io_board;

Structure is not used, and probably does not match your board. You
should drop it.

> +#define POUT_HS (PAD_CTL_DRV_HIGH | PAD_CTL_SRE_FAST)
> +#define POUT_MS (PAD_CTL_DRV_MAX | PAD_CTL_SRE_FAST)
> +#define POUT_LS (PAD_CTL_DRV_MEDIUM)
> +#define PIN_HYS (PAD_CTL_HYS_ENABLE)
> +#define PIN_HYS_PULL (PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE | PAD_CTL_PUE_PULL)
> +#define PIN_HYS_KEEP (PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE)
> +#define PIO_OD  (PIN_HYS_PULL | PAD_CTL_22K_PU | PAD_CTL_ODE_OPENDRAIN_ENABLE | PAD_CTL_DRV_MEDIUM)

Consider to put these defines in include/asm/arch-mx51/iomux.h. They
could be usefult for other boards, too.

> +	/* Raise the core frequency to 800MHz */
> +	/* printf("Core at 400 MHz!\n"); */
> +	/* writel(0x1, &mxc_ccm->cacrr); */
> +	writel(0x0, &mxc_ccm->cacrr);

Comment is misleading. Remove what is not needed.

> +	/* Setup other io's */
> +	for(t=0; other_io_conf[t].pin>=0; t++) {

Add spaces when needed.

> +#ifdef BOARD_LATE_INIT

Probably it is better (and in mx51evk, too) to remove the #ifdef,
because we need to initialize the pmic, else some functionalities cannot
work. It is mandatory that power_init is called.

> diff --git a/board/Protonic/prtlvt2/prtlvt2.h b/board/Protonic/prtlvt2/prtlvt2.h
> +#ifndef __BOARD_FREESCALE_MX51_EVK_H__
> +#define __BOARD_FREESCALE_MX51_EVK_H__
> +
> +#ifndef __ASSEMBLY__
> +struct io_board_ctrl {
> +	u16 led_ctrl;		/* 0x00 */
> +	u16 resv1[0x03];
> +	u16 sb_stat;		/* 0x08 */
> +	u16 resv2[0x03];
> +	u16 int_stat;		/* 0x10 */
> +	u16 resv3[0x07];
> +	u16 int_rest;		/* 0x20 */
> +	u16 resv4[0x0B];
> +	u16 int_mask;		/* 0x38 */
> +	u16 resv5[0x03];
> +	u16 id1;		/* 0x40 */
> +	u16 resv6[0x03];
> +	u16 id2;		/* 0x48 */
> +	u16 resv7[0x03];
> +	u16 version;		/* 0x50 */
> +	u16 resv8[0x03];
> +	u16 id3;		/* 0x58 */
> +	u16 resv9[0x03];
> +	u16 sw_reset;		/* 0x60 */
> +};
> +#endif

Is this structure really used ? I have not seen in code. Or does it come
only from mx51evk ? You can remove the whole file, if it is not needed.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================