diff mbox

[U-Boot,v3,1/3] ARM: I2C: I2C Multi byte address support

Message ID 1327311852-2868-1-git-send-email-rachna@ti.com
State Accepted
Commit 2faa76196af4b3e93bcb9e38ed9090cbd3b06db3
Delegated to: Heiko Schocher
Headers show

Commit Message

Patil, Rachna Jan. 23, 2012, 9:44 a.m. UTC
Existing OMAP I2C driver does not support address
length greater than one. Hence this patch is to
add support for 2 byte address read/write.

Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
Signed-off-by: Patil, Rachna <rachna@ti.com>
---
Changes for v3:
rebased, patches applied on top of master branch

 drivers/i2c/omap24xx_i2c.c |  467 ++++++++++++++++++++++++++++----------------
 drivers/i2c/omap24xx_i2c.h |    2 +
 2 files changed, 296 insertions(+), 173 deletions(-)

Comments

Heiko Schocher Jan. 25, 2012, 6:25 a.m. UTC | #1
Hello Patil,

Patil, Rachna wrote:
> Existing OMAP I2C driver does not support address
> length greater than one. Hence this patch is to
> add support for 2 byte address read/write.
> 
> Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
> Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
> Signed-off-by: Patil, Rachna <rachna@ti.com>
> ---
> Changes for v3:
> rebased, patches applied on top of master branch
> 
>  drivers/i2c/omap24xx_i2c.c |  467 ++++++++++++++++++++++++++++----------------
>  drivers/i2c/omap24xx_i2c.h |    2 +
>  2 files changed, 296 insertions(+), 173 deletions(-)

Applied to u-boot-i2c.git

Thanks.

bye,
Heiko
Thomas Weber Feb. 16, 2012, 7:33 p.m. UTC | #2
Hello Rachna,

On 01/23/2012 10:44 AM, Patil, Rachna wrote:
> Existing OMAP I2C driver does not support address
> length greater than one. Hence this patch is to
> add support for 2 byte address read/write.
>
> Signed-off-by: Philip, Avinash<avinashphilip@ti.com>
> Signed-off-by: Hebbar, Gururaja<gururaja.hebbar@ti.com>
> Signed-off-by: Patil, Rachna<rachna@ti.com>
> ---
> Changes for v3:
> rebased, patches applied on top of master branch
>
>   drivers/i2c/omap24xx_i2c.c |  467 ++++++++++++++++++++++++++++----------------
>   drivers/i2c/omap24xx_i2c.h |    2 +
>   2 files changed, 296 insertions(+), 173 deletions(-)
>
> diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
> index a7ffd95..80932ef 100644
> --- a/drivers/i2c/omap24xx_i2c.c
> +++ b/drivers/i2c/omap24xx_i2c.c
> @@ -29,10 +29,11 @@
>
>   DECLARE_GLOBAL_DATA_PTR;
>
> -#define I2C_TIMEOUT	1000
> +#define I2C_STAT_TIMEO	(1<<  31)
> +#define I2C_TIMEOUT	10
>   

why do you change I2C_TIMEOUT from 1000 to 10?

Thomas
balajitk@ti.com Feb. 17, 2012, 7:05 a.m. UTC | #3
On Mon, Jan 23, 2012 at 3:14 PM, Patil, Rachna <rachna@ti.com> wrote:
> commit 2faa76196af4b3e93bcb9e38ed9090cbd3b06db3
> Author: Patil, Rachna <rachna@ti.com>
> Date:   Sun Jan 22 23:44:12 2012 +0000
>
>     ARM: I2C: I2C Multi byte address support
>
>     Existing OMAP I2C driver does not support address
>     length greater than one. Hence this patch is to
>     add support for 2 byte address read/write.
>
>     Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
>     Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
>     Signed-off-by: Patil, Rachna <rachna@ti.com>
>
> diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
> index a7ffd95..80932ef 100644
> --- a/drivers/i2c/omap24xx_i2c.c
> +++ b/drivers/i2c/omap24xx_i2c.c
> @@ -29,10 +29,11 @@
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> -#define I2C_TIMEOUT	1000
> +#define I2C_STAT_TIMEO	(1 << 31)
> +#define I2C_TIMEOUT	10
>
> -static void wait_for_bb(void);
> -static u16 wait_for_pin(void);
> +static u32 wait_for_bb(void);
> +static u32 wait_for_status_mask(u16 mask);
>  static void flush_fifo(void);
>
>  /*
> @@ -50,7 +51,6 @@ void i2c_init(int speed, int slaveadd)
>  	int psc, fsscll, fssclh;
>  	int hsscll = 0, hssclh = 0;
>  	u32 scll, sclh;
> -	int timeout = I2C_TIMEOUT;
>
>  	/* Only handle standard, fast and high speeds */
>  	if ((speed != OMAP_I2C_STANDARD) &&
> @@ -112,24 +112,14 @@ void i2c_init(int speed, int slaveadd)
>  		sclh = (unsigned int)fssclh;
>  	}
>
> +	if (gd->flags & GD_FLG_RELOC)
> +		bus_initialized[current_bus] = 1;
> +
>  	if (readw(&i2c_base->con) & I2C_CON_EN) {
>  		writew(0, &i2c_base->con);
>  		udelay(50000);
>  	}
>
> -	writew(0x2, &i2c_base->sysc); /* for ES2 after soft reset */
> -	udelay(1000);
> -
> -	writew(I2C_CON_EN, &i2c_base->con);
> -	while (!(readw(&i2c_base->syss) & I2C_SYSS_RDONE) && timeout--) {
> -		if (timeout <= 0) {
> -			puts("ERROR: Timeout in soft-reset\n");
> -			return;
> -		}
> -		udelay(1000);
> -	}
> -
> -	writew(0, &i2c_base->con);

This change in not related to multi byte address
and why is this removed

>  	writew(psc, &i2c_base->psc);
>  	writew(scll, &i2c_base->scll);
>  	writew(sclh, &i2c_base->sclh);
> @@ -145,81 +135,6 @@ void i2c_init(int speed, int slaveadd)
>  	flush_fifo();
>  	writew(0xFFFF, &i2c_base->stat);
>  	writew(0, &i2c_base->cnt);
> -
> -	if (gd->flags & GD_FLG_RELOC)
> -		bus_initialized[current_bus] = 1;

why is this section of code moved above ?

> -}
> -
> -static int i2c_read_byte(u8 devaddr, u8 regoffset, u8 *value)
> -{
> -	int i2c_error = 0;
> -	u16 status;
> -
> -	/* wait until bus not busy */
> -	wait_for_bb();
> -
> -	/* one byte only */
> -	writew(1, &i2c_base->cnt);
> -	/* set slave address */
> -	writew(devaddr, &i2c_base->sa);
> -	/* no stop bit needed here */
> -	writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT |
> -	      I2C_CON_TRX, &i2c_base->con);
> -
> -	/* send register offset */
> -	while (1) {
> -		status = wait_for_pin();
> -		if (status == 0 || status & I2C_STAT_NACK) {
> -			i2c_error = 1;
> -			goto read_exit;
> -		}
> -		if (status & I2C_STAT_XRDY) {
> -			/* Important: have to use byte access */
> -			writeb(regoffset, &i2c_base->data);
> -			writew(I2C_STAT_XRDY, &i2c_base->stat);
> -		}
> -		if (status & I2C_STAT_ARDY) {
> -			writew(I2C_STAT_ARDY, &i2c_base->stat);
> -			break;
> -		}
> -	}
> -
> -	/* set slave address */
> -	writew(devaddr, &i2c_base->sa);
> -	/* read one byte from slave */
> -	writew(1, &i2c_base->cnt);
> -	/* need stop bit here */
> -	writew(I2C_CON_EN | I2C_CON_MST |
> -		I2C_CON_STT | I2C_CON_STP,
> -		&i2c_base->con);
> -
> -	/* receive data */
> -	while (1) {
> -		status = wait_for_pin();
> -		if (status == 0 || status & I2C_STAT_NACK) {
> -			i2c_error = 1;
> -			goto read_exit;
> -		}
> -		if (status & I2C_STAT_RRDY) {
> -#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \
> -	defined(CONFIG_OMAP44XX)

Note you have removed CONFIG_OMAP44XX here
and not added in any other place in this patch
was this patch developed on latest baseline or something went wrong
while rebasing ?

> -			*value = readb(&i2c_base->data);
> -#else
> -			*value = readw(&i2c_base->data);
> -#endif
> -			writew(I2C_STAT_RRDY, &i2c_base->stat);
> -		}
> -		if (status & I2C_STAT_ARDY) {
> -			writew(I2C_STAT_ARDY, &i2c_base->stat);
> -			break;
> -		}
> -	}
> -
> -read_exit:
> -	flush_fifo();
> -	writew(0xFFFF, &i2c_base->stat);
> -	writew(0, &i2c_base->cnt);
> -	return i2c_error;
>  }
>
>  static void flush_fifo(void)
> @@ -246,32 +161,42 @@ static void flush_fifo(void)
>
>  int i2c_probe(uchar chip)
>  {
> -	u16 status;
> +	u32 status;
>  	int res = 1; /* default = fail */
>
>  	if (chip == readw(&i2c_base->oa))
>  		return res;
>
>  	/* wait until bus not busy */
> -	wait_for_bb();
> +	status = wait_for_bb();
> +	/* exit on BUS busy */
> +	if (status & I2C_STAT_TIMEO)
> +		return res;
>
>  	/* try to write one byte */
>  	writew(1, &i2c_base->cnt);
>  	/* set slave address */
>  	writew(chip, &i2c_base->sa);
>  	/* stop bit needed here */
> -	writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX |
> -	       I2C_CON_STP, &i2c_base->con);
> -
> -	status = wait_for_pin();
> -
> -	/* check for ACK (!NAK) */
> -	if (!(status & I2C_STAT_NACK))
> -		res = 0;
> -
> -	/* abort transfer (force idle state) */
> -	writew(0, &i2c_base->con);
> -
> +	writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT
> +			| I2C_CON_STP, &i2c_base->con);
> +	/* enough delay for the NACK bit set */
> +	udelay(9000);
> +
> +	if (!(readw(&i2c_base->stat) & I2C_STAT_NACK)) {
> +		res = 0;      /* success case */

> +		flush_fifo();
> +		writew(0xFFFF, &i2c_base->stat);

flush_fifo and clearing stat is anyway at the end of function
why is this repeated

> +	} else {
> +		/* failure, clear sources*/
> +		writew(0xFFFF, &i2c_base->stat);
> +		/* finish up xfer */
> +		writew(readw(&i2c_base->con) | I2C_CON_STP, &i2c_base->con);
> +		status = wait_for_bb();
> +		/* exit on BUS busy */
> +		if (status & I2C_STAT_TIMEO)
> +			return res;
> +	}
>  	flush_fifo();
>  	/* don't allow any more data in... we don't want it. */
>  	writew(0, &i2c_base->cnt);
> @@ -281,111 +206,309 @@ int i2c_probe(uchar chip)
>
>  int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
>  {
> -	int i;
> +	int i2c_error = 0, i;
> +	u32 status;
> +
> +	if ((alen > 2) || (alen < 0))
> +		return 1;
>
> -	if (alen > 1) {
> -		printf("I2C read: addr len %d not supported\n", alen);
> +	if (alen < 2) {
> +		if (addr + len > 256)
> +			return 1;
> +	} else if (addr + len > 0xFFFF) {
>  		return 1;
>  	}
>
> -	if (addr + len > 256) {
> -		puts("I2C read: address out of range\n");
> +	/* wait until bus not busy */
> +	status = wait_for_bb();
> +
> +	/* exit on BUS busy */
> +	if (status & I2C_STAT_TIMEO)
>  		return 1;
> +
> +	writew((alen & 0xFF), &i2c_base->cnt);
> +	/* set slave address */
> +	writew(chip, &i2c_base->sa);
> +	/* Clear the Tx & Rx FIFOs */
> +	writew((readw(&i2c_base->buf) | I2C_RXFIFO_CLEAR |
> +		I2C_TXFIFO_CLEAR), &i2c_base->buf);
> +	/* no stop bit needed here */
> +	writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_TRX |
> +		I2C_CON_STT, &i2c_base->con);
> +
> +	/* wait for Transmit ready condition */
> +	status = wait_for_status_mask(I2C_STAT_XRDY | I2C_STAT_NACK);
> +
> +	if (status & (I2C_STAT_NACK | I2C_STAT_TIMEO))
> +		i2c_error = 1;
> +
> +	if (!i2c_error) {
> +		if (status & I2C_STAT_XRDY) {
> +			switch (alen) {
> +			case 2:
> +				/* Send address MSByte */
> +#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
> +				writew(((addr >> 8) & 0xFF), &i2c_base->data);

writew or writeb??

> +
> +				/* Clearing XRDY event */
> +				writew((status & I2C_STAT_XRDY),
> +						&i2c_base->stat);
> +				/* wait for Transmit ready condition */
> +				status = wait_for_status_mask(I2C_STAT_XRDY |
> +						I2C_STAT_NACK);
> +
> +				if (status & (I2C_STAT_NACK |
> +						I2C_STAT_TIMEO)) {
> +					i2c_error = 1;
> +					break;
> +				}
> +#endif
> +			case 1:
> +#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
> +				/* Send address LSByte */
> +				writew((addr & 0xFF), &i2c_base->data);

writew or writeb??

> +#else
> +				/* Send address Short word */
> +				writew((addr & 0xFFFF), &i2c_base->data);
> +#endif
> +				/* Clearing XRDY event */
> +				writew((status & I2C_STAT_XRDY),
> +					&i2c_base->stat);
> +				/*wait for Transmit ready condition */
> +				status = wait_for_status_mask(I2C_STAT_ARDY |
> +						I2C_STAT_NACK);
> +
> +				if (status & (I2C_STAT_NACK |
> +					I2C_STAT_TIMEO)) {
> +					i2c_error = 1;
> +					break;
> +				}
> +			}
> +		} else
> +			i2c_error = 1;
>  	}
>
> -	for (i = 0; i < len; i++) {
> -		if (i2c_read_byte(chip, addr + i, &buffer[i])) {
> -			puts("I2C read: I/O error\n");
> -			i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
> -			return 1;
> +	/* Wait for ARDY to set */
> +	status = wait_for_status_mask(I2C_STAT_ARDY | I2C_STAT_NACK
> +			| I2C_STAT_AL);
> +
> +	if (!i2c_error) {
> +		/* set slave address */
> +		writew(chip, &i2c_base->sa);
> +		writew((len & 0xFF), &i2c_base->cnt);
> +		/* Clear the Tx & Rx FIFOs */
> +		writew((readw(&i2c_base->buf) | I2C_RXFIFO_CLEAR |
> +			I2C_TXFIFO_CLEAR), &i2c_base->buf);
> +		/* need stop bit here */
> +		writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP,
> +			&i2c_base->con);
> +
> +		for (i = 0; i < len; i++) {
> +			/* wait for Receive condition */
> +			status = wait_for_status_mask(I2C_STAT_RRDY |
> +				I2C_STAT_NACK);
> +			if (status & (I2C_STAT_NACK | I2C_STAT_TIMEO)) {
> +				i2c_error = 1;
> +				break;
> +			}
> +
> +			if (status & I2C_STAT_RRDY) {
> +#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
> +				buffer[i] = readb(&i2c_base->data);
> +#else
> +				*((u16 *)&buffer[i]) =
> +					readw(&i2c_base->data) & 0xFFFF;
> +				i++;
> +#endif
> +				writew((status & I2C_STAT_RRDY),
> +					&i2c_base->stat);
> +				udelay(1000);
> +			} else {
> +				i2c_error = 1;
> +			}
>  		}
>  	}
>
> +	/* Wait for ARDY to set */
> +	status = wait_for_status_mask(I2C_STAT_ARDY | I2C_STAT_NACK
> +			| I2C_STAT_AL);

why do you have to wait for ARDY when transaction is done?

> +
> +	if (i2c_error) {
> +		writew(0, &i2c_base->con);
> +		return 1;
> +	}
> +
> +	writew(I2C_CON_EN, &i2c_base->con);
> +
> +	while (readw(&i2c_base->stat)
> +		|| (readw(&i2c_base->con) & I2C_CON_MST)) {
> +		udelay(10000);
> +		writew(0xFFFF, &i2c_base->stat);
> +	}
> +
> +	writew(I2C_CON_EN, &i2c_base->con);
> +	flush_fifo();
> +	writew(0xFFFF, &i2c_base->stat);
> +	writew(0, &i2c_base->cnt);
> +
>  	return 0;
>  }
>
>  int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
>  {
> -	int i;
> -	u16 status;
> -	int i2c_error = 0;
>
> -	if (alen > 1) {
> -		printf("I2C write: addr len %d not supported\n", alen);
> +	int i, i2c_error = 0;
> +	u32 status;
> +	u16 writelen;
> +
> +	if (alen > 2)
>  		return 1;
> -	}
>
> -	if (addr + len > 256) {
> -		printf("I2C write: address 0x%x + 0x%x out of range\n",
> -				addr, len);
> +	if (alen < 2) {
> +		if (addr + len > 256)
> +			return 1;
> +	} else if (addr + len > 0xFFFF) {
>  		return 1;
>  	}
>
>  	/* wait until bus not busy */
> -	wait_for_bb();
> +	status = wait_for_bb();
> +
> +	/* exiting on BUS busy */
> +	if (status & I2C_STAT_TIMEO)
> +		return 1;
>
> -	/* start address phase - will write regoffset + len bytes data */
> -	/* TODO consider case when !CONFIG_OMAP243X/34XX/44XX */
> -	writew(alen + len, &i2c_base->cnt);
> +	writelen = (len & 0xFFFF) + alen;
> +
> +	/* two bytes */
> +	writew((writelen & 0xFFFF), &i2c_base->cnt);

where else is this variable writelen used ?

> +	/* Clear the Tx & Rx FIFOs */
> +	writew((readw(&i2c_base->buf) | I2C_RXFIFO_CLEAR |
> +			I2C_TXFIFO_CLEAR), &i2c_base->buf);
>  	/* set slave address */
>  	writew(chip, &i2c_base->sa);
>  	/* stop bit needed here */
>  	writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX |
>  		I2C_CON_STP, &i2c_base->con);
>
> -	/* Send address byte */
> -	status = wait_for_pin();
> +	/* wait for Transmit ready condition */
> +	status = wait_for_status_mask(I2C_STAT_XRDY | I2C_STAT_NACK);
>
> -	if (status == 0 || status & I2C_STAT_NACK) {
> +	if (status & (I2C_STAT_NACK | I2C_STAT_TIMEO))
>  		i2c_error = 1;
> -		printf("error waiting for i2c address ACK (status=0x%x)\n",
> -		      status);
> -		goto write_exit;
> -	}
>
> -	if (status & I2C_STAT_XRDY) {
> -		writeb(addr & 0xFF, &i2c_base->data);
> -		writew(I2C_STAT_XRDY, &i2c_base->stat);
> -	} else {
> -		i2c_error = 1;
> -		printf("i2c bus not ready for transmit (status=0x%x)\n",
> -		      status);
> -		goto write_exit;
> -	}
> +	if (!i2c_error) {
> +		if (status & I2C_STAT_XRDY) {
> +			switch (alen) {
> +#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
> +			case 2:

what is the logical reason behind keeping case statement under
compilation flag ?

> +				/* send out MSB byte */
> +				writeb(((addr >> 8) & 0xFF), &i2c_base->data);
> +#else
> +				writeb((addr  & 0xFFFF), &i2c_base->data);

bit field is masked for 16 bits but byte write is used !!

> +				break;
> +#endif
> +				/* Clearing XRDY event */
> +				writew((status & I2C_STAT_XRDY),
> +					&i2c_base->stat);
> +				/*waiting for Transmit ready * condition */
> +				status = wait_for_status_mask(I2C_STAT_XRDY |
> +						I2C_STAT_NACK);
> +
> +				if (status & (I2C_STAT_NACK | I2C_STAT_TIMEO)) {
> +					i2c_error = 1;
> +					break;
> +				}

This should be in #if part for better readablity

> +			case 1:
> +#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
> +				/* send out MSB byte */
> +				writeb((addr  & 0xFF), &i2c_base->data);
> +#else
> +				writew(((buffer[0] << 8) | (addr & 0xFF)),
> +					&i2c_base->data);

can you explain the logic behind using buffer[0]

> +#endif
> +			}
>
> -	/* address phase is over, now write data */
> -	for (i = 0; i < len; i++) {
> -		status = wait_for_pin();
> +			/* Clearing XRDY event */
> +			writew((status & I2C_STAT_XRDY), &i2c_base->stat);
> +		}
> +
> +		/* waiting for Transmit ready condition */
> +		status = wait_for_status_mask(I2C_STAT_XRDY | I2C_STAT_NACK);
>
> -		if (status == 0 || status & I2C_STAT_NACK) {
> +		if (status & (I2C_STAT_NACK | I2C_STAT_TIMEO))
>  			i2c_error = 1;
> -			printf("i2c error waiting for data ACK (status=0x%x)\n",
> -					status);
> -			goto write_exit;
> +
> +		if (!i2c_error) {
> +			for (i = ((alen > 1) ? 0 : 1); i < len; i++) {

this is again not correct
Did you test this patch for alen = 1 ?

> +				if (status & I2C_STAT_XRDY) {
> +#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
> +					writeb((buffer[i] & 0xFF),
> +						&i2c_base->data);
> +#else
> +					writew((((buffer[i] << 8) |
> +					buffer[i + 1]) & 0xFFFF),
> +						&i2c_base->data);
> +					i++;
> +#endif
> +				} else
> +					i2c_error = 1;

> +					/* Clearing XRDY event */
> +					writew((status & I2C_STAT_XRDY),
> +						&i2c_base->stat);

wrong indent

> +					/* waiting for XRDY condition */
> +					status = wait_for_status_mask(
> +						I2C_STAT_XRDY |
> +						I2C_STAT_ARDY |
> +						I2C_STAT_NACK);
> +					if (status & (I2C_STAT_NACK |
> +						I2C_STAT_TIMEO)) {
> +						i2c_error = 1;
> +						break;
> +					}
> +					if (status & I2C_STAT_ARDY)
> +						break;
> +			}
>  		}
> +	}
>
> -		if (status & I2C_STAT_XRDY) {
> -			writeb(buffer[i], &i2c_base->data);
> -			writew(I2C_STAT_XRDY, &i2c_base->stat);
> -		} else {
> -			i2c_error = 1;
> -			printf("i2c bus not ready for Tx (i=%d)\n", i);
> -			goto write_exit;
> +	status = wait_for_status_mask(I2C_STAT_ARDY | I2C_STAT_NACK |
> +				I2C_STAT_AL);

why do have to wait for ARDY here?

> +
> +	if (status & (I2C_STAT_NACK | I2C_STAT_TIMEO))
> +		i2c_error = 1;
> +
> +	if (i2c_error) {
> +		writew(0, &i2c_base->con);
> +		return 1;
> +	}
> +
> +	if (!i2c_error) {
> +		int eout = 200;
> +
> +		writew(I2C_CON_EN, &i2c_base->con);
> +		while ((status = readw(&i2c_base->stat)) ||
> +				(readw(&i2c_base->con) & I2C_CON_MST)) {

is this a workaround or errata ? if yes can you please add some comment?
why should MST bit get set after writing I2C_CON_EN ??

> +			udelay(1000);
> +			/* have to read to clear intrrupt */

wrong comment and typo
This patch series is not really tested for other platform
like (CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
and the patch is buggy for OMAP4430 panda

<snip>
Thomas Weber Feb. 17, 2012, 9:02 a.m. UTC | #4
Hello Rachna,

I get multiple timeouts from wait_for_status_mask() during i2c_write(),
 line 481.

The Result is 1410h und status_mask is 7h.

I use a devkit8000, booting from sd-card.


Regards,
Thomas


U-Boot SPL 2011.12-00323-g9a3aae2-dirty (Feb 17 2012 - 09:30:36)
Texas Instruments Revision detection unimplemented
OMAP SD/MMC: 0
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
reading u-boot.img
reading u-boot.img


U-Boot 2011.12-00323-g9a3aae2-dirty (Feb 17 2012 - 09:30:36)

OMAP3530-GP ES3.1, CPU-OPP2, L3-165MHz, Max CPU Clock 600 mHz
OMAP3 DevKit8000 + LPDDR/NAND
I2C:   ready
DRAM:  128 MiB
NAND:  128 MiB
MMC:   OMAP SD/MMC: 0
NAND read from offset 260000 failed -74
*** Warning - readenv() failed, using default environment

In:    serial
Out:   serial
Err:   serial
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
ethaddr not set, using Die ID
Die ID #1d82000400000000040365f90100b017
Net:   dm9000
Hit any key to stop autoboot:  0
OMAP3 DevKit8000 #
Patil, Rachna Feb. 17, 2012, 9:37 a.m. UTC | #5
Hi Thomas,

On Fri, Feb 17, 2012 at 14:32:02, Thomas Weber wrote:
> Hello Rachna,
> 
> I get multiple timeouts from wait_for_status_mask() during i2c_write(),  line 481.
> 
> The Result is 1410h und status_mask is 7h.
> 
> I use a devkit8000, booting from sd-card.
> 

Currently I do not have an OMAP board to debug this, But can throw some pointers.
After u-boot prompt is up, can you try doing an i2c read/write/detect operation.
Does i2c fail at this stage as well?
From the log I see that i2c has not received an ACK.
Can you increase the timeout and check if this scenario persists.

Regards,
Rachna.

> 
> Regards,
> Thomas
> 
<SNIP>
Thomas Weber Feb. 17, 2012, 10:08 a.m. UTC | #6
Hello Rachna,

On 17.02.2012 10:37, Patil, Rachna wrote:
> Hi Thomas,
> 
> On Fri, Feb 17, 2012 at 14:32:02, Thomas Weber wrote:
>> Hello Rachna,
>>
>> I get multiple timeouts from wait_for_status_mask() during i2c_write(),  line 481.
>>
>> The Result is 1410h und status_mask is 7h.
>>
>> I use a devkit8000, booting from sd-card.
>>
> 
> Currently I do not have an OMAP board to debug this, But can throw some pointers.
> After u-boot prompt is up, can you try doing an i2c read/write/detect operation.
> Does i2c fail at this stage as well?


OMAP3 DevKit8000 # i2c md 48 5
0005: 49 49 18 18 18 06 06 06 1f 1f 1f 1f 1f 1f 09 00    II..............
OMAP3 DevKit8000 # i2c mw 48 4 4
timed out in wait_for_status_mask: I2C_STAT=1410


> From the log I see that i2c has not received an ACK.
> Can you increase the timeout and check if this scenario persists.

Same behaviour. No changes in output.

Thomas
Thomas Weber Feb. 19, 2012, 9:44 a.m. UTC | #7
Hello Tom, hello Rachna,

I don't know if this is related to the i2c changes or mmc changes?

When I boot from nand and then make "mmc rescan" the card is not detected.
When booting with boot_key on Devkit8000 directly from sd-card 
everything is okay.

Thomas


Log below:

U-Boot SPL 2011.12-00372-g4f7ed4f (Feb 19 2012 - 09:13:32)
Texas Instruments Revision detection unimplemented


U-Boot 2011.12-00372-g4f7ed4f (Feb 19 2012 - 09:13:32)

OMAP3530-GP ES3.1, CPU-OPP2, L3-165MHz, Max CPU Clock 600 mHz
OMAP3 DevKit8000 + LPDDR/NAND
I2C:   ready
DRAM:  128 MiB
NAND:  128 MiB
MMC:   OMAP SD/MMC: 0
NAND read from offset 260000 failed -74
*** Warning - readenv() failed, using default environment

In:    serial
Out:   serial
Err:   serial
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
ethaddr not set, using Die ID
Die ID #1d82000400000000040365f90100b017
Net:   dm9000
Hit any key to stop autoboot:  0
OMAP3 DevKit8000 # mmc rescan
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
mmc_send_cmd: timedout waiting on cmd inhibit to clear
mmc_send_cmd: timedout waiting on cmd inhibit to clear
mmc_send_cmd: timedout waiting on cmd inhibit to clear
Card did not respond to voltage select!
OMAP3 DevKit8000 # mmc rescan
timed out in wait_for_status_mask: I2C_STAT=1410
timed out in wait_for_status_mask: I2C_STAT=1410
mmc_send_cmd: timedout waiting on cmd inhibit to clear
mmc_send_cmd: timedout waiting on cmd inhibit to clear
mmc_send_cmd: timedout waiting on cmd inhibit to clear
Card did not respond to voltage select!
OMAP3 DevKit8000 #
Heiko Schocher Feb. 20, 2012, 8:03 a.m. UTC | #8
Hello T Krishnamoorthy

T Krishnamoorthy, Balaji wrote:
> On Mon, Jan 23, 2012 at 3:14 PM, Patil, Rachna <rachna@ti.com> wrote:
>> commit 2faa76196af4b3e93bcb9e38ed9090cbd3b06db3
>> Author: Patil, Rachna <rachna@ti.com>
>> Date:   Sun Jan 22 23:44:12 2012 +0000
>>
>>     ARM: I2C: I2C Multi byte address support
>>
>>     Existing OMAP I2C driver does not support address
>>     length greater than one. Hence this patch is to
>>     add support for 2 byte address read/write.
>>
>>     Signed-off-by: Philip, Avinash <avinashphilip@ti.com>
>>     Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
>>     Signed-off-by: Patil, Rachna <rachna@ti.com>
[...]
> wrong comment and typo
> This patch series is not really tested for other platform
> like (CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
> and the patch is buggy for OMAP4430 panda

:-(

Annoyingly this comment comes too late, as this patch series is in
mainline ... Can you (or Patil) fix this?

bye,
Heiko
Tom Rini Feb. 20, 2012, 10:23 p.m. UTC | #9
On Sun, Feb 19, 2012 at 10:44:52AM +0100, Thomas Weber wrote:
> Hello Tom, hello Rachna,
> 
> I don't know if this is related to the i2c changes or mmc changes?
> 
> When I boot from nand and then make "mmc rescan" the card is not detected.
> When booting with boot_key on Devkit8000 directly from sd-card
> everything is okay.

It's the i2c changes, I'm fairly certain.
Thomas Weber Feb. 21, 2012, 8:51 a.m. UTC | #10
Hello Tom,
On 02/20/2012 11:23 PM, Tom Rini wrote:
> On Sun, Feb 19, 2012 at 10:44:52AM +0100, Thomas Weber wrote:
>> Hello Tom, hello Rachna,
>>
>> I don't know if this is related to the i2c changes or mmc changes?
>>
>> When I boot from nand and then make "mmc rescan" the card is not detected.
>> When booting with boot_key on Devkit8000 directly from sd-card
>> everything is okay.
> It's the i2c changes, I'm fairly certain.
>

reverting

"nand: make 1-bit software ECC configurable"
"ARM: I2C: I2C Multi byte address support"
"ARM: AM33XX: Add AM33XX I2C driver support"

help for booting on Devkit8000.

You told me in IRC, that you don't need the CONFIG_MTD_ECC_SOFT for your 
boards? Can you explain why you don't need it? Perhaps something is 
misconfigured on Devkit8000. Or do you have no boards with nand flash?

Regards,
Thomas
Tom Rini Feb. 21, 2012, 2:22 p.m. UTC | #11
On Tue, Feb 21, 2012 at 09:51:59AM +0100, Thomas Weber wrote:
> Hello Tom,
> On 02/20/2012 11:23 PM, Tom Rini wrote:
> >On Sun, Feb 19, 2012 at 10:44:52AM +0100, Thomas Weber wrote:
> >>Hello Tom, hello Rachna,
> >>
> >>I don't know if this is related to the i2c changes or mmc changes?
> >>
> >>When I boot from nand and then make "mmc rescan" the card is not detected.
> >>When booting with boot_key on Devkit8000 directly from sd-card
> >>everything is okay.
> >It's the i2c changes, I'm fairly certain.
> >
> 
> reverting
> 
> "nand: make 1-bit software ECC configurable"
> "ARM: I2C: I2C Multi byte address support"
> "ARM: AM33XX: Add AM33XX I2C driver support"
> 
> help for booting on Devkit8000.
> 
> You told me in IRC, that you don't need the CONFIG_MTD_ECC_SOFT for
> your boards? Can you explain why you don't need it? Perhaps
> something is misconfigured on Devkit8000. Or do you have no boards
> with nand flash?

How are you writing your files to NAND?  I'd have sworn that on
devkit8000 you can write it all with 'nandecc hw' being done (adding
to my TODO list now..)
diff mbox

Patch

diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
index a7ffd95..80932ef 100644
--- a/drivers/i2c/omap24xx_i2c.c
+++ b/drivers/i2c/omap24xx_i2c.c
@@ -29,10 +29,11 @@ 
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define I2C_TIMEOUT	1000
+#define I2C_STAT_TIMEO	(1 << 31)
+#define I2C_TIMEOUT	10
 
-static void wait_for_bb(void);
-static u16 wait_for_pin(void);
+static u32 wait_for_bb(void);
+static u32 wait_for_status_mask(u16 mask);
 static void flush_fifo(void);
 
 /*
@@ -50,7 +51,6 @@  void i2c_init(int speed, int slaveadd)
 	int psc, fsscll, fssclh;
 	int hsscll = 0, hssclh = 0;
 	u32 scll, sclh;
-	int timeout = I2C_TIMEOUT;
 
 	/* Only handle standard, fast and high speeds */
 	if ((speed != OMAP_I2C_STANDARD) &&
@@ -112,24 +112,14 @@  void i2c_init(int speed, int slaveadd)
 		sclh = (unsigned int)fssclh;
 	}
 
+	if (gd->flags & GD_FLG_RELOC)
+		bus_initialized[current_bus] = 1;
+
 	if (readw(&i2c_base->con) & I2C_CON_EN) {
 		writew(0, &i2c_base->con);
 		udelay(50000);
 	}
 
-	writew(0x2, &i2c_base->sysc); /* for ES2 after soft reset */
-	udelay(1000);
-
-	writew(I2C_CON_EN, &i2c_base->con);
-	while (!(readw(&i2c_base->syss) & I2C_SYSS_RDONE) && timeout--) {
-		if (timeout <= 0) {
-			puts("ERROR: Timeout in soft-reset\n");
-			return;
-		}
-		udelay(1000);
-	}
-
-	writew(0, &i2c_base->con);
 	writew(psc, &i2c_base->psc);
 	writew(scll, &i2c_base->scll);
 	writew(sclh, &i2c_base->sclh);
@@ -145,81 +135,6 @@  void i2c_init(int speed, int slaveadd)
 	flush_fifo();
 	writew(0xFFFF, &i2c_base->stat);
 	writew(0, &i2c_base->cnt);
-
-	if (gd->flags & GD_FLG_RELOC)
-		bus_initialized[current_bus] = 1;
-}
-
-static int i2c_read_byte(u8 devaddr, u8 regoffset, u8 *value)
-{
-	int i2c_error = 0;
-	u16 status;
-
-	/* wait until bus not busy */
-	wait_for_bb();
-
-	/* one byte only */
-	writew(1, &i2c_base->cnt);
-	/* set slave address */
-	writew(devaddr, &i2c_base->sa);
-	/* no stop bit needed here */
-	writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT |
-	      I2C_CON_TRX, &i2c_base->con);
-
-	/* send register offset */
-	while (1) {
-		status = wait_for_pin();
-		if (status == 0 || status & I2C_STAT_NACK) {
-			i2c_error = 1;
-			goto read_exit;
-		}
-		if (status & I2C_STAT_XRDY) {
-			/* Important: have to use byte access */
-			writeb(regoffset, &i2c_base->data);
-			writew(I2C_STAT_XRDY, &i2c_base->stat);
-		}
-		if (status & I2C_STAT_ARDY) {
-			writew(I2C_STAT_ARDY, &i2c_base->stat);
-			break;
-		}
-	}
-
-	/* set slave address */
-	writew(devaddr, &i2c_base->sa);
-	/* read one byte from slave */
-	writew(1, &i2c_base->cnt);
-	/* need stop bit here */
-	writew(I2C_CON_EN | I2C_CON_MST |
-		I2C_CON_STT | I2C_CON_STP,
-		&i2c_base->con);
-
-	/* receive data */
-	while (1) {
-		status = wait_for_pin();
-		if (status == 0 || status & I2C_STAT_NACK) {
-			i2c_error = 1;
-			goto read_exit;
-		}
-		if (status & I2C_STAT_RRDY) {
-#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \
-	defined(CONFIG_OMAP44XX)
-			*value = readb(&i2c_base->data);
-#else
-			*value = readw(&i2c_base->data);
-#endif
-			writew(I2C_STAT_RRDY, &i2c_base->stat);
-		}
-		if (status & I2C_STAT_ARDY) {
-			writew(I2C_STAT_ARDY, &i2c_base->stat);
-			break;
-		}
-	}
-
-read_exit:
-	flush_fifo();
-	writew(0xFFFF, &i2c_base->stat);
-	writew(0, &i2c_base->cnt);
-	return i2c_error;
 }
 
 static void flush_fifo(void)
@@ -246,32 +161,42 @@  static void flush_fifo(void)
 
 int i2c_probe(uchar chip)
 {
-	u16 status;
+	u32 status;
 	int res = 1; /* default = fail */
 
 	if (chip == readw(&i2c_base->oa))
 		return res;
 
 	/* wait until bus not busy */
-	wait_for_bb();
+	status = wait_for_bb();
+	/* exit on BUS busy */
+	if (status & I2C_STAT_TIMEO)
+		return res;
 
 	/* try to write one byte */
 	writew(1, &i2c_base->cnt);
 	/* set slave address */
 	writew(chip, &i2c_base->sa);
 	/* stop bit needed here */
-	writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX |
-	       I2C_CON_STP, &i2c_base->con);
-
-	status = wait_for_pin();
-
-	/* check for ACK (!NAK) */
-	if (!(status & I2C_STAT_NACK))
-		res = 0;
-
-	/* abort transfer (force idle state) */
-	writew(0, &i2c_base->con);
-
+	writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT
+			| I2C_CON_STP, &i2c_base->con);
+	/* enough delay for the NACK bit set */
+	udelay(9000);
+
+	if (!(readw(&i2c_base->stat) & I2C_STAT_NACK)) {
+		res = 0;      /* success case */
+		flush_fifo();
+		writew(0xFFFF, &i2c_base->stat);
+	} else {
+		/* failure, clear sources*/
+		writew(0xFFFF, &i2c_base->stat);
+		/* finish up xfer */
+		writew(readw(&i2c_base->con) | I2C_CON_STP, &i2c_base->con);
+		status = wait_for_bb();
+		/* exit on BUS busy */
+		if (status & I2C_STAT_TIMEO)
+			return res;
+	}
 	flush_fifo();
 	/* don't allow any more data in... we don't want it. */
 	writew(0, &i2c_base->cnt);
@@ -281,111 +206,309 @@  int i2c_probe(uchar chip)
 
 int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
 {
-	int i;
+	int i2c_error = 0, i;
+	u32 status;
+
+	if ((alen > 2) || (alen < 0))
+		return 1;
 
-	if (alen > 1) {
-		printf("I2C read: addr len %d not supported\n", alen);
+	if (alen < 2) {
+		if (addr + len > 256)
+			return 1;
+	} else if (addr + len > 0xFFFF) {
 		return 1;
 	}
 
-	if (addr + len > 256) {
-		puts("I2C read: address out of range\n");
+	/* wait until bus not busy */
+	status = wait_for_bb();
+
+	/* exit on BUS busy */
+	if (status & I2C_STAT_TIMEO)
 		return 1;
+
+	writew((alen & 0xFF), &i2c_base->cnt);
+	/* set slave address */
+	writew(chip, &i2c_base->sa);
+	/* Clear the Tx & Rx FIFOs */
+	writew((readw(&i2c_base->buf) | I2C_RXFIFO_CLEAR |
+		I2C_TXFIFO_CLEAR), &i2c_base->buf);
+	/* no stop bit needed here */
+	writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_TRX |
+		I2C_CON_STT, &i2c_base->con);
+
+	/* wait for Transmit ready condition */
+	status = wait_for_status_mask(I2C_STAT_XRDY | I2C_STAT_NACK);
+
+	if (status & (I2C_STAT_NACK | I2C_STAT_TIMEO))
+		i2c_error = 1;
+
+	if (!i2c_error) {
+		if (status & I2C_STAT_XRDY) {
+			switch (alen) {
+			case 2:
+				/* Send address MSByte */
+#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
+				writew(((addr >> 8) & 0xFF), &i2c_base->data);
+
+				/* Clearing XRDY event */
+				writew((status & I2C_STAT_XRDY),
+						&i2c_base->stat);
+				/* wait for Transmit ready condition */
+				status = wait_for_status_mask(I2C_STAT_XRDY |
+						I2C_STAT_NACK);
+
+				if (status & (I2C_STAT_NACK |
+						I2C_STAT_TIMEO)) {
+					i2c_error = 1;
+					break;
+				}
+#endif
+			case 1:
+#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
+				/* Send address LSByte */
+				writew((addr & 0xFF), &i2c_base->data);
+#else
+				/* Send address Short word */
+				writew((addr & 0xFFFF), &i2c_base->data);
+#endif
+				/* Clearing XRDY event */
+				writew((status & I2C_STAT_XRDY),
+					&i2c_base->stat);
+				/*wait for Transmit ready condition */
+				status = wait_for_status_mask(I2C_STAT_ARDY |
+						I2C_STAT_NACK);
+
+				if (status & (I2C_STAT_NACK |
+					I2C_STAT_TIMEO)) {
+					i2c_error = 1;
+					break;
+				}
+			}
+		} else
+			i2c_error = 1;
 	}
 
-	for (i = 0; i < len; i++) {
-		if (i2c_read_byte(chip, addr + i, &buffer[i])) {
-			puts("I2C read: I/O error\n");
-			i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
-			return 1;
+	/* Wait for ARDY to set */
+	status = wait_for_status_mask(I2C_STAT_ARDY | I2C_STAT_NACK
+			| I2C_STAT_AL);
+
+	if (!i2c_error) {
+		/* set slave address */
+		writew(chip, &i2c_base->sa);
+		writew((len & 0xFF), &i2c_base->cnt);
+		/* Clear the Tx & Rx FIFOs */
+		writew((readw(&i2c_base->buf) | I2C_RXFIFO_CLEAR |
+			I2C_TXFIFO_CLEAR), &i2c_base->buf);
+		/* need stop bit here */
+		writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP,
+			&i2c_base->con);
+
+		for (i = 0; i < len; i++) {
+			/* wait for Receive condition */
+			status = wait_for_status_mask(I2C_STAT_RRDY |
+				I2C_STAT_NACK);
+			if (status & (I2C_STAT_NACK | I2C_STAT_TIMEO)) {
+				i2c_error = 1;
+				break;
+			}
+
+			if (status & I2C_STAT_RRDY) {
+#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
+				buffer[i] = readb(&i2c_base->data);
+#else
+				*((u16 *)&buffer[i]) =
+					readw(&i2c_base->data) & 0xFFFF;
+				i++;
+#endif
+				writew((status & I2C_STAT_RRDY),
+					&i2c_base->stat);
+				udelay(1000);
+			} else {
+				i2c_error = 1;
+			}
 		}
 	}
 
+	/* Wait for ARDY to set */
+	status = wait_for_status_mask(I2C_STAT_ARDY | I2C_STAT_NACK
+			| I2C_STAT_AL);
+
+	if (i2c_error) {
+		writew(0, &i2c_base->con);
+		return 1;
+	}
+
+	writew(I2C_CON_EN, &i2c_base->con);
+
+	while (readw(&i2c_base->stat)
+		|| (readw(&i2c_base->con) & I2C_CON_MST)) {
+		udelay(10000);
+		writew(0xFFFF, &i2c_base->stat);
+	}
+
+	writew(I2C_CON_EN, &i2c_base->con);
+	flush_fifo();
+	writew(0xFFFF, &i2c_base->stat);
+	writew(0, &i2c_base->cnt);
+
 	return 0;
 }
 
 int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
 {
-	int i;
-	u16 status;
-	int i2c_error = 0;
 
-	if (alen > 1) {
-		printf("I2C write: addr len %d not supported\n", alen);
+	int i, i2c_error = 0;
+	u32 status;
+	u16 writelen;
+
+	if (alen > 2)
 		return 1;
-	}
 
-	if (addr + len > 256) {
-		printf("I2C write: address 0x%x + 0x%x out of range\n",
-				addr, len);
+	if (alen < 2) {
+		if (addr + len > 256)
+			return 1;
+	} else if (addr + len > 0xFFFF) {
 		return 1;
 	}
 
 	/* wait until bus not busy */
-	wait_for_bb();
+	status = wait_for_bb();
+
+	/* exiting on BUS busy */
+	if (status & I2C_STAT_TIMEO)
+		return 1;
 
-	/* start address phase - will write regoffset + len bytes data */
-	/* TODO consider case when !CONFIG_OMAP243X/34XX/44XX */
-	writew(alen + len, &i2c_base->cnt);
+	writelen = (len & 0xFFFF) + alen;
+
+	/* two bytes */
+	writew((writelen & 0xFFFF), &i2c_base->cnt);
+	/* Clear the Tx & Rx FIFOs */
+	writew((readw(&i2c_base->buf) | I2C_RXFIFO_CLEAR |
+			I2C_TXFIFO_CLEAR), &i2c_base->buf);
 	/* set slave address */
 	writew(chip, &i2c_base->sa);
 	/* stop bit needed here */
 	writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX |
 		I2C_CON_STP, &i2c_base->con);
 
-	/* Send address byte */
-	status = wait_for_pin();
+	/* wait for Transmit ready condition */
+	status = wait_for_status_mask(I2C_STAT_XRDY | I2C_STAT_NACK);
 
-	if (status == 0 || status & I2C_STAT_NACK) {
+	if (status & (I2C_STAT_NACK | I2C_STAT_TIMEO))
 		i2c_error = 1;
-		printf("error waiting for i2c address ACK (status=0x%x)\n",
-		      status);
-		goto write_exit;
-	}
 
-	if (status & I2C_STAT_XRDY) {
-		writeb(addr & 0xFF, &i2c_base->data);
-		writew(I2C_STAT_XRDY, &i2c_base->stat);
-	} else {
-		i2c_error = 1;
-		printf("i2c bus not ready for transmit (status=0x%x)\n",
-		      status);
-		goto write_exit;
-	}
+	if (!i2c_error) {
+		if (status & I2C_STAT_XRDY) {
+			switch (alen) {
+#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
+			case 2:
+				/* send out MSB byte */
+				writeb(((addr >> 8) & 0xFF), &i2c_base->data);
+#else
+				writeb((addr  & 0xFFFF), &i2c_base->data);
+				break;
+#endif
+				/* Clearing XRDY event */
+				writew((status & I2C_STAT_XRDY),
+					&i2c_base->stat);
+				/*waiting for Transmit ready * condition */
+				status = wait_for_status_mask(I2C_STAT_XRDY |
+						I2C_STAT_NACK);
+
+				if (status & (I2C_STAT_NACK | I2C_STAT_TIMEO)) {
+					i2c_error = 1;
+					break;
+				}
+			case 1:
+#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
+				/* send out MSB byte */
+				writeb((addr  & 0xFF), &i2c_base->data);
+#else
+				writew(((buffer[0] << 8) | (addr & 0xFF)),
+					&i2c_base->data);
+#endif
+			}
 
-	/* address phase is over, now write data */
-	for (i = 0; i < len; i++) {
-		status = wait_for_pin();
+			/* Clearing XRDY event */
+			writew((status & I2C_STAT_XRDY), &i2c_base->stat);
+		}
+
+		/* waiting for Transmit ready condition */
+		status = wait_for_status_mask(I2C_STAT_XRDY | I2C_STAT_NACK);
 
-		if (status == 0 || status & I2C_STAT_NACK) {
+		if (status & (I2C_STAT_NACK | I2C_STAT_TIMEO))
 			i2c_error = 1;
-			printf("i2c error waiting for data ACK (status=0x%x)\n",
-					status);
-			goto write_exit;
+
+		if (!i2c_error) {
+			for (i = ((alen > 1) ? 0 : 1); i < len; i++) {
+				if (status & I2C_STAT_XRDY) {
+#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX)
+					writeb((buffer[i] & 0xFF),
+						&i2c_base->data);
+#else
+					writew((((buffer[i] << 8) |
+					buffer[i + 1]) & 0xFFFF),
+						&i2c_base->data);
+					i++;
+#endif
+				} else
+					i2c_error = 1;
+					/* Clearing XRDY event */
+					writew((status & I2C_STAT_XRDY),
+						&i2c_base->stat);
+					/* waiting for XRDY condition */
+					status = wait_for_status_mask(
+						I2C_STAT_XRDY |
+						I2C_STAT_ARDY |
+						I2C_STAT_NACK);
+					if (status & (I2C_STAT_NACK |
+						I2C_STAT_TIMEO)) {
+						i2c_error = 1;
+						break;
+					}
+					if (status & I2C_STAT_ARDY)
+						break;
+			}
 		}
+	}
 
-		if (status & I2C_STAT_XRDY) {
-			writeb(buffer[i], &i2c_base->data);
-			writew(I2C_STAT_XRDY, &i2c_base->stat);
-		} else {
-			i2c_error = 1;
-			printf("i2c bus not ready for Tx (i=%d)\n", i);
-			goto write_exit;
+	status = wait_for_status_mask(I2C_STAT_ARDY | I2C_STAT_NACK |
+				I2C_STAT_AL);
+
+	if (status & (I2C_STAT_NACK | I2C_STAT_TIMEO))
+		i2c_error = 1;
+
+	if (i2c_error) {
+		writew(0, &i2c_base->con);
+		return 1;
+	}
+
+	if (!i2c_error) {
+		int eout = 200;
+
+		writew(I2C_CON_EN, &i2c_base->con);
+		while ((status = readw(&i2c_base->stat)) ||
+				(readw(&i2c_base->con) & I2C_CON_MST)) {
+			udelay(1000);
+			/* have to read to clear intrrupt */
+			writew(0xFFFF, &i2c_base->stat);
+			if (--eout == 0)
+				/* better leave with error than hang */
+				break;
 		}
 	}
 
-write_exit:
 	flush_fifo();
 	writew(0xFFFF, &i2c_base->stat);
-	return i2c_error;
+	writew(0, &i2c_base->cnt);
+	return 0;
 }
 
-static void wait_for_bb(void)
+static u32 wait_for_bb(void)
 {
 	int timeout = I2C_TIMEOUT;
-	u16 stat;
+	u32 stat;
 
-	writew(0xFFFF, &i2c_base->stat);	/* clear current interrupts...*/
 	while ((stat = readw(&i2c_base->stat) & I2C_STAT_BB) && timeout--) {
 		writew(stat, &i2c_base->stat);
 		udelay(1000);
@@ -394,30 +517,28 @@  static void wait_for_bb(void)
 	if (timeout <= 0) {
 		printf("timed out in wait_for_bb: I2C_STAT=%x\n",
 			readw(&i2c_base->stat));
+		stat |= I2C_STAT_TIMEO;
 	}
 	writew(0xFFFF, &i2c_base->stat);	 /* clear delayed stuff*/
+	return stat;
 }
 
-static u16 wait_for_pin(void)
+static u32 wait_for_status_mask(u16 mask)
 {
-	u16 status;
+	u32 status;
 	int timeout = I2C_TIMEOUT;
 
 	do {
 		udelay(1000);
 		status = readw(&i2c_base->stat);
-	} while (!(status &
-		   (I2C_STAT_ROVR | I2C_STAT_XUDF | I2C_STAT_XRDY |
-		    I2C_STAT_RRDY | I2C_STAT_ARDY | I2C_STAT_NACK |
-		    I2C_STAT_AL)) && timeout--);
+	} while (!(status & mask) && timeout--);
 
 	if (timeout <= 0) {
-		printf("timed out in wait_for_pin: I2C_STAT=%x\n",
+		printf("timed out in wait_for_status_mask: I2C_STAT=%x\n",
 			readw(&i2c_base->stat));
 		writew(0xFFFF, &i2c_base->stat);
-		status = 0;
+		status |= I2C_STAT_TIMEO;
 	}
-
 	return status;
 }
 
diff --git a/drivers/i2c/omap24xx_i2c.h b/drivers/i2c/omap24xx_i2c.h
index 1f38c23..9a0b126 100644
--- a/drivers/i2c/omap24xx_i2c.h
+++ b/drivers/i2c/omap24xx_i2c.h
@@ -60,7 +60,9 @@ 
 /* I2C Buffer Configuration Register (I2C_BUF): */
 
 #define I2C_BUF_RDMA_EN		(1 << 15) /* Receive DMA channel enable */
+#define I2C_RXFIFO_CLEAR	(1 << 14) /* RX FIFO Clear */
 #define I2C_BUF_XDMA_EN		(1 << 7)  /* Transmit DMA channel enable */
+#define I2C_TXFIFO_CLEAR	(1 << 6)  /* TX FIFO clear */
 
 /* I2C Configuration Register (I2C_CON): */