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

[Marek Vasut]

On Thursday 13 January 2011 10:37:47 Stefano Babic wrote:
> On 01/12/2011 11:46 PM, Marek Vasut wrote:
> > Signed-off-by: Marek Vasut <marek.vasut at 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.

Will do, sry

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

Fixed in v2
> 
> > +	 * 	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.

DTTO

> 
> > +	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 ?

I don't think there is even NAND used on efikamx. Though I might be mistaken.
> 
> > +/***********************************************************************
> > ******* + * 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.

I added compile-time check. Fixed in v2
> 
> > +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.

Is any sane datasheet for this thing available ? If so, can you link me to it 
please?

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

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

Fixed in v2
> > 
> > +
> > +	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 ?

Ah, that's because I used SD card to boot it, it should officially boot from SPI 
nor. Fixed in v2

> 
> > 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 at 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.

Fixed in v2
> 
> > +/*
> > + * 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.

Fixed in v2
> 
> > +
> > +#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 ?

seems to be the same.
> 
> > +#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 ?

The thread where we discussed this got deaf. I tried asking further, but no 
reply :-(
> 
> Best regards
> Stefano Babic

Thanks for the review, it helped greatly.

Cheers