[PATCH 0/5] Blackfin I2C updates

[PATCH 0/5] Blackfin I2C updates

[Mike Frysinger]

There were posted a few months ago, but seem to have stalled.

These can also be pulled from:
	  git://git.kernel.org/pub/scm/linux/kernel/git/vapier/blackfin.git for-i2c

Michael Hennerich (1):
  i2c-bfin-twi: add debug output for error status

Sonic Zhang (4):
  i2c-bfin-twi: integrate timeout timer with completion interface
  i2c-bfin-twi: fix lost interrupts at high speeds
  i2c-bfin-twi: remove redundant retry
  i2c-bfin-twi: return completion in interrupt for smbus quick
    transfers

 drivers/i2c/busses/i2c-bfin-twi.c |  168 +++++++++++++++++--------------------
 1 files changed, 78 insertions(+), 90 deletions(-)

[PATCH 1/5] i2c-bfin-twi: integrate timeout timer with completion interface

[Mike Frysinger]

From: Sonic Zhang <sonic.zhang@analog.com>

There isn't much point in managing our own custom timeout timer when the
completion interface already includes support for it.  This makes the
resulting code much simpler and robust.

Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/i2c/busses/i2c-bfin-twi.c |  125 +++++++++++++++++++-----------------
 1 files changed, 66 insertions(+), 59 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index fe3fb56..e016b1b 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -24,8 +24,6 @@
 #include <asm/portmux.h>
 #include <asm/irq.h>
 
-#define POLL_TIMEOUT       (2 * HZ)
-
 /* SMBus mode*/
 #define TWI_I2C_MODE_STANDARD		1
 #define TWI_I2C_MODE_STANDARDSUB	2
@@ -43,8 +41,6 @@ struct bfin_twi_iface {
 	int			cur_mode;
 	int			manual_stop;
 	int			result;
-	int			timeout_count;
-	struct timer_list	timeout_timer;
 	struct i2c_adapter	adap;
 	struct completion	complete;
 	struct i2c_msg 		*pmsg;
@@ -168,16 +164,13 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
 			write_INT_MASK(iface, 0);
 			write_MASTER_CTL(iface, 0);
 			SSYNC();
-			/* If it is a quick transfer, only address bug no data,
+			/* If it is a quick transfer, only address without data,
 			 * not an err, return 1.
+			 * If address is acknowledged return 1.
 			 */
-			if (iface->writeNum == 0 && (mast_stat & BUFRDERR))
+			if ((iface->writeNum == 0 && (mast_stat & BUFRDERR))
+				|| !(mast_stat & ANAK))
 				iface->result = 1;
-			/* If address not acknowledged return -1,
-			 * else return 0.
-			 */
-			else if (!(mast_stat & ANAK))
-				iface->result = 0;
 		}
 		complete(&iface->complete);
 		return;
@@ -249,9 +242,9 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
 			write_INT_MASK(iface, 0);
 			write_MASTER_CTL(iface, 0);
 			SSYNC();
-			complete(&iface->complete);
 		}
 	}
+	complete(&iface->complete);
 }
 
 /* Interrupt handler */
@@ -261,36 +254,15 @@ static irqreturn_t bfin_twi_interrupt_entry(int irq, void *dev_id)
 	unsigned long flags;
 
 	spin_lock_irqsave(&iface->lock, flags);
-	del_timer(&iface->timeout_timer);
 	bfin_twi_handle_interrupt(iface);
 	spin_unlock_irqrestore(&iface->lock, flags);
 	return IRQ_HANDLED;
 }
 
-static void bfin_twi_timeout(unsigned long data)
-{
-	struct bfin_twi_iface *iface = (struct bfin_twi_iface *)data;
-	unsigned long flags;
-
-	spin_lock_irqsave(&iface->lock, flags);
-	bfin_twi_handle_interrupt(iface);
-	if (iface->result == 0) {
-		iface->timeout_count--;
-		if (iface->timeout_count > 0) {
-			iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
-			add_timer(&iface->timeout_timer);
-		} else {
-			iface->result = -1;
-			complete(&iface->complete);
-		}
-	}
-	spin_unlock_irqrestore(&iface->lock, flags);
-}
-
 /*
- * Generic i2c master transfer entrypoint
+ * One i2c master transfer
  */
