[U-Boot] [PATCH V2 1/5] pxa: move i2c driver to the common place

[Heiko Schocher]

Hello Lei,

Lei Wen wrote:
> For better sharing with other platform other than pxa's,
> it is more convenient to put the driver to the common place.
> 
> Signed-off-by: Lei Wen <leiwen at marvell.com>
> ---
> Changelog:
> v2: rename previous pxa_i2c to mvi2c.

Maybe a Coding Style cleanup would be good for this file ...
(I know, you don;t wrote this file...)

>  arch/arm/cpu/pxa/Makefile |    1 -
>  arch/arm/cpu/pxa/i2c.c    |  469 ---------------------------------------------
>  drivers/i2c/Makefile      |    1 +
>  drivers/i2c/mvi2c.c       |  469 +++++++++++++++++++++++++++++++++++++++++++++

Why you don;t use git format-patch -M ?
If so, I wouldn;t stumbled over the Coding Style ;-)

>  include/configs/innokom.h |    1 +
>  include/configs/xm250.h   |    1 +
>  6 files changed, 472 insertions(+), 470 deletions(-)
>  delete mode 100644 arch/arm/cpu/pxa/i2c.c
>  create mode 100644 drivers/i2c/mvi2c.c
> 
[...]
> diff --git a/drivers/i2c/mvi2c.c b/drivers/i2c/mvi2c.c
> new file mode 100644
> index 0000000..7aa49ae
> --- /dev/null
> +++ b/drivers/i2c/mvi2c.c
> @@ -0,0 +1,469 @@
> +/*
> + * (C) Copyright 2000
> + * Paolo Scaffardi, AIRVENT SAM s.p.a - RIMINI(ITALY), arsenio at tin.it
> + *
> + * (C) Copyright 2000 Sysgo Real-Time Solutions, GmbH <www.elinos.com>
> + * Marius Groeger <mgroeger at sysgo.de>
> + *
> + * (C) Copyright 2003 Pengutronix e.K.
> + * Robert Schwebel <r.schwebel at pengutronix.de>
> + *
> + * 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
> + *
> + * Back ported to the 8xx platform (from the 8260 platform) by
> + * Murray.Jensen at cmst.csiro.au, 27-Jan-01.
> + */
> +
> +/* FIXME: this file is PXA255 specific! What about other XScales? */

As you rename this file to mv, please change this comment.

> +
> +#include <common.h>
> +#include <asm/io.h>
> +
> +#ifdef CONFIG_HARD_I2C
> +
> +/*
> + *	- CONFIG_SYS_I2C_SPEED
> + *	- I2C_PXA_SLAVE_ADDR
> + */
> +
> +#include <asm/arch/hardware.h>
> +#include <asm/arch/pxa-regs.h>
> +#include <i2c.h>
> +
> +/*#define	DEBUG_I2C	1	/###* activate local debugging output  */

Please remove dead code.

> +#define I2C_PXA_SLAVE_ADDR	0x1	/* slave pxa unit address           */
> +
> +#if (CONFIG_SYS_I2C_SPEED == 400000)
> +#define I2C_ICR_INIT	(ICR_FM | ICR_BEIE | ICR_IRFIE | ICR_ITEIE | ICR_GCD | ICR_SCLE)

Line too long.

> +#else
> +#define I2C_ICR_INIT	(ICR_BEIE | ICR_IRFIE | ICR_ITEIE | ICR_GCD | ICR_SCLE)
> +#endif
> +
> +#define I2C_ISR_INIT		0x7FF
> +
> +#ifdef DEBUG_I2C
> +#define PRINTD(x) printf x
> +#else
> +#define PRINTD(x)
> +#endif
> +
> +
> +/* Shall the current transfer have a start/stop condition? */
> +#define I2C_COND_NORMAL		0
> +#define I2C_COND_START		1
> +#define I2C_COND_STOP		2
> +
> +/* Shall the current transfer be ack/nacked or being waited for it? */
> +#define I2C_ACKNAK_WAITACK	1
> +#define I2C_ACKNAK_SENDACK	2
> +#define I2C_ACKNAK_SENDNAK	4
> +
> +/* Specify who shall transfer the data (master or slave) */
> +#define I2C_READ		0
> +#define I2C_WRITE		1
> +
> +/* All transfers are described by this data structure */
> +struct i2c_msg {
> +	u8 condition;
> +	u8 acknack;
> +	u8 direction;
> +	u8 data;
> +};
> +
> +

Only one empty line.

> +/**
> + * i2c_pxa_reset: - reset the host controller
> + *
> + */

Wrong comment style, no empty line after the comment necessary.
Fix globally please.

> +
> +static void i2c_reset( void )

No spaces after "(" and before ")"
Fix globally please.

> +{
> +	writel(readl(ICR) & ~ICR_IUE, ICR);	/* disable unit */
> +	writel(readl(ICR) | ICR_UR, ICR);	/* reset the unit */
> +	udelay(100);
> +	writel(readl(ICR) & ~ICR_IUE, ICR);	/* disable unit */
> +#ifdef CONFIG_CPU_MONAHANS
> +	/* | CKENB_1_PWM1 | CKENB_0_PWM0); */
> +	writel(readl(CKENB) | (CKENB_4_I2C), CKENB);
> +#else /* CONFIG_CPU_MONAHANS */
> +	/* set the global I2C clock on */
> +	writel(readl(CKEN) | CKEN14_I2C, CKEN);
> +#endif
> +	writel(I2C_PXA_SLAVE_ADDR, ISAR);	/* set our slave address */
> +	writel(I2C_ICR_INIT, ICR);		/* set control reg values */
> +	writel(I2C_ISR_INIT, ISR);		/* set clear interrupt bits */
> +	writel(readl(ICR) | ICR_IUE, ICR);	/* enable unit */
> +	udelay(100);
> +}
> +
> +
> +/**
> + * i2c_isr_set_cleared: - wait until certain bits of the I2C status register
> + *	                  are set and cleared
> + *
> + * @return: 1 in case of success, 0 means timeout (no match within 10 ms).
> + */
> +static int i2c_isr_set_cleared( unsigned long set_mask, unsigned long cleared_mask )

Line too long.

> +{
> +	int timeout = 10000;
> +
> +	while( ((ISR & set_mask)!=set_mask) || ((ISR & cleared_mask)!=0) ){
              ^                 ^                                         ^
              no space          space around "!="                         space

> +		udelay( 10 );
> +		if( timeout-- < 0 ) return 0;

return please in a new line, please fix globally.

> +	}
> +
> +	return 1;
> +}
> +
> +
> +/**
> + * i2c_transfer: - Transfer one byte over the i2c bus
> + *
> + * This function can tranfer a byte over the i2c bus in both directions.
> + * It is used by the public API functions.
> + *
> + * @return:  0: transfer successful
> + *          -1: message is empty
> + *          -2: transmit timeout
> + *          -3: ACK missing
> + *          -4: receive timeout
> + *          -5: illegal parameters
> + *          -6: bus is busy and couldn't be aquired
> + */
> +int i2c_transfer(struct i2c_msg *msg)
> +{
> +	int ret;
> +
> +	if (!msg)
> +		goto transfer_error_msg_empty;
> +
> +	switch(msg->direction) {
> +

no empty line please.

> +	case I2C_WRITE:
> +
> +		/* check if bus is not busy */
> +		if (!i2c_isr_set_cleared(0,ISR_IBB))
                                           ^
                                           add a space please
> +			goto transfer_error_bus_busy;
> +
> +		/* start transmission */
> +		writel(readl(ICR) & ~ICR_START, ICR);
> +		writel(readl(ICR) & ~ICR_STOP, ICR);
> +		writel(msg->data, IDBR);
> +		if (msg->condition == I2C_COND_START)
> +			writel(readl(ICR) | ICR_START, ICR);
> +		if (msg->condition == I2C_COND_STOP)
> +			writel(readl(ICR) | ICR_STOP, ICR);
> +		if (msg->acknack == I2C_ACKNAK_SENDNAK)
> +			writel(readl(ICR) | ICR_ACKNAK, ICR);
> +		if (msg->acknack == I2C_ACKNAK_SENDACK)
> +			writel(readl(ICR) & ~ICR_ACKNAK, ICR);
> +		writel(readl(ICR) & ~ICR_ALDIE, ICR);
> +		writel(readl(ICR) | ICR_TB, ICR);
> +
> +		/* transmit register empty? */
> +		if (!i2c_isr_set_cleared(ISR_ITE,0))
                                                 ^
                                                 here too, fix
                                                 globally please.

> +			goto transfer_error_transmit_timeout;
> +
> +		/* clear 'transmit empty' state */
> +		writel(readl(ISR) | ISR_ITE, ISR);
> +
> +		/* wait for ACK from slave */
> +		if (msg->acknack == I2C_ACKNAK_WAITACK)
> +			if (!i2c_isr_set_cleared(0,ISR_ACKNAK))
> +				goto transfer_error_ack_missing;
> +		break;
> +
> +	case I2C_READ:
> +
> +		/* check if bus is not busy */
> +		if (!i2c_isr_set_cleared(0,ISR_IBB))
> +			goto transfer_error_bus_busy;
> +
> +		/* start receive */
> +		writel(readl(ICR) & ~ICR_START, ICR);
> +		writel(readl(ICR) & ~ICR_STOP, ICR);
> +		if (msg->condition == I2C_COND_START)
> +			writel(readl(ICR) | ICR_START, ICR);
> +		if (msg->condition == I2C_COND_STOP)
> +			writel(readl(ICR) | ICR_STOP, ICR);
> +		if (msg->acknack == I2C_ACKNAK_SENDNAK)
> +			writel(readl(ICR) | ICR_ACKNAK, ICR);
> +		if (msg->acknack == I2C_ACKNAK_SENDACK)
> +			writel(readl(ICR) & ~ICR_ACKNAK, ICR);
> +		writel(readl(ICR) & ~ICR_ALDIE, ICR);
> +		writel(readl(ICR) | ICR_TB, ICR);
> +
> +		/* receive register full? */
> +		if (!i2c_isr_set_cleared(ISR_IRF,0))
> +			goto transfer_error_receive_timeout;
> +
> +		msg->data = readl(IDBR);
> +
> +		/* clear 'receive empty' state */
> +		writel(readl(ISR) | ISR_IRF, ISR);
> +
> +		break;
> +
> +	default:
> +
> +		goto transfer_error_illegal_param;
> +
> +	}
> +
> +	return 0;
> +
> +transfer_error_msg_empty:
> +		PRINTD(("i2c_transfer: error: 'msg' is empty\n"));
> +		ret = -1; goto i2c_transfer_finish;
> +
> +transfer_error_transmit_timeout:
> +		PRINTD(("i2c_transfer: error: transmit timeout\n"));
> +		ret = -2; goto i2c_transfer_finish;
> +
> +transfer_error_ack_missing:
> +		PRINTD(("i2c_transfer: error: ACK missing\n"));
> +		ret = -3; goto i2c_transfer_finish;
> +
> +transfer_error_receive_timeout:
> +		PRINTD(("i2c_transfer: error: receive timeout\n"));
> +		ret = -4; goto i2c_transfer_finish;
> +
> +transfer_error_illegal_param:
> +		PRINTD(("i2c_transfer: error: illegal parameters\n"));
> +		ret = -5; goto i2c_transfer_finish;
> +
> +transfer_error_bus_busy:
> +		PRINTD(("i2c_transfer: error: bus is busy\n"));
> +		ret = -6; goto i2c_transfer_finish;
> +
> +i2c_transfer_finish:
> +		PRINTD(("i2c_transfer: ISR: 0x%04x\n",ISR));
> +		i2c_reset();
> +		return ret;
> +
> +}
> +
> +/* ------------------------------------------------------------------------ */
> +/* API Functions                                                            */
> +/* ------------------------------------------------------------------------ */
> +
> +void i2c_init(int speed, int slaveaddr)
> +{
> +#ifdef CONFIG_SYS_I2C_INIT_BOARD
> +	/* call board specific i2c bus reset routine before accessing the   */
> +	/* environment, which might be in a chip on that bus. For details   */
> +	/* about this problem see doc/I2C_Edge_Conditions.                  */
> +	i2c_init_board();
> +#endif
> +}
> +
> +
> +/**
> + * i2c_probe: - Test if a chip answers for a given i2c address
> + *
> + * @chip:	address of the chip which is searched for
> + * @return:	0 if a chip was found, -1 otherwhise
> + */
> +
> +int i2c_probe(uchar chip)
> +{
> +	struct i2c_msg msg;
> +
> +	i2c_reset();
> +
> +	msg.condition = I2C_COND_START;
> +	msg.acknack   = I2C_ACKNAK_WAITACK;
> +	msg.direction = I2C_WRITE;
> +	msg.data      = (chip << 1) + 1;
> +	if (i2c_transfer(&msg)) return -1;
> +
> +	msg.condition = I2C_COND_STOP;
> +	msg.acknack   = I2C_ACKNAK_SENDNAK;
> +	msg.direction = I2C_READ;
> +	msg.data      = 0x00;
> +	if (i2c_transfer(&msg)) return -1;
> +
> +	return 0;
> +}
> +
> +
> +/**
> + * i2c_read: - Read multiple bytes from an i2c device
> + *
> + * The higher level routines take into account that this function is only
> + * called with len < page length of the device (see configuration file)
> + *
> + * @chip:	address of the chip which is to be read
> + * @addr:	i2c data address within the chip
> + * @alen:	length of the i2c data address (1..2 bytes)
> + * @buffer:	where to write the data
> + * @len:	how much byte do we want to read
> + * @return:	0 in case of success
> + */
> +
> +int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
> +{
> +	struct i2c_msg msg;
> +	u8 addr_bytes[3]; /* lowest...highest byte of data address */
> +	int ret;
> +
> +	PRINTD(("i2c_read(chip=0x%02x, addr=0x%02x, alen=0x%02x, len=0x%02x)\n",chip,addr,alen,len));

Line too long, add space after ","

> +
> +	i2c_reset();
> +
> +	/* dummy chip address write */
> +	PRINTD(("i2c_read: dummy chip address write\n"));
> +	msg.condition = I2C_COND_START;
> +	msg.acknack   = I2C_ACKNAK_WAITACK;
> +	msg.direction = I2C_WRITE;
> +	msg.data      = (chip << 1);
> +	msg.data     &= 0xFE;
> +	if ((ret=i2c_transfer(&msg))) return -1;

Please move the ret = i2c_transfer out of the if condition
"return -1" please in a new line.

> +
> +	/*
> +	 * send memory address bytes;
> +	 * alen defines how much bytes we have to send.
> +	 */
> +	/*addr &= ((1 << CONFIG_SYS_EEPROM_PAGE_WRITE_BITS)-1); */
> +	addr_bytes[0] = (u8)((addr >>  0) & 0x000000FF);
> +	addr_bytes[1] = (u8)((addr >>  8) & 0x000000FF);
> +	addr_bytes[2] = (u8)((addr >> 16) & 0x000000FF);
> +
> +	while (--alen >= 0) {
> +
> +		PRINTD(("i2c_read: send memory word address byte %1d\n",alen));
> +		msg.condition = I2C_COND_NORMAL;
> +		msg.acknack   = I2C_ACKNAK_WAITACK;
> +		msg.direction = I2C_WRITE;
> +		msg.data      = addr_bytes[alen];
> +		if ((ret=i2c_transfer(&msg))) return -1;
> +	}
> +
> +
> +	/* start read sequence */
> +	PRINTD(("i2c_read: start read sequence\n"));
> +	msg.condition = I2C_COND_START;
> +	msg.acknack   = I2C_ACKNAK_WAITACK;
> +	msg.direction = I2C_WRITE;
> +	msg.data      = (chip << 1);
> +	msg.data     |= 0x01;
> +	if ((ret=i2c_transfer(&msg))) return -1;
> +
> +	/* read bytes; send NACK at last byte */
> +	while (len--) {
> +
> +		if (len==0) {
> +			msg.condition = I2C_COND_STOP;
> +			msg.acknack   = I2C_ACKNAK_SENDNAK;
> +		} else {
> +			msg.condition = I2C_COND_NORMAL;
> +			msg.acknack   = I2C_ACKNAK_SENDACK;
> +		}
> +
> +		msg.direction = I2C_READ;
> +		msg.data      = 0x00;
> +		if ((ret=i2c_transfer(&msg))) return -1;
> +
> +		*buffer = msg.data;
> +		PRINTD(("i2c_read: reading byte (0x%08x)=0x%02x\n",(unsigned int)buffer,*buffer));
> +		buffer++;
> +
> +	}
> +
> +	i2c_reset();
> +
> +	return 0;
> +}
> +
> +
> +/**
> + * i2c_write: -  Write multiple bytes to an i2c device
> + *
> + * The higher level routines take into account that this function is only
> + * called with len < page length of the device (see configuration file)
> + *
> + * @chip:	address of the chip which is to be written
> + * @addr:	i2c data address within the chip
> + * @alen:	length of the i2c data address (1..2 bytes)
> + * @buffer:	where to find the data to be written
> + * @len:	how much byte do we want to read
> + * @return:	0 in case of success
> + */
> +
> +int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
> +{
> +	struct i2c_msg msg;
> +	u8 addr_bytes[3]; /* lowest...highest byte of data address */
> +
> +	PRINTD(("i2c_write(chip=0x%02x, addr=0x%02x, alen=0x%02x, len=0x%02x)\n",chip,addr,alen,len));
> +
> +	i2c_reset();
> +
> +	/* chip address write */
> +	PRINTD(("i2c_write: chip address write\n"));
> +	msg.condition = I2C_COND_START;
> +	msg.acknack   = I2C_ACKNAK_WAITACK;
> +	msg.direction = I2C_WRITE;
> +	msg.data      = (chip << 1);
> +	msg.data     &= 0xFE;
> +	if (i2c_transfer(&msg)) return -1;
> +
> +	/*
> +	 * send memory address bytes;
> +	 * alen defines how much bytes we have to send.
> +	 */
> +	addr_bytes[0] = (u8)((addr >>  0) & 0x000000FF);
> +	addr_bytes[1] = (u8)((addr >>  8) & 0x000000FF);
> +	addr_bytes[2] = (u8)((addr >> 16) & 0x000000FF);
> +
> +	while (--alen >= 0) {
> +
> +		PRINTD(("i2c_write: send memory word address\n"));
> +		msg.condition = I2C_COND_NORMAL;
> +		msg.acknack   = I2C_ACKNAK_WAITACK;
> +		msg.direction = I2C_WRITE;
> +		msg.data      = addr_bytes[alen];
> +		if (i2c_transfer(&msg)) return -1;
> +	}
> +
> +	/* write bytes; send NACK at last byte */
> +	while (len--) {
> +
> +		PRINTD(("i2c_write: writing byte (0x%08x)=0x%02x\n",(unsigned int)buffer,*buffer));
> +
> +		if (len==0)
> +			msg.condition = I2C_COND_STOP;
> +		else
> +			msg.condition = I2C_COND_NORMAL;
> +
> +		msg.acknack   = I2C_ACKNAK_WAITACK;
> +		msg.direction = I2C_WRITE;
> +		msg.data      = *(buffer++);
> +
> +		if (i2c_transfer(&msg)) return -1;
> +
> +	}
> +
> +	i2c_reset();
> +
> +	return 0;
> +
> +}
> +
> +#endif	/* CONFIG_HARD_I2C */
> diff --git a/include/configs/innokom.h b/include/configs/innokom.h
> index d8fcbdb..0ea73c9 100644
> --- a/include/configs/innokom.h
> +++ b/include/configs/innokom.h
> @@ -140,6 +140,7 @@
>  /*
>   * I2C bus
>   */
> +#define CONFIG_I2C_MV			1
>  #define CONFIG_HARD_I2C			1
>  #define CONFIG_SYS_I2C_SPEED			50000
>  #define CONFIG_SYS_I2C_SLAVE			0xfe
> diff --git a/include/configs/xm250.h b/include/configs/xm250.h
> index 497cb91..b4b940a 100644
> --- a/include/configs/xm250.h
> +++ b/include/configs/xm250.h
> @@ -61,6 +61,7 @@
>  /*
>   * I2C bus
>   */
> +#define CONFIG_I2C_MV			1
>  #define CONFIG_HARD_I2C			1
>  #define CONFIG_SYS_I2C_SPEED			50000
>  #define CONFIG_SYS_I2C_SLAVE			0xfe

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany