Patchwork [U-Boot,07/24] mxc_i2c: combine i2c_imx_bus_busy and i2c_imx_trx_complete into wait_for_sr_state

login
register
mail settings
Submitter Troy Kisky
Date June 22, 2012, 4:12 a.m.
Message ID <1340338339-11626-7-git-send-email-troy.kisky@boundarydevices.com>
Download mbox | patch
Permalink /patch/166478/
State Superseded
Delegated to: Heiko Schocher
Headers show

Comments

Troy Kisky - June 22, 2012, 4:12 a.m.
Not using udelay gives a more accurate timeout. The current implementation of udelay
in imx-common does not seem to wait at all for a udelay(1).

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 drivers/i2c/mxc_i2c.c |   71 ++++++++++++++++---------------------------------
 1 file changed, 23 insertions(+), 48 deletions(-)
Marek Vasut - June 22, 2012, 5:01 p.m.
Dear Troy Kisky,

> Not using udelay gives a more accurate timeout. The current implementation
> of udelay in imx-common does not seem to wait at all for a udelay(1).
> 

Add WATCHDOG_RESET() please.

> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---

Best regards,
Marek Vasut
Troy Kisky - June 22, 2012, 5:34 p.m.
On 6/22/2012 10:01 AM, Marek Vasut wrote:
> Dear Troy Kisky,
>
>> Not using udelay gives a more accurate timeout. The current implementation
>> of udelay in imx-common does not seem to wait at all for a udelay(1).
>>
> Add WATCHDOG_RESET() please.

Are you sure it is needed for a 0.1 second max delay?

>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>> ---
> Best regards,
> Marek Vasut
>
Troy Kisky - June 22, 2012, 6:01 p.m.
On 6/22/2012 10:34 AM, Troy Kisky wrote:
> On 6/22/2012 10:01 AM, Marek Vasut wrote:
>> Dear Troy Kisky,
>>
>>> Not using udelay gives a more accurate timeout. The current 
>>> implementation
>>> of udelay in imx-common does not seem to wait at all for a udelay(1).
>>>
>> Add WATCHDOG_RESET() please.
>
> Are you sure it is needed for a 0.1 second max delay?

Hmm, I agree. Retries could increase the delay.


>
>>> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
>>> ---
>> Best regards,
>> Marek Vasut
>>
>
>

Thanks for the reviews Marek.

Troy

Patch

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 4f12b9e..7b1b75c 100644
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -63,8 +63,6 @@  struct mxc_i2c_regs {
 #error "define CONFIG_SYS_I2C_BASE to use the mxc_i2c driver"
 #endif
 
-#define I2C_MAX_TIMEOUT		10000
-
 static u16 i2c_clk_div[50][2] = {
 	{ 22,	0x20 }, { 24,	0x21 }, { 26,	0x22 }, { 28,	0x23 },
 	{ 30,	0x00 }, { 32,	0x24 }, { 36,	0x25 }, { 40,	0x26 },
@@ -164,48 +162,25 @@  unsigned int i2c_get_bus_speed(void)
 	return mxc_get_clock(MXC_IPG_PERCLK) / i2c_clk_div[clk_div][0];
 }
 
-/*
- * Wait for bus to be busy (or free if for_busy = 0)
- *
- * for_busy = 1: Wait for IBB to be asserted
- * for_busy = 0: Wait for IBB to be de-asserted
- */
-int i2c_imx_bus_busy(int for_busy)
-{
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	unsigned int temp;
-
-	int timeout = I2C_MAX_TIMEOUT;
-
-	while (timeout--) {
-		temp = readb(&i2c_regs->i2sr);
+#define ST_BUS_IDLE (0 | (I2SR_IBB << 8))
+#define ST_BUS_BUSY (I2SR_IBB | (I2SR_IBB << 8))
+#define ST_IIF (I2SR_IIF | (I2SR_IIF << 8))
 
-		if (for_busy && (temp & I2SR_IBB))
-			return 0;
-		if (!for_busy && !(temp & I2SR_IBB))
-			return 0;
-
-		udelay(1);
-	}
-
-	return 1;
-}
-
-/*
- * Wait for transaction to complete
- */
-int i2c_imx_trx_complete(void)
+static unsigned wait_for_sr_state(struct mxc_i2c_regs *i2c_regs, unsigned state)
 {
-	struct mxc_i2c_regs *i2c_regs = (struct mxc_i2c_regs *)I2C_BASE;
-	int timeout = I2C_MAX_TIMEOUT;
-
-	while (timeout--) {
-		if (readb(&i2c_regs->i2sr) & I2SR_IIF)
-			return 0;
-
-		udelay(1);
+	unsigned sr;
+	ulong elapsed;
+	ulong start_time = get_timer(0);
+	for (;;) {
+		sr = readb(&i2c_regs->i2sr);
+		if ((sr & (state >> 8)) == (unsigned char)state)
+			return sr;
+		elapsed = get_timer(start_time);
+		if (elapsed > (CONFIG_SYS_HZ / 10))	/* .1 seconds */
+			break;
 	}
-
+	printf("%s: failed sr=%x cr=%x state=%x\n", __func__,
+			sr, readb(&i2c_regs->i2cr), state);
 	return -ETIMEDOUT;
 }
 
@@ -215,7 +190,7 @@  static int tx_byte(struct mxc_i2c_regs *i2c_regs, u8 byte)
 
 	writeb(0, &i2c_regs->i2sr);
 	writeb(byte, &i2c_regs->i2dr);
-	ret = i2c_imx_trx_complete();
+	ret = wait_for_sr_state(i2c_regs, ST_IIF);
 	if (ret < 0)
 		return ret;
 	ret = readb(&i2c_regs->i2sr);
@@ -245,8 +220,8 @@  int i2c_imx_start(void)
 	temp |= I2CR_MSTA;
 	writeb(temp, &i2c_regs->i2cr);
 
-	result = i2c_imx_bus_busy(1);
-	if (result)
+	result = wait_for_sr_state(i2c_regs, ST_BUS_BUSY);
+	if (result < 0)
 		return result;
 
 	temp |= I2CR_MTX | I2CR_TX_NO_AK;
@@ -268,7 +243,7 @@  void i2c_imx_stop(void)
 	temp &= ~(I2CR_MSTA | I2CR_MTX);
 	writeb(temp, &i2c_regs->i2cr);
 
-	i2c_imx_bus_busy(0);
+	wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
 
 	/* Disable I2C controller */
 	writeb(0, &i2c_regs->i2cr);
@@ -333,8 +308,8 @@  int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 
 	/* read data */
 	for (i = 0; i < len; i++) {
-		ret = i2c_imx_trx_complete();
-		if (ret)
+		ret = wait_for_sr_state(i2c_regs, ST_IIF);
+		if (ret < 0)
 			return ret;
 
 		/*
@@ -345,7 +320,7 @@  int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 			temp = readb(&i2c_regs->i2cr);
 			temp &= ~(I2CR_MSTA | I2CR_MTX);
 			writeb(temp, &i2c_regs->i2cr);
-			i2c_imx_bus_busy(0);
+			wait_for_sr_state(i2c_regs, ST_BUS_IDLE);
 		} else if (i == (len - 2)) {
 			temp = readb(&i2c_regs->i2cr);
 			temp |= I2CR_TX_NO_AK;