-static int bfin_twi_master_xfer(struct i2c_adapter *adap,
+static int bfin_twi_do_master_xfer(struct i2c_adapter *adap,
 				struct i2c_msg *msgs, int num)
 {
 	struct bfin_twi_iface *iface = adap->algo_data;
@@ -318,7 +290,6 @@ static int bfin_twi_master_xfer(struct i2c_adapter *adap,
 	iface->transPtr = pmsg->buf;
 	iface->writeNum = iface->readNum = pmsg->len;
 	iface->result = 0;
-	iface->timeout_count = 10;
 	init_completion(&(iface->complete));
 	/* Set Transmit device address */
 	write_MASTER_ADDR(iface, pmsg->addr);
@@ -357,30 +328,49 @@ static int bfin_twi_master_xfer(struct i2c_adapter *adap,
 		iface->manual_stop = 1;
 	}
 
-	iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
-	add_timer(&iface->timeout_timer);
-
 	/* Master enable */
 	write_MASTER_CTL(iface, read_MASTER_CTL(iface) | MEN |
 		((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0) |
 		((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ > 100) ? FAST : 0));
 	SSYNC();
 
-	wait_for_completion(&iface->complete);
-
-	rc = iface->result;
+	while (!iface->result) {
+		if (!wait_for_completion_timeout(&iface->complete,
+			adap->timeout)) {
+			iface->result = -1;
+			dev_err(&adap->dev, "master transfer timeout\n");
+		}
+	}
 
-	if (rc == 1)
-		return num;
+	if (iface->result == 1)
+		rc = iface->cur_msg + 1;
 	else
-		return rc;
+		rc = iface->result;
+
+	return rc;
 }
 
 /*
- * SMBus type transfer entrypoint
+ * Generic i2c master transfer entrypoint
  */
+static int bfin_twi_master_xfer(struct i2c_adapter *adap,
+				struct i2c_msg *msgs, int num)
+{
+	int i, ret = 0;
 
-int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
+	for (i = 0; i < adap->retries; i++) {
+		ret = bfin_twi_do_master_xfer(adap, msgs, num);
+		if (ret > 0)
+			break;
+	}
+
+	return ret;
+}
+
+/*
+ * One I2C SMBus transfer
+ */
+int bfin_twi_do_smbus_xfer(struct i2c_adapter *adap, u16 addr,
 			unsigned short flags, char read_write,
 			u8 command, int size, union i2c_smbus_data *data)
 {
@@ -468,7 +458,6 @@ int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
 	iface->manual_stop = 0;
 	iface->read_write = read_write;
 	iface->command = command;
-	iface->timeout_count = 10;
 	init_completion(&(iface->complete));
 
 	/* FIFO Initiation. Data in FIFO should be discarded before
@@ -485,9 +474,6 @@ int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
 	write_MASTER_ADDR(iface, addr);
 	SSYNC();
 
-	iface->timeout_timer.expires = jiffies + POLL_TIMEOUT;
-	add_timer(&iface->timeout_timer);
-
 	switch (iface->cur_mode) {
 	case TWI_I2C_MODE_STANDARDSUB:
 		write_XMT_DATA8(iface, iface->command);
@@ -549,10 +535,8 @@ int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
 				else if (iface->readNum > 255) {
 					write_MASTER_CTL(iface, 0xff << 6);
 					iface->manual_stop = 1;
-				} else {
-					del_timer(&iface->timeout_timer);
+				} else
 					break;
-				}
 			}
 		}
 		write_INT_MASK(iface, MCOMP | MERR |
@@ -568,7 +552,13 @@ int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
 	}
 	SSYNC();
 
-	wait_for_completion(&iface->complete);
+	while (!iface->result) {
+		if (!wait_for_completion_timeout(&iface->complete,
+			adap->timeout)) {
+			iface->result = -1;
+			dev_err(&adap->dev, "smbus transfer timeout\n");
+		}
+	}
 
 	rc = (iface->result >= 0) ? 0 : -1;
 
@@ -576,6 +566,25 @@ int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
 }
 
 /*
+ * Generic I2C SMBus transfer entrypoint
+ */
+int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
+			unsigned short flags, char read_write,
+			u8 command, int size, union i2c_smbus_data *data)
+{
+	int i, ret = 0;
+
+	for (i = 0; i < adap->retries; i++) {
+		ret = bfin_twi_do_smbus_xfer(adap, addr, flags,
+			read_write, command, size, data);
+		if (ret == 0)
+			break;
+	}
+
+	return ret;
+}
+
+/*
  * Return what the adapter supports
  */
 static u32 bfin_twi_functionality(struct i2c_adapter *adap)
@@ -666,10 +675,6 @@ static int i2c_bfin_twi_probe(struct platform_device *pdev)
 		goto out_error_no_irq;
 	}
 
-	init_timer(&(iface->timeout_timer));
-	iface->timeout_timer.function = bfin_twi_timeout;
-	iface->timeout_timer.data = (unsigned long)iface;
-
 	p_adap = &iface->adap;
 	p_adap->nr = pdev->id;
 	strlcpy(p_adap->name, pdev->name, sizeof(p_adap->name));
@@ -677,6 +682,8 @@ static int i2c_bfin_twi_probe(struct platform_device *pdev)
 	p_adap->algo_data = iface;
 	p_adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
 	p_adap->dev.parent = &pdev->dev;
+	p_adap->timeout = 5 * HZ;
+	p_adap->retries = 3;
 
 	rc = peripheral_request_list(pin_req[pdev->id], "i2c-bfin-twi");
 	if (rc) {
-- 
1.7.0.2

[PATCH 2/5] i2c-bfin-twi: add debug output for error status

[Mike Frysinger]

From: Michael Hennerich <michael.hennerich@analog.com>

Add some debug() code to decode the error register.

Signed-off-by: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/i2c/busses/i2c-bfin-twi.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index e016b1b..758bb1f 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -156,6 +156,18 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
 		write_MASTER_CTL(iface, 0);
 		SSYNC();
 		iface->result = -EIO;
+
+		if (mast_stat & LOSTARB)
+			dev_dbg(&iface->adap.dev, "Lost Arbitration\n");
+		if (mast_stat & ANAK)
+			dev_dbg(&iface->adap.dev, "Address Not Acknowledged\n");
+		if (mast_stat & DNAK)
+			dev_dbg(&iface->adap.dev, "Data Not Acknowledged\n");
+		if (mast_stat & BUFRDERR)
+			dev_dbg(&iface->adap.dev, "Buffer Read Error\n");
+		if (mast_stat & BUFWRERR)
+			dev_dbg(&iface->adap.dev, "Buffer Write Error\n");
+
 		/* if both err and complete int stats are set, return proper
 		 * results.
 		 */
-- 
1.7.0.2

[PATCH 3/5] i2c-bfin-twi: fix lost interrupts at high speeds

[Mike Frysinger]

From: Sonic Zhang <sonic.zhang-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>

i2c event of next read/write byte may trigger before current int state
is cleared in the interrupt handler. So, this should be done at the
beginning of interrupt handler to avoid losing new i2c events.

Signed-off-by: Sonic Zhang <sonic.zhang-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 drivers/i2c/busses/i2c-bfin-twi.c |   37 +++++++++++++------------------------
 1 files changed, 13 insertions(+), 24 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index 758bb1f..3495019 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -80,14 +80,15 @@ static const u16 pin_req[2][3] = {
 	{P_TWI1_SCL, P_TWI1_SDA, 0},
 };
 
-static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
+static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
+					unsigned short twi_int_status)
 {
-	unsigned short twi_int_status = read_INT_STAT(iface);
 	unsigned short mast_stat = read_MASTER_STAT(iface);
 
 	if (twi_int_status & XMTSERV) {
 		/* Transmit next data */
 		if (iface->writeNum > 0) {
+			SSYNC();
 			write_XMT_DATA8(iface, *(iface->transPtr++));
 			iface->writeNum--;
 		}
@@ -109,10 +110,6 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
 				write_MASTER_CTL(iface,
 					(read_MASTER_CTL(iface) | RSTART) & ~MDIR);
 		}
-		SSYNC();
-		/* Clear status */
-		write_INT_STAT(iface, XMTSERV);
-		SSYNC();
 	}
 	if (twi_int_status & RCVSERV) {
 		if (iface->readNum > 0) {
@@ -134,7 +131,6 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
 		} else if (iface->manual_stop) {
 			write_MASTER_CTL(iface,
 				read_MASTER_CTL(iface) | STOP);
-			SSYNC();
 		} else if (iface->cur_mode == TWI_I2C_MODE_REPEAT &&
 		           iface->cur_msg + 1 < iface->msg_num) {
 			if (iface->pmsg[iface->cur_msg + 1].flags & I2C_M_RD)
@@ -143,18 +139,12 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
 			else
 				write_MASTER_CTL(iface,
 					(read_MASTER_CTL(iface) | RSTART) & ~MDIR);
-			SSYNC();
 		}
-		/* Clear interrupt source */
-		write_INT_STAT(iface, RCVSERV);
-		SSYNC();
 	}
 	if (twi_int_status & MERR) {
-		write_INT_STAT(iface, MERR);
 		write_INT_MASK(iface, 0);
 		write_MASTER_STAT(iface, 0x3e);
 		write_MASTER_CTL(iface, 0);
-		SSYNC();
 		iface->result = -EIO;
 
 		if (mast_stat & LOSTARB)
@@ -172,10 +162,6 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
 		 * results.
 		 */
 		if (twi_int_status & MCOMP) {
-			write_INT_STAT(iface, MCOMP);
-			write_INT_MASK(iface, 0);
-			write_MASTER_CTL(iface, 0);
-			SSYNC();
 			/* If it is a quick transfer, only address without data,
 			 * not an err, return 1.
 			 * If address is acknowledged return 1.
@@ -188,8 +174,6 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
 		return;
 	}
 	if (twi_int_status & MCOMP) {
-		write_INT_STAT(iface, MCOMP);
-		SSYNC();
 		if (iface->cur_mode == TWI_I2C_MODE_COMBINED) {
 			if (iface->readNum == 0) {
 				/* set the read number to 1 and ask for manual
@@ -211,7 +195,6 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
 			/* remove restart bit and enable master receive */
 			write_MASTER_CTL(iface,
 				read_MASTER_CTL(iface) & ~RSTART);
-			SSYNC();
 		} else if (iface->cur_mode == TWI_I2C_MODE_REPEAT &&
 				iface->cur_msg+1 < iface->msg_num) {
 			iface->cur_msg++;
@@ -230,7 +213,6 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
 					write_XMT_DATA8(iface,
 						*(iface->transPtr++));
 					iface->writeNum--;
-					SSYNC();
 				}
 			}
 
@@ -248,12 +230,10 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface)
 			/* remove restart bit and enable master receive */
 			write_MASTER_CTL(iface,
 				read_MASTER_CTL(iface) & ~RSTART);
-			SSYNC();
 		} else {
 			iface->result = 1;
 			write_INT_MASK(iface, 0);
 			write_MASTER_CTL(iface, 0);
-			SSYNC();
 		}
 	}
 	complete(&iface->complete);
