[U-Boot] [PATCH] mx28evk: Add initial support for MX28EVK board

[Stefano Babic <sbabic@denx.de>]

On 14/12/2011 20:52, Fabio Estevam wrote:
> Add initial support for Freescale MX28EVK board.
> 
> Tested boot via SD card and by loading a kernel via TFTP through
> the FEC interface.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---

Hi Fabio,

> - Currently MAC addresses are being taken from environment variables:
> set ethaddr xx:xx:xx:xx:xx:xx
> set eth1addr yy:yy:yy:yy:yy:yy
> 
> - For correct operation of saving environment variables into the SD card,
> the following patch is needed:
> http://lists.denx.de/pipermail/u-boot/2011-November/111448.html
> 
>  MAINTAINERS                       |    1 +
>  board/freescale/mx28evk.c         |  190 +++++++++++++++++++++++++++++++++++++

Slipped in wrong directory, I presume

> +
> +#define	HW_DIGCTRL_SCRATCH0	0x8001c280
> +#define	HW_DIGCTRL_SCRATCH1	0x8001c290
> +int dram_init(void)
> +{
> +	uint32_t sz[2];
> +
> +	sz[0] = readl(HW_DIGCTRL_SCRATCH0);
> +	sz[1] = readl(HW_DIGCTRL_SCRATCH1);
> +
> +	if (sz[0] != sz[1]) {
> +		printf("MX28:\n"
> +			"Error, the RAM size in HW_DIGCTRL_SCRATCH0 and\n"
> +			"HW_DIGCTRL_SCRATCH1 is not the same. Please\n"
> +			"verify these two registers contain valid RAM size!\n");

Here we should use puts instead of printf.

> +		hang();
> +	}
> +
> +	gd->ram_size = sz[0];
> +	return 0;
> +}

This function is copied from m28evk.c. Can we factorize it ? Then the
board dram_init() can call the common function if a special
implementation is still needed.

