[U-Boot] [PATCH V2 1/5] pxa: move i2c driver to the common place
Lei Wen
adrian.wenl at gmail.com
Thu Mar 17 07:25:14 CET 2011
Hi Heiko,
On Tue, Mar 15, 2011 at 2:48 PM, Heiko Schocher <hs at denx.de> wrote:
> 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 for reviewing, I would post the V3 patch to fix the code style issue...
Best regards,
Lei
More information about the U-Boot
mailing list