@@ -264,9 +244,18 @@ static irqreturn_t bfin_twi_interrupt_entry(int irq, void *dev_id)
 {
 	struct bfin_twi_iface *iface = dev_id;
 	unsigned long flags;
+	unsigned short twi_int_status;
 
 	spin_lock_irqsave(&iface->lock, flags);
-	bfin_twi_handle_interrupt(iface);
+	while (1) {
+		twi_int_status = read_INT_STAT(iface);
+		if (!twi_int_status)
+			break;
+		/* Clear interrupt status */
+		write_INT_STAT(iface, twi_int_status);
+		bfin_twi_handle_interrupt(iface, twi_int_status);
+		SSYNC();
+	}
 	spin_unlock_irqrestore(&iface->lock, flags);
 	return IRQ_HANDLED;
 }
-- 
1.7.0.2

[PATCH 4/5] i2c-bfin-twi: remove redundant retry

[Mike Frysinger]

From: Sonic Zhang <sonic.zhang-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>

Signed-off-by: Sonic Zhang <sonic.zhang-OyLXuOCK7orQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 drivers/i2c/busses/i2c-bfin-twi.c |   20 ++------------------
 1 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index 3495019..d157105 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -357,15 +357,7 @@ static int bfin_twi_do_master_xfer(struct i2c_adapter *adap,
 static int bfin_twi_master_xfer(struct i2c_adapter *adap,
 				struct i2c_msg *msgs, int num)
 {
-	int i, ret = 0;
-
-	for (i = 0; i < adap->retries; i++) {
-		ret = bfin_twi_do_master_xfer(adap, msgs, num);
-		if (ret > 0)
-			break;
-	}
-
-	return ret;
+	return bfin_twi_do_master_xfer(adap, msgs, num);
 }
 
 /*
@@ -573,16 +565,8 @@ int bfin_twi_smbus_xfer(struct i2c_adapter *adap, u16 addr,
 			unsigned short flags, char read_write,
 			u8 command, int size, union i2c_smbus_data *data)
 {
-	int i, ret = 0;
-
-	for (i = 0; i < adap->retries; i++) {
-		ret = bfin_twi_do_smbus_xfer(adap, addr, flags,
+	return bfin_twi_do_smbus_xfer(adap, addr, flags,
 			read_write, command, size, data);
-		if (ret == 0)
-			break;
-	}
-
-	return ret;
 }
 
 /*
-- 
1.7.0.2

[PATCH 5/5] i2c-bfin-twi: return completion in interrupt for smbus quick transfers

[Mike Frysinger]

From: Sonic Zhang <sonic.zhang@analog.com>

A smbus quick transfer has no data after the address byte.

Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/i2c/busses/i2c-bfin-twi.c |   18 +++++++-----------
 1 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index d157105..02a8c86 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -158,18 +158,14 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
 		if (mast_stat & BUFWRERR)
 			dev_dbg(&iface->adap.dev, "Buffer Write Error\n");
 
-		/* if both err and complete int stats are set, return proper
-		 * results.
+		/* If it is a quick transfer, only address without data,
+		 * not an err, return 1.
 		 */
-		if (twi_int_status & MCOMP) {
-			/* If it is a quick transfer, only address without data,
-			 * not an err, return 1.
-			 * If address is acknowledged return 1.
-			 */
-			if ((iface->writeNum == 0 && (mast_stat & BUFRDERR))
-				|| !(mast_stat & ANAK))
-				iface->result = 1;
-		}
+		if (iface->cur_mode == TWI_I2C_MODE_STANDARD &&
+			iface->transPtr == NULL &&
+			(twi_int_status & MCOMP) && (mast_stat & DNAK))
+			iface->result = 1;
+
 		complete(&iface->complete);
 		return;
 	}
-- 
1.7.0.2

Re: [Uclinux-dist-devel] [PATCH 0/5] Blackfin I2C updates

[Mike Frysinger]

On Mon, Mar 22, 2010 at 03:23, Mike Frysinger wrote:
> There were posted a few months ago, but seem to have stalled.
>
> These can also be pulled from:
>          git://git.kernel.org/pub/scm/linux/kernel/git/vapier/blackfin.git for-i2c

ping ...
-mike