[U-Boot] [PATCH] add initial support for bluegiga apx4devkit

[Stefano Babic <sbabic@denx.de>]

On 13/12/2011 15:54, Veli-Pekka Peltola wrote:
> Add initial support for Bluegiga APX4 CoM and development kit.
> 
> Signed-off-by: Veli-Pekka Peltola <veli-pekka.peltola@bluegiga.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---

Hi Veli-Pekka,

>  board/bluegiga/apx4devkit/Makefile     |   45 +++++++
>  board/bluegiga/apx4devkit/apx4devkit.c |  204 ++++++++++++++++++++++++++++++++
>  boards.cfg                             |    1 +
>  include/configs/apx4devkit.h           |  176 +++++++++++++++++++++++++++

You should also update the MAINTAINERS file, adding your name as board
maintainer.

>  4 files changed, 426 insertions(+), 0 deletions(-)
>  create mode 100644 board/bluegiga/apx4devkit/Makefile
>  create mode 100644 board/bluegiga/apx4devkit/apx4devkit.c
>  create mode 100644 include/configs/apx4devkit.h
> 

your board configuration file does not set any SPL constant. I cannot
understand how it works. Only u-boot.bin can be built, and this is only
a part of the system. What about the first boot loader (SPL) ?

Please add also in the commit message which components (Ethernet, MMC,
I2C, SPI,...) you support with your patch.

> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define MUX_CONFIG_GENERIC	(MXS_PAD_3V3 | MXS_PAD_8MA | MXS_PAD_PULLUP)
> +#define MUX_CONFIG_GPMI	(MXS_PAD_3V3 | MXS_PAD_4MA | MXS_PAD_NOPULL)
> +#define MUX_CONFIG_GPMI_12MA	(MXS_PAD_3V3 | MXS_PAD_12MA | MXS_PAD_NOPULL)
> +
> +const iomux_cfg_t iomux_setup[] = {
> +	/* GPMI NAND */
> +	MX28_PAD_GPMI_D00__GPMI_D0 | MUX_CONFIG_GPMI,
> +	MX28_PAD_GPMI_D01__GPMI_D1 | MUX_CONFIG_GPMI,
> +	MX28_PAD_GPMI_D02__GPMI_D2 | MUX_CONFIG_GPMI,
> +	MX28_PAD_GPMI_D03__GPMI_D3 | MUX_CONFIG_GPMI,
> +	MX28_PAD_GPMI_D04__GPMI_D4 | MUX_CONFIG_GPMI,
> +	MX28_PAD_GPMI_D05__GPMI_D5 | MUX_CONFIG_GPMI,
> +	MX28_PAD_GPMI_D06__GPMI_D6 | MUX_CONFIG_GPMI,
> +	MX28_PAD_GPMI_D07__GPMI_D7 | MUX_CONFIG_GPMI,
> +	MX28_PAD_GPMI_ALE__GPMI_ALE | MUX_CONFIG_GPMI,
> +	MX28_PAD_GPMI_CLE__GPMI_CLE | MUX_CONFIG_GPMI,
> +	MX28_PAD_GPMI_CE0N__GPMI_CE0N | MUX_CONFIG_GPMI,
> +	MX28_PAD_GPMI_RDY0__GPMI_READY0 | MUX_CONFIG_GPMI,
> +	MX28_PAD_GPMI_RDN__GPMI_RDN | MUX_CONFIG_GPMI_12MA,
> +	MX28_PAD_GPMI_WRN__GPMI_WRN | MUX_CONFIG_GPMI_12MA,
> +	MX28_PAD_GPMI_RESETN__GPMI_RESETN | MUX_CONFIG_GPMI_12MA,
> +
> +	/* MMC0 */
> +	MX28_PAD_SSP0_DATA0__SSP0_D0 | MUX_CONFIG_GENERIC,
> +	MX28_PAD_SSP0_DATA1__SSP0_D1 | MUX_CONFIG_GENERIC,
> +	MX28_PAD_SSP0_DATA2__SSP0_D2 | MUX_CONFIG_GENERIC,
> +	MX28_PAD_SSP0_DATA3__SSP0_D3 | MUX_CONFIG_GENERIC,
> +	MX28_PAD_SSP0_DATA4__SSP0_D4 | MUX_CONFIG_GENERIC,
> +	MX28_PAD_SSP0_DATA5__SSP0_D5 | MUX_CONFIG_GENERIC,
> +	MX28_PAD_SSP0_DATA6__SSP0_D6 | MUX_CONFIG_GENERIC,
> +	MX28_PAD_SSP0_DATA7__SSP0_D7 | MUX_CONFIG_GENERIC,
> +	MX28_PAD_SSP0_CMD__SSP0_CMD | MUX_CONFIG_GENERIC,
> +	MX28_PAD_SSP0_DETECT__SSP0_CARD_DETECT |
> +		(MXS_PAD_3V3 | MXS_PAD_8MA | MXS_PAD_NOPULL),
> +	MX28_PAD_SSP0_SCK__SSP0_SCK |
> +		(MXS_PAD_3V3 | MXS_PAD_12MA | MXS_PAD_NOPULL),
> +
> +	/* FEC Ethernet */
> +	MX28_PAD_ENET0_MDC__ENET0_MDC | MUX_CONFIG_GENERIC,
> +	MX28_PAD_ENET0_MDIO__ENET0_MDIO | MUX_CONFIG_GENERIC,
> +	MX28_PAD_ENET0_RX_EN__ENET0_RX_EN | MUX_CONFIG_GENERIC,
> +	MX28_PAD_ENET0_TX_EN__ENET0_TX_EN | MUX_CONFIG_GENERIC,
> +	MX28_PAD_ENET0_RXD0__ENET0_RXD0 | MUX_CONFIG_GENERIC,
> +	MX28_PAD_ENET0_RXD1__ENET0_RXD1 | MUX_CONFIG_GENERIC,
> +	MX28_PAD_ENET0_TXD0__ENET0_TXD0 | MUX_CONFIG_GENERIC,
> +	MX28_PAD_ENET0_TXD1__ENET0_TXD1 | MUX_CONFIG_GENERIC,
> +	MX28_PAD_ENET_CLK__CLKCTRL_ENET | MUX_CONFIG_GENERIC,
> +};
> +
> +/*
> + * Functions
> + */
> +int board_early_init_f(void)
> +{
> +	/* Setup only pads needed by U-Boot that are not initialized before */
> +	mxs_iomux_setup_multiple_pads(iomux_setup, ARRAY_SIZE(iomux_setup));

Mmmhh..die Pins are setup in the SPL code, checks in
arch/arm/cpu/arm926ejs/mx28/spl_boot.c.

You are adding a lot of pins that are maybe duplicated, are'nt they ?

> +#ifdef CONFIG_CMD_NET
> +
> +#define MII_PHY_CTRL2	0x1f
> +
> +int fecmxc_mii_postcall(int phy)
> +{
> +	miiphy_write("FEC", 0, MII_PHY_CTRL2, 0x8180);
> +	return 0;
> +}

So you have only one ethernet interface - add also this info in your
commit message.

> +void imx_get_mac_from_fuse(char *mac)
> +{
> +	memset(mac, 0, 6);
> +}

This is wrong, and surely you do not get the address from fuses.

> +#ifdef CONFIG_SERIAL_TAG
> +#define MXS_OCOTP_MAX_TIMEOUT	1000000
> +void get_board_serial(struct tag_serialnr *serialnr)
> +{
> +	struct mx28_ocotp_regs *ocotp_regs =
> +		(struct mx28_ocotp_regs *)MXS_OCOTP_BASE;
> +
> +	serialnr->high = 0;
> +	serialnr->low = 0;
> +
> +	writel(OCOTP_CTRL_RD_BANK_OPEN, &ocotp_regs->hw_ocotp_ctrl_set);
> +
> +	if (mx28_wait_mask_clr(&ocotp_regs->hw_ocotp_ctrl_reg, OCOTP_CTRL_BUSY,
> +				MXS_OCOTP_MAX_TIMEOUT)) {
> +		printf("MXS: Can't get serial number from OCOTP\n");
> +		return;
> +	}

I think we can factorize some code with m28evk, and putting code to
access OCOTP in a common directory, such as arch/arm/cpu/arm926ejs/mx28.

> +#ifdef CONFIG_REVISION_TAG
> +u32 get_board_rev(void)
> +{
> +	if (getenv("revision#") != NULL)
> +		return simple_strtoul(getenv("revision#"), NULL, 10);
> +	return 0;
> +}

Do you get the revision from an environment and not from hardware ?
There is a big chance to have the wrong revision number...

> +/* Environment is in NAND */
> +#define CONFIG_ENV_IS_IN_NAND
> +#define CONFIG_ENV_SECT_SIZE		(128 * 1024)
> +#define CONFIG_ENV_SIZE		(128 * 1024)
> +#define CONFIG_ENV_SIZE_REDUND		CONFIG_ENV_SIZE
> +#define CONFIG_ENV_OFFSET		0x120000
> +#define CONFIG_ENV_OFFSET_REDUND	\
> +		(CONFIG_ENV_OFFSET + CONFIG_ENV_SECT_SIZE)

You set the environment in NAND, but you reserve only one sector for
each of the two copies. If the single sector becomes bad, you lose the
redundancy. Should you not have more as one sector for the environment ?

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
=====================================================================