> +#ifdef	CONFIG_CMD_MMC
> +static int mx28evk_mmc_wp(int id)
> +{
> +	if (id != 0) {
> +		printf("MXS MMC: Invalid card selected (card id = %d)\n", id);
> +		return 1;
> +	}
> +
> +	return gpio_get_value(MX28_PAD_SSP1_SCK__GPIO_2_12);
> +}
> +
> +int board_mmc_init(bd_t *bis)
> +{
> +	/* Configure WP as input */
> +	gpio_direction_input(MX28_PAD_SSP1_SCK__GPIO_2_12);
> +
> +	/* Configure MMC0 Power Enable */
> +	gpio_direction_output(MX28_PAD_PWM3__GPIO_3_28, 0);
> +
> +	return mxsmmc_initialize(bis, 0, mx28evk_mmc_wp);
> +}
> +#endif
> +
> +#ifdef	CONFIG_CMD_NET
> +
> +#define	MII_OPMODE_STRAP_OVERRIDE	0x16
> +#define	MII_PHY_CTRL1			0x1e
> +#define	MII_PHY_CTRL2			0x1f
> +
> +int fecmxc_mii_postcall(int phy)
> +{
> +	miiphy_write("FEC1", phy, MII_BMCR, 0x9000);
> +	miiphy_write("FEC1", phy, MII_OPMODE_STRAP_OVERRIDE, 0x0202);
> +	if (phy == 3)
> +		miiphy_write("FEC1", 3, MII_PHY_CTRL2, 0x8180);
> +	return 0;
> +}
> +
> +int board_eth_init(bd_t *bis)
> +{
> +	struct mx28_clkctrl_regs *clkctrl_regs =
> +		(struct mx28_clkctrl_regs *)MXS_CLKCTRL_BASE;
> +	struct eth_device *dev;
> +	int ret;
> +
> +	ret = cpu_eth_init(bis);
> +
> +	/* MX28EVK uses ENET_CLK PAD to drive FEC clock */
> +	writel(CLKCTRL_ENET_TIME_SEL_RMII_CLK | CLKCTRL_ENET_CLK_OUT_EN,
> +					&clkctrl_regs->hw_clkctrl_enet);

Right, I know this issue.

> +	/* Power-on FECs */
> +	gpio_direction_output(MX28_PAD_SSP1_DATA3__GPIO_2_15, 0);
> +
> +	/* Reset FEC PHYs */
> +	gpio_direction_output(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 0);
> +	udelay(200);
> +	gpio_set_value(MX28_PAD_ENET0_RX_CLK__GPIO_4_13, 1);
> +
> +	ret = fecmxc_initialize_multi(bis, 0, 0, MXS_ENET0_BASE);
> +	if (ret) {
> +		printf("FEC MXS: Unable to init FEC0\n");

Use puts, check globally

> +	ret = fecmxc_initialize_multi(bis, 1, 3, MXS_ENET1_BASE);
> +	if (ret) {
> +		printf("FEC MXS: Unable to init FEC1\n");

Ditto

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

Sure imx_get_mac_from_fuse() belong to the processor and not to the
board - it must be moved. If it is not desired to get the MAC address
from the fuses (have Freescale's SOC always a valid MAC in fuses or are
they also delivered with empty MAC ?), you can add a CONFIG_ define to
fall back to the implementation in this patch clearing the MAC.

> diff --git a/include/configs/mx28evk.h b/include/configs/mx28evk.h
> new file mode 100644
> index 0000000..2c8d06b
> --- /dev/null
> +++ b/include/configs/mx28evk.h
> @@ -0,0 +1,164 @@
> +/*
> + * Copyright (C) 2011 Marek Vasut <marek.vasut@gmail.com>
> + * on behalf of DENX Software Engineering GmbH
> + *
> + * 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.
> + */
> +#ifndef __MX28_H__
> +#define __MX28_H__
> +
> +#include <asm/arch/regs-base.h>
> +
> +/*
> + * SoC configurations
> + */
> +#define	CONFIG_MX28				/* i.MX28 SoC */
> +#define	CONFIG_MXS_GPIO				/* GPIO control */
> +#define	CONFIG_SYS_HZ		1000		/* Ticks per second */
> +
> +#define CONFIG_MACH_TYPE	MACH_TYPE_MX28EVK
> +
> +#define	CONFIG_SYS_NO_FLASH
> +#define	CONFIG_SYS_ICACHE_OFF
> +#define	CONFIG_SYS_DCACHE_OFF
> +#define	CONFIG_BOARD_EARLY_INIT_F
> +#define	CONFIG_ARCH_CPU_INIT
> +#define	CONFIG_ARCH_MISC_INIT
> +
> +/*
> + * SPL
> + */
> +#define CONFIG_SPL
> +#define CONFIG_SPL_NO_CPU_SUPPORT_CODE
> +#define CONFIG_SPL_START_S_PATH	"arch/arm/cpu/arm926ejs/mx28"
> +#define CONFIG_SPL_LDSCRIPT	"arch/arm/cpu/arm926ejs/mx28/u-boot-spl.lds"
> +#define CONFIG_SPL_LIBCOMMON_SUPPORT
> +#define CONFIG_SPL_LIBGENERIC_SUPPORT
> +
> +/*
> + * U-Boot Commands
> + */
> +#include <config_cmd_default.h>
> +#define	CONFIG_DISPLAY_CPUINFO
> +#define	CONFIG_DOS_PARTITION
> +#define CONFIG_CMD_FAT
> +
> +#define	CONFIG_CMD_CACHE
> +#define	CONFIG_CMD_DHCP
> +#define	CONFIG_CMD_GPIO
> +#define	CONFIG_CMD_MII
> +#define	CONFIG_CMD_MMC
> +#define	CONFIG_CMD_NET
> +#define	CONFIG_CMD_NFS
> +#define	CONFIG_CMD_PING
> +
> +/*
> + * Memory configurations
> + */
> +#define	CONFIG_NR_DRAM_BANKS		1		/* 1 bank of DRAM */
> +#define	PHYS_SDRAM_1			0x40000000	/* Base address */
> +#define	PHYS_SDRAM_1_SIZE		0x40000000	/* Max 1 GB RAM */
> +#define	CONFIG_STACKSIZE		0x00010000	/* 128 KB stack */
                                                ^-- I read 64KB


> +#define	CONFIG_SYS_MALLOC_LEN		0x00400000	/* 4 MB for malloc */
> +#define	CONFIG_SYS_GBL_DATA_SIZE	128		/* Initial data */
		
Why is not used GENERATED_GBL_DATA_SIZE ? The same question I had to
post for m28evk, I know, but it seems I have not seen there during review.

> +
> +/*
> + * MMC Driver
> + */
> +#define CONFIG_ENV_IS_IN_MMC
> +#define CONFIG_ENV_OFFSET	(256 * 1024)
> +#define CONFIG_ENV_SIZE		(16 * 1024)
> +#define CONFIG_SYS_MMC_ENV_DEV 0
> +#define CONFIG_CMD_SAVEENV
> +#ifdef	CONFIG_CMD_MMC

Should be not CONFIG_ENV_IS_IN_MMC put inside the #ifdef ? We cannot use
CONFIG_ENV_IS_IN_MMC if CONFIG_MMC is not set.

> +/*
> + * Extra Environments
> + */
> +#define CONFIG_EXTRA_ENV_SETTINGS \
> +	"console_fsl=console=ttyAM0" \
> +	"console_mainline=console=ttyAMA0" \

This depends on the tty driver in kernel - as far as I know, it is
ttyAMA. Which is the reason for ttyAM0 ?

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