[U-Boot] [PATCH 2/2] iMX5: EfikaMX: Work-in-progress board support

Stefano Babic
2011-01-13T09:37:47Z

On 01/12/2011 11:46 PM, Marek Vasut wrote:
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> ---
> board/efikamx/Makefile | 52 ++++
> board/efikamx/config.mk | 25 ++
> board/efikamx/efikamx.c | 668 ++++++++++++++++++++++++++++++++++++++++++++
> board/efikamx/imximage.cfg | 124 ++++++++
> boards.cfg | 1 +
> include/configs/efikamx.h | 233 +++++++++++++++
> 6 files changed, 1103 insertions(+), 0 deletions(-)
> create mode 100644 board/efikamx/Makefile
> create mode 100644 board/efikamx/config.mk
> create mode 100644 board/efikamx/efikamx.c
> create mode 100644 board/efikamx/imximage.cfg
> create mode 100644 include/configs/efikamx.h

Could you describe better which is the current status for this porting
and define what is already supported ? I mean, "work-in-progress" tells
me nothing about which peripherals are working and which not. Could you
add to your commit-id a short description, adding on which hardware
u-boot is running (on the development board ? on efika smarttotp ? on
efiga smartbook ? on all of them ?), and which peripherals are currently
supported.

You should add your name to the MAINTAINERS file, too.

> diff --git a/board/efikamx/efikamx.c b/board/efikamx/efikamx.c
> new file mode 100644
> index 0000000..10cf111
> +/******************************************************************************
> + * Shared variables / local defines
> + ******************************************************************************/
> +/* LED */
> +#define EFIKAMX_LED_BLUE 0x1
> +#define EFIKAMX_LED_GREEN 0x2
> +#define EFIKAMX_LED_RED 0x4
> +
> +void efikamx_toggle_led(uint32_t mask);
> +
> +/* Board revisions */
> +#define EFIKAMX_BOARD_REV_11 0x1
> +#define EFIKAMX_BOARD_REV_12 0x2
> +#define EFIKAMX_BOARD_REV_13 0x3
> +#define EFIKAMX_BOARD_REV_14 0x4
> +
> +/******************************************************************************
> + * Board identification
> + ******************************************************************************/
> +static u32 board_rev;
> +
> +u32 get_efika_rev(void)
> +{
> + u32 rev = 0;
> + /*
> + * Retrieve board ID:
> + * rev1.1: 1,1,1
^---spaces instead of tabs, please fix globally

> + * rev1.2: 1,1,0
> + * rev1.3: 1,0,1
> + * rev1.4: 1,0,0
> + */
> + mxc_request_iomux(MX51_PIN_NANDF_CS0, IOMUX_CONFIG_GPIO);
> + mxc_gpio_direction(IOMUX_TO_GPIO(MX51_PIN_NANDF_CS0), MXC_GPIO_DIRECTION_OUT);

Line too long, please fix globally.

> + mxc_request_iomux(MX51_PIN_NANDF_RB3, IOMUX_CONFIG_GPIO);
> + mxc_iomux_set_pad(MX51_PIN_NANDF_RB3, PAD_CTL_100K_PU);
> + mxc_gpio_direction(IOMUX_TO_GPIO(MX51_PIN_NANDF_RB3), MXC_GPIO_DIRECTION_IN);
> + rev |= (!!mxc_gpio_get(IOMUX_TO_GPIO(MX51_PIN_NANDF_RB3))) << 2;

Is it ok to leave the NAND pins configured as GPIO, or should they set
back for the NFC controller ?

> +/******************************************************************************
> + * SPI configuration
> + ******************************************************************************/
> +#ifdef CONFIG_MXC_SPI

Is there a reason why CONFIG_MXC_SPI is not set ? As I understand, it is
required to set the PMIC to make things working. If CONFIG_MXC_SPI,
something wrong happens. I would prefer you drop the #ifdef, and instead
of that you add a check at the beginning of the file reporting a
compiler error if CONFIG_MXC_SPI is not set.

> +static void power_init(void)
> +{
> + unsigned int val;
> + unsigned int reg;
> + struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)MXC_CCM_BASE;
> +
> + /* Write needed to Power Gate 2 register */
> + val = pmic_reg_read(REG_POWER_MISC);
> + val &= ~PWGT2SPIEN;
> + pmic_reg_write(REG_POWER_MISC, val);
> +
> + /* Externally powered */
> + val = pmic_reg_read(REG_CHARGE);
> + val |= ICHRG0 | ICHRG1 | ICHRG2 | ICHRG3 | CHGAUTOB;
> + pmic_reg_write(REG_CHARGE, val);
> +
> + /* power up the system first */
> + pmic_reg_write(REG_POWER_MISC, PWUP);
> +
> + /* NOTE: if (is_soc_rev(CHIP_REV_2_0) >= 0) */
> +
> + /* Set core voltage to 1.1V */
> + val = pmic_reg_read(REG_SW_0);
> + val = (val & (~0x1F)) | 0x14;

Replace fix constants with defines, adding them to mc13892.h if they are
missing.

> + pmic_reg_write(REG_SW_0, val);
> +
> + /* Setup VCC (SW2) to 1.25 */
> + val = pmic_reg_read(REG_SW_1);
> + val = (val & (~0x1F)) | 0x1A;

Ditto

> + /* Green LED */
> + mxc_request_iomux(MX51_PIN_CSI1_VSYNC, IOMUX_CONFIG_ALT3);
> + mxc_gpio_direction(IOMUX_TO_GPIO(MX51_PIN_CSI1_VSYNC), MXC_GPIO_DIRECTION_OUT);

Line to long

> +void efikamx_toggle_led(uint32_t mask)
> +{
> + mxc_gpio_set(IOMUX_TO_GPIO(MX51_PIN_CSI1_D9), mask & EFIKAMX_LED_BLUE);
^
|--- trailing whitespaces

> +
> + switch (__raw_readl(SRC_BASE_ADDR + 0x8)) {

We have a structure for this register,please use struct src from imx_regs.h

> +# Boot Device : one of
> +# spi, sd (the board has no nand neither onenand)
> +
> +# FIX XXX

Why fix ?

> diff --git a/include/configs/efikamx.h b/include/configs/efikamx.h
> new file mode 100644
> index 0000000..cb29bcd
> --- /dev/null
> +++ b/include/configs/efikamx.h
> @@ -0,0 +1,233 @@
> +/*
> + * Copyright (C) 2007, Guennadi Liakhovetski <lg@denx.de>
> + *
> + * (C) Copyright 2009 Freescale Semiconductor, Inc.
> + *
> + * Configuration settings for the MX51EVK Board
> + *
> + * 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
> + */
> +
> +#ifndef __CONFIG_H
> +#define __CONFIG_H
> +
> +#include <asm/arch/imx-regs.h>
> +#include <config_cmd_default.h>
> +
> +/*
> + * High Level Board Configuration Options
> + */
> +/* An i.MX51 CPU */
> +#define CONFIG_MX51

Move this define before including imx-regs.h. Patches for mx53evk are
currently merged in u-boot-imx, and the imx-regs.h file has an #ifdef
CONFIG_MX51 switch to define the different offsets for the two processors.

> +/*
> + * SPI Interface
> + */
> +#ifdef CONFIG_CMD_SPI
> +
> +#define CONFIG_HARD_SPI 1
> +#define CONFIG_MXC_SPI 1
> +#define CONFIG_DEFAULT_SPI_BUS 1
> +#define CONFIG_DEFAULT_SPI_MODE (SPI_MODE_0 | SPI_CS_HIGH)
> +
> +/* SPI FLASH */
> +#ifdef CONFIG_CMD_SF
> +#define CONFIG_FSL_SF

I see, I let this useless CONFIG_ when I developped the vision2 board.
However, CONFIG_FSL_SF is not used at all in u-boot. It was a test I
made using a SPI flash driver from Freescale's code. You should drop it,
and I have to do the same for the vision2.

> +
> +#define CONFIG_SPI_FLASH
> +#define CONFIG_SPI_FLASH_SST
> +#define CONFIG_SPI_FLASH_CS (1 | 121 << 8)

Is it possible that the efika use exactly the same gpio (GPIO4_25) as
the vision2 board, or it is only a cut&paste error ?

> +#define __io

I think we have already discussed abot this define. Should it not move
to another file, such as imx-regs.h ? Or is there a better solution ?

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