[U-Boot] [U-BOOT][PATCH 5/5] Add initial support for Freescale mx53evk board

[Stefano Babic]

Jason Liu wrote:
> The patch adds initial support for the Freescale mx53evk board.
> SD/MMC boot is supported
> 
> Signed-off-by:Jason Liu <r64343 at freescale.com>

Hi Jason,


> diff --git a/arch/arm/include/asm/arch-mx53/sys_proto.h b/arch/arm/include/asm/arch-mx53/sys_proto.h

Identical to arch-mx51/sys_proto.h Do not add copies of current files.


> diff --git a/board/freescale/mx53evk/Makefile b/board/freescale/mx53evk/Makefile
> new file mode 100644
> index 0000000..3da299f
> --- /dev/null
> +++ b/board/freescale/mx53evk/Makefile
> @@ -0,0 +1,49 @@
> +#
> +# Copyright (C) 2007, Guennadi Liakhovetski <lg at denx.de>
> +#
> +# (C) Copyright 2009 Freescale Semiconductor, Inc.
> +#
> +# 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
> +#
> +
> +include $(TOPDIR)/config.mk
> +
> +LIB	= $(obj)lib$(BOARD).a
> +
> +COBJS	:= mx53evk.o
> +SOBJS	:= flash_header.o 

As I said, it was decided to use a different approach to set the DCD
table. Linking the table directly to u-boot will not be accepted.

> diff --git a/board/freescale/mx53evk/mx53evk.c b/board/freescale/mx53evk/mx53evk.c
> new file mode 100644
> index 0000000..0723cdf
> --- /dev/null
> +++ b/board/freescale/mx53evk/mx53evk.c
> @@ -0,0 +1,411 @@
> +/*
> + * (C) Copyright 2010 Freescale Semiconductor, Inc.
> + *
> + * 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
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <asm/arch/imx-regs.h>
> +#include <asm/arch/mx53_pins.h>
> +#include <asm/arch/sys_proto.h>
> +#include <asm/arch/iomux.h>
> +#include <asm/errno.h>
> +#include <asm/arch/crm_regs.h>
> +#include <i2c.h>
> +#include <mmc.h>
> +#include <fsl_esdhc.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +static u32 system_rev;
> +
> +#ifdef CONFIG_FSL_ESDHC
> +struct fsl_esdhc_cfg esdhc_cfg[2] = {
> +	{MMC_SDHC1_BASE_ADDR, 1},
> +	{MMC_SDHC3_BASE_ADDR, 1},

The structure is not correct. There are changes in fsl_esdhc and the
structure contains only one field (the address). Please fix it (check
with the code in mx51evk.c)

> +u32 inline get_board_rev(void)
> +{
> +
> +	return (system_rev >> 8) & 0xF;
> +}
> +

Can you explain where is the revision coming ? system_rev contains the
CPU version, because is is assigne via get_cpu_rev(). Are we assuming
boad revision = cpu revision ? On mx51evk only a cpu revision is
printed, because I had no idea where I could ge a board version.

> +inline void set_board_rev(int rev)
> +{
> +        system_rev |= (rev & 0xF) << 8;
> +}

This seems dead code. Please drop it.

> +
> +static void setup_iomux_uart(void)
> +{
> +	 /* UART1 RXD */
> +	mxc_request_iomux(MX53_PIN_CSI0_D11, IOMUX_CONFIG_ALT2);
> +	mxc_iomux_set_pad(MX53_PIN_CSI0_D11, 0x1E4);

Could you use or add some constants to better explain what you are doing
here ?  If I am not wrong, this should be substituted by:

	mxc_iomux_set_pad(MX53_PIN_CSI0_D11,
	 PAD_CTL_HYS_ENABLE | PAD_CTL_DRV_HIGH | PAD_CTL_PUE_PULL |
	 PAD_CTL_100K_PU);

Probably I used the wrong constants, but as sure it is easier to
understand as simply 0x1E4. Can you fix globally ?

> +static void setup_iomux_fec(void)
> +{
> +	/*FEC_MDIO*/
> +	mxc_request_iomux(MX53_PIN_FEC_MDIO, IOMUX_CONFIG_ALT0);
> +	mxc_iomux_set_pad(MX53_PIN_FEC_MDIO, 0x1FC);
> +	mxc_iomux_set_input(MUX_IN_FEC_FEC_MDI_SELECT_INPUT, 0x1);
> +

Ditto. Please use constants.

> +#ifdef CONFIG_MXC_SPI

You do not set CONFIG_MXC_SPI, so it seems this is dead code. Drop it if
unnecessary, you can introduce it when SPI is required.

> +void spi_io_init(struct imx_spi_dev_t *dev)
> +{
> +	switch (dev->base) {
> +	case CSPI1_BASE_ADDR:
> +		/* Select mux mode: ALT4 mux port: MOSI of instance: ecspi1 */
> +		mxc_request_iomux(MX53_PIN_EIM_D18, IOMUX_CONFIG_ALT4);
> +		mxc_iomux_set_pad(MX53_PIN_EIM_D18, 0x104);
> +		mxc_iomux_set_input(
> +				MUX_IN_ECSPI1_IPP_IND_MOSI_SELECT_INPUT, 0x3);
> +
> +		/* Select mux mode: ALT4 mux port: MISO of instance: ecspi1. */
> +		mxc_request_iomux(MX53_PIN_EIM_D17, IOMUX_CONFIG_ALT4);
> +		mxc_iomux_set_pad(MX53_PIN_EIM_D17, 0x104);
> +		mxc_iomux_set_input(
> +				MUX_IN_ECSPI1_IPP_IND_MISO_SELECT_INPUT, 0x3);
> +
> +		if (dev->ss == 0) {
> +			/* de-select SS1 of instance: ecspi1. */
> +			mxc_request_iomux(MX53_PIN_EIM_D19, IOMUX_CONFIG_ALT1);
> +			mxc_iomux_set_pad(MX53_PIN_EIM_D19, 0x1E4);
> +
> +			/* mux mode: ALT4 mux port: SS0 of instance: ecspi1. */
> +			mxc_request_iomux(MX53_PIN_EIM_EB2, IOMUX_CONFIG_ALT4);
> +			mxc_iomux_set_pad(MX53_PIN_EIM_EB2, 0x104);
> +			mxc_iomux_set_input(
> +				MUX_IN_ECSPI1_IPP_IND_SS_B_1_SELECT_INPUT, 0x3);

Has the MX53 the same errata as the MX51 ? Logically, we do not need to
deselect pins in the multiplexer when we are simply changing the chip
select to access a different device. However, I know the errata for MX51
 and this code is required in case the chip select is active high
instead of low. Really do we need this code for mx53evk ?

> +#ifdef CONFIG_I2C_MXC
> +static void setup_core_voltages(void)
> +{
> +	unsigned char buf[4] = { 0 };
> +	i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
> +
> +	/* Set core voltage VDDGP to 1.1V */
> +	buf[0] = 0x45;
> +	buf[1] = 0x4a;
> +	buf[2] = 0x54;
> +	if (i2c_write(0x8, 24, 1, buf, 3))
> +		return;
> +
> +	/* Set DDR voltage VDDA to 1.225V */
> +	buf[0] = 0;
> +	buf[1] = 0x63;
> +	buf[2] = 0x1a;
> +	if (i2c_write(0x8, 26, 1, buf, 3))
> +		return;

I do not know what is connected on the I2C bus, and I cannot understand
the code. I can only suppose there is a Power Controller, as the mx51evk
has, but in this case the I2C instead of SPI is used. However, we need
an header file to describe the internal register of this chip (at least
for the registers you are using !).

> +#ifdef CONFIG_FSL_ESDHC
> +int board_mmc_getcd(u8 *cd, struct mmc *mmc)
> +{
> +	*cd = 0;
> +
> +        return 0;
> +}

I do not understand, the code seems wrong. The function is a weak
function, if you do not need, you do not need to implement. It seems you
suppose that a SD card is *always* inserted, even if this is not true.


> +	puts("Boot Device: ");
> +	switch (get_boot_device()) {
> +	case WEIM_NOR_BOOT:
> +		printf("NOR\n");

Use puts if you do not need to format variables.


> +/*
> + * Disabled for now due to build problems under Debian and a significant
> + * increase in the final file size: 144260 vs. 109536 Bytes.
> + */

Comments unclear. What do you have disabled ?

> +/*
> + * Hardware drivers
> + */
> +#define CONFIG_MXC_UART
> +#define CONFIG_SYS_MX53_UART1
> +
> +/*
> + * Eth Configs
> + */
> +#define CONFIG_NET_MULTI
> +#define CONFIG_MII
> +#define CONFIG_MII_GASKET
> +#define CONFIG_DISCOVER_PHY

Why is CONFIG_FEC_MXC not set ? You set network parameters, but I do not
see a network driver. And it seems CONFIG_DISCOVER_PHY is obsolete (must
be removed from mx51evk, too).

> +
> +#define CONFIG_CMD_PING
> +#define CONFIG_CMD_DHCP
> +#define CONFIG_CMD_MII
> +#define CONFIG_CMD_NET
> +#define CONFIG_BOOTP_SUBNETMASK
> +#define CONFIG_BOOTP_GATEWAY
> +#define CONFIG_BOOTP_DNS

Network commands, but no ethernet driver...

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