[PATCH v2] i2c-designware updates

[PATCH v2] i2c-designware updates

[Shinya Kuribayashi]

Hi Baruch and Ben,

here's v2 patchset of DesignWare I2C driver (i2c-designware.c).
I did all test I can do for now (including arbitration tests), and
fixed more issues left in the v1 patchset.  I think now the driver is
in reasonable shape for the mainline, and hope gets merged.

The patches which already have Baruch's Acked-by: line are logically
same as the previous v1 patches.  For the rest are revised version of
v1 patches, or newly added in v2.

There are 22 patches in total and might be slightly annoying.  I'll
send subsequent patches only to linux-i2c.


Changes in v2
==============

* i2c-designware: Set a clock name to DW I2C clock source

  -> dropped.

* 02/22: Don't use the IC_CLR_INTR register to clear interrupts

  We need to preserve IC_TX_ABRT_SOURCE value before clearing
  interrupts, or we can't get correct abort_source.  ->Fixed

* 08/22: i2c_dw_xfer_msg: Fix i2c_msg search bug

  The intr_mask handling is folded into the for-loop.

* 15/22: i2c_dw_func: Set I2C_FUNC_SMBUS_foo bits

  Commit log is updated.

* 01--15 (except for 02, 08, and 15) are logically same as the
  previous v1 patches.

* For the rest (16--22) are newly added.


Shinya Kuribayashi (22):
      i2c-designware: Consolidate to use 32-bit word accesses
      i2c-designware: Don't use the IC_CLR_INTR register to clear interrupts
      i2c-designware: Use platform_get_irq helper
      i2c-designware: i2c_dw_read: Use "struct dw_i2c_dev" pointer
      i2c-designware: i2c_dw_xfer_msg: Use "struct dw_i2c_dev" pointer
      i2c-designware: Remove an useless local variable "num"
      i2c-designware: Improved _HCNT/_LCNT calculation
      i2c-designware: i2c_dw_xfer_msg: Fix i2c_msg search bug
      i2c-designware: Process i2c_msg messages in the interrupt handler
      i2c-designware: Set Tx/Rx FIFO threshold levels
      i2c-designware: Enable RX_FULL interrupt
      i2c-designware: Divide i2c_dw_xfer_msg into two functions
      i2c-designware: i2c_dw_xfer_msg: Introduce a local "buf" pointer
      i2c-designware: Initialize byte count variables just prior to being used
      i2c-designware: i2c_dw_func: Set I2C_FUNC_SMBUS_foo bits
      i2c-designware: i2c_dw_read: Remove redundant target address checker
      i2c-designware: Process all i2c_msg messages in the interrupt handler
      i2c-designware: Disable TX_EMPTY when all i2c_msg msgs has been processed
      i2c-designware: i2c_dw_xfer_msg: Fix error handling procedures
      i2c-designware: Skip RX_FULL and TX_EMPTY bits on tx abort errors
      i2c-designware: Tx abort cleanups
      i2c-designware: Cosmetic cleanups

 drivers/i2c/busses/i2c-designware.c |  483 +++++++++++++++++++++++++----------
 1 files changed, 350 insertions(+), 133 deletions(-)

-- 
Shinya Kuribayashi
NEC Electronics

[PATCH v2] i2c-designware updates

[Shinya Kuribayashi]

Hi Baruch and Ben,

here's v2 patchset of DesignWare I2C driver (i2c-designware.c).
I did all test I can do for now (including arbitration tests), and
fixed more issues left in the v1 patchset.  I think now the driver is
in reasonable shape for the mainline, and hope gets merged.

The patches which already have Baruch's Acked-by: line are logically
same as the previous v1 patches.  For the rest are revised version of
v1 patches, or newly added in v2.

There are 22 patches in total and might be slightly annoying.  I'll
send subsequent patches only to linux-i2c.


Changes in v2
==============

* i2c-designware: Set a clock name to DW I2C clock source

  -> dropped.

* 02/22: Don't use the IC_CLR_INTR register to clear interrupts

  We need to preserve IC_TX_ABRT_SOURCE value before clearing
  interrupts, or we can't get correct abort_source.  ->Fixed

* 08/22: i2c_dw_xfer_msg: Fix i2c_msg search bug

  The intr_mask handling is folded into the for-loop.

* 15/22: i2c_dw_func: Set I2C_FUNC_SMBUS_foo bits

  Commit log is updated.

* 01--15 (except for 02, 08, and 15) are logically same as the
  previous v1 patches.

* For the rest (16--22) are newly added.


Shinya Kuribayashi (22):
      i2c-designware: Consolidate to use 32-bit word accesses
      i2c-designware: Don't use the IC_CLR_INTR register to clear interrupts
      i2c-designware: Use platform_get_irq helper
      i2c-designware: i2c_dw_read: Use "struct dw_i2c_dev" pointer
      i2c-designware: i2c_dw_xfer_msg: Use "struct dw_i2c_dev" pointer
      i2c-designware: Remove an useless local variable "num"
      i2c-designware: Improved _HCNT/_LCNT calculation
      i2c-designware: i2c_dw_xfer_msg: Fix i2c_msg search bug
      i2c-designware: Process i2c_msg messages in the interrupt handler
      i2c-designware: Set Tx/Rx FIFO threshold levels
      i2c-designware: Enable RX_FULL interrupt
      i2c-designware: Divide i2c_dw_xfer_msg into two functions
      i2c-designware: i2c_dw_xfer_msg: Introduce a local "buf" pointer
      i2c-designware: Initialize byte count variables just prior to being used
      i2c-designware: i2c_dw_func: Set I2C_FUNC_SMBUS_foo bits
      i2c-designware: i2c_dw_read: Remove redundant target address checker
      i2c-designware: Process all i2c_msg messages in the interrupt handler
      i2c-designware: Disable TX_EMPTY when all i2c_msg msgs has been processed
      i2c-designware: i2c_dw_xfer_msg: Fix error handling procedures
      i2c-designware: Skip RX_FULL and TX_EMPTY bits on tx abort errors
      i2c-designware: Tx abort cleanups
      i2c-designware: Cosmetic cleanups

 drivers/i2c/busses/i2c-designware.c |  483 +++++++++++++++++++++++++----------
 1 files changed, 350 insertions(+), 133 deletions(-)

-- 
Shinya Kuribayashi
NEC Electronics

[PATCH v2] i2c-designware updates

[Shinya Kuribayashi]

Hi Baruch and Ben,

here's v2 patchset of DesignWare I2C driver (i2c-designware.c).
I did all test I can do for now (including arbitration tests), and
fixed more issues left in the v1 patchset.  I think now the driver is
in reasonable shape for the mainline, and hope gets merged.

The patches which already have Baruch's Acked-by: line are logically
same as the previous v1 patches.  For the rest are revised version of
v1 patches, or newly added in v2.

There are 22 patches in total and might be slightly annoying.  I'll
send subsequent patches only to linux-i2c.


Changes in v2
==============

* i2c-designware: Set a clock name to DW I2C clock source

  -> dropped.

* 02/22: Don't use the IC_CLR_INTR register to clear interrupts

  We need to preserve IC_TX_ABRT_SOURCE value before clearing
  interrupts, or we can't get correct abort_source.  ->Fixed

* 08/22: i2c_dw_xfer_msg: Fix i2c_msg search bug

  The intr_mask handling is folded into the for-loop.

* 15/22: i2c_dw_func: Set I2C_FUNC_SMBUS_foo bits

  Commit log is updated.

* 01--15 (except for 02, 08, and 15) are logically same as the
  previous v1 patches.

* For the rest (16--22) are newly added.


Shinya Kuribayashi (22):
      i2c-designware: Consolidate to use 32-bit word accesses
      i2c-designware: Don't use the IC_CLR_INTR register to clear interrupts
      i2c-designware: Use platform_get_irq helper
      i2c-designware: i2c_dw_read: Use "struct dw_i2c_dev" pointer
      i2c-designware: i2c_dw_xfer_msg: Use "struct dw_i2c_dev" pointer
      i2c-designware: Remove an useless local variable "num"
      i2c-designware: Improved _HCNT/_LCNT calculation
      i2c-designware: i2c_dw_xfer_msg: Fix i2c_msg search bug
      i2c-designware: Process i2c_msg messages in the interrupt handler
      i2c-designware: Set Tx/Rx FIFO threshold levels
      i2c-designware: Enable RX_FULL interrupt
      i2c-designware: Divide i2c_dw_xfer_msg into two functions
      i2c-designware: i2c_dw_xfer_msg: Introduce a local "buf" pointer
      i2c-designware: Initialize byte count variables just prior to being used
      i2c-designware: i2c_dw_func: Set I2C_FUNC_SMBUS_foo bits
      i2c-designware: i2c_dw_read: Remove redundant target address checker
      i2c-designware: Process all i2c_msg messages in the interrupt handler
      i2c-designware: Disable TX_EMPTY when all i2c_msg msgs has been processed
      i2c-designware: i2c_dw_xfer_msg: Fix error handling procedures
      i2c-designware: Skip RX_FULL and TX_EMPTY bits on tx abort errors
      i2c-designware: Tx abort cleanups
      i2c-designware: Cosmetic cleanups

 drivers/i2c/busses/i2c-designware.c |  483 +++++++++++++++++++++++++----------
 1 files changed, 350 insertions(+), 133 deletions(-)

-- 
Shinya Kuribayashi
NEC Electronics

[PATCH v2 01/22] i2c-designware: Consolidate to use 32-bit word accesses

[Shinya Kuribayashi]

This driver looks originally meant for armel machines where readw()/
writew() works perfectly fine with this hardware.  But that doens't
work for big-endian systems.

This patch converts all 8/16-bit-aware usages to 32-bit variants.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Acked-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |   76 +++++++++++++++++-----------------
 1 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index b444762..a4f928e 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -162,14 +162,14 @@ struct dw_i2c_dev {
 	struct i2c_msg		*msgs;
 	int			msgs_num;
 	int			msg_write_idx;
-	u16			tx_buf_len;
+	u32			tx_buf_len;
 	u8			*tx_buf;
 	int			msg_read_idx;
-	u16			rx_buf_len;
+	u32			rx_buf_len;
 	u8			*rx_buf;
 	int			msg_err;
 	unsigned int		status;
-	u16			abort_source;
+	u32			abort_source;
 	int			irq;
 	struct i2c_adapter	adapter;
 	unsigned int		tx_fifo_depth;
@@ -187,25 +187,25 @@ struct dw_i2c_dev {
 static void i2c_dw_init(struct dw_i2c_dev *dev)
 {
 	u32 input_clock_khz = clk_get_rate(dev->clk) / 1000;
-	u16 ic_con;
+	u32 ic_con;
 
 	/* Disable the adapter */
-	writeb(0, dev->base + DW_IC_ENABLE);
+	writel(0, dev->base + DW_IC_ENABLE);
 
 	/* set standard and fast speed deviders for high/low periods */
-	writew((input_clock_khz * 40 / 10000)+1, /* std speed high, 4us */
+	writel((input_clock_khz * 40 / 10000)+1, /* std speed high, 4us */
 			dev->base + DW_IC_SS_SCL_HCNT);
-	writew((input_clock_khz * 47 / 10000)+1, /* std speed low, 4.7us */
+	writel((input_clock_khz * 47 / 10000)+1, /* std speed low, 4.7us */
 			dev->base + DW_IC_SS_SCL_LCNT);
-	writew((input_clock_khz *  6 / 10000)+1, /* fast speed high, 0.6us */
+	writel((input_clock_khz *  6 / 10000)+1, /* fast speed high, 0.6us */
 			dev->base + DW_IC_FS_SCL_HCNT);
-	writew((input_clock_khz * 13 / 10000)+1, /* fast speed low, 1.3us */
+	writel((input_clock_khz * 13 / 10000)+1, /* fast speed low, 1.3us */
 			dev->base + DW_IC_FS_SCL_LCNT);
 
 	/* configure the i2c master */
 	ic_con = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
 		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
-	writew(ic_con, dev->base + DW_IC_CON);
+	writel(ic_con, dev->base + DW_IC_CON);
 }
 
 /*
@@ -215,7 +215,7 @@ static int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
 {
 	int timeout = TIMEOUT;
 
-	while (readb(dev->base + DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY) {
+	while (readl(dev->base + DW_IC_STATUS) & DW_IC_STATUS_ACTIVITY) {
 		if (timeout <= 0) {
 			dev_warn(dev->dev, "timeout waiting for bus ready\n");
 			return -ETIMEDOUT;
@@ -239,29 +239,29 @@ i2c_dw_xfer_msg(struct i2c_adapter *adap)
 	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
 	struct i2c_msg *msgs = dev->msgs;
 	int num = dev->msgs_num;
-	u16 ic_con, intr_mask;
-	int tx_limit = dev->tx_fifo_depth - readb(dev->base + DW_IC_TXFLR);
-	int rx_limit = dev->rx_fifo_depth - readb(dev->base + DW_IC_RXFLR);
-	u16 addr = msgs[dev->msg_write_idx].addr;
-	u16 buf_len = dev->tx_buf_len;
+	u32 ic_con, intr_mask;
+	int tx_limit = dev->tx_fifo_depth - readl(dev->base + DW_IC_TXFLR);
+	int rx_limit = dev->rx_fifo_depth - readl(dev->base + DW_IC_RXFLR);
+	u32 addr = msgs[dev->msg_write_idx].addr;
+	u32 buf_len = dev->tx_buf_len;
 
 	if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
 		/* Disable the adapter */
-		writeb(0, dev->base + DW_IC_ENABLE);
+		writel(0, dev->base + DW_IC_ENABLE);
 
 		/* set the slave (target) address */
-		writew(msgs[dev->msg_write_idx].addr, dev->base + DW_IC_TAR);
+		writel(msgs[dev->msg_write_idx].addr, dev->base + DW_IC_TAR);
 
 		/* if the slave address is ten bit address, enable 10BITADDR */
-		ic_con = readw(dev->base + DW_IC_CON);
+		ic_con = readl(dev->base + DW_IC_CON);
 		if (msgs[dev->msg_write_idx].flags & I2C_M_TEN)
 			ic_con |= DW_IC_CON_10BITADDR_MASTER;
 		else
 			ic_con &= ~DW_IC_CON_10BITADDR_MASTER;
-		writew(ic_con, dev->base + DW_IC_CON);
+		writel(ic_con, dev->base + DW_IC_CON);
 
 		/* Enable the adapter */
-		writeb(1, dev->base + DW_IC_ENABLE);
+		writel(1, dev->base + DW_IC_ENABLE);
 	}
 
 	for (; dev->msg_write_idx < num; dev->msg_write_idx++) {
@@ -287,10 +287,10 @@ i2c_dw_xfer_msg(struct i2c_adapter *adap)
 
 		while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
 			if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
-				writew(0x100, dev->base + DW_IC_DATA_CMD);
+				writel(0x100, dev->base + DW_IC_DATA_CMD);
 				rx_limit--;
 			} else
-				writew(*(dev->tx_buf++),
+				writel(*(dev->tx_buf++),
 						dev->base + DW_IC_DATA_CMD);
 			tx_limit--; buf_len--;
 		}
@@ -302,7 +302,7 @@ i2c_dw_xfer_msg(struct i2c_adapter *adap)
 		dev->status |= STATUS_WRITE_IN_PROGRESS;
 	} else
 		dev->status &= ~STATUS_WRITE_IN_PROGRESS;
-	writew(intr_mask, dev->base + DW_IC_INTR_MASK);
+	writel(intr_mask, dev->base + DW_IC_INTR_MASK);
 
 	dev->tx_buf_len = buf_len;
 }
@@ -313,11 +313,11 @@ i2c_dw_read(struct i2c_adapter *adap)
 	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
 	struct i2c_msg *msgs = dev->msgs;
 	int num = dev->msgs_num;
-	u16 addr = msgs[dev->msg_read_idx].addr;
-	int rx_valid = readw(dev->base + DW_IC_RXFLR);
+	u32 addr = msgs[dev->msg_read_idx].addr;
+	int rx_valid = readl(dev->base + DW_IC_RXFLR);
 
 	for (; dev->msg_read_idx < num; dev->msg_read_idx++) {
-		u16 len;
+		u32 len;
 		u8 *buf;
 
 		if (!(msgs[dev->msg_read_idx].flags & I2C_M_RD))
@@ -336,7 +336,7 @@ i2c_dw_read(struct i2c_adapter *adap)
 		}
 
 		for (; len > 0 && rx_valid > 0; len--, rx_valid--)
-			*buf++ = readb(dev->base + DW_IC_DATA_CMD);
+			*buf++ = readl(dev->base + DW_IC_DATA_CMD);
 
 		if (len > 0) {
 			dev->status |= STATUS_READ_IN_PROGRESS;
@@ -398,7 +398,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 		do {
 			i2c_dw_read(adap);
 		} while (dev->status & STATUS_READ_IN_PROGRESS);
-		writeb(0, dev->base + DW_IC_ENABLE);
+		writel(0, dev->base + DW_IC_ENABLE);
 		ret = num;
 		goto done;
 	}
@@ -428,7 +428,7 @@ static u32 i2c_dw_func(struct i2c_adapter *adap)
 static void dw_i2c_pump_msg(unsigned long data)
 {
 	struct dw_i2c_dev *dev = (struct dw_i2c_dev *) data;
-	u16 intr_mask;
+	u32 intr_mask;
 
 	i2c_dw_read(&dev->adapter);
 	i2c_dw_xfer_msg(&dev->adapter);
@@ -436,7 +436,7 @@ static void dw_i2c_pump_msg(unsigned long data)
 	intr_mask = DW_IC_INTR_STOP_DET | DW_IC_INTR_TX_ABRT;
 	if (dev->status & STATUS_WRITE_IN_PROGRESS)
 		intr_mask |= DW_IC_INTR_TX_EMPTY;
-	writew(intr_mask, dev->base + DW_IC_INTR_MASK);
+	writel(intr_mask, dev->base + DW_IC_INTR_MASK);
 }
 
 /*
@@ -446,19 +446,19 @@ static void dw_i2c_pump_msg(unsigned long data)
 static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 {
 	struct dw_i2c_dev *dev = dev_id;
-	u16 stat;
+	u32 stat;
 
-	stat = readw(dev->base + DW_IC_INTR_STAT);
+	stat = readl(dev->base + DW_IC_INTR_STAT);
 	dev_dbg(dev->dev, "%s: stat=0x%x\n", __func__, stat);
 	if (stat & DW_IC_INTR_TX_ABRT) {
-		dev->abort_source = readw(dev->base + DW_IC_TX_ABRT_SOURCE);
+		dev->abort_source = readl(dev->base + DW_IC_TX_ABRT_SOURCE);
 		dev->cmd_err |= DW_IC_ERR_TX_ABRT;
 		dev->status = STATUS_IDLE;
 	} else if (stat & DW_IC_INTR_TX_EMPTY)
 		tasklet_schedule(&dev->pump_msg);
 
-	readb(dev->base + DW_IC_CLR_INTR);	/* clear interrupts */
-	writew(0, dev->base + DW_IC_INTR_MASK);	/* disable interrupts */
+	readl(dev->base + DW_IC_CLR_INTR);	/* clear interrupts */
+	writel(0, dev->base + DW_IC_INTR_MASK);	/* disable interrupts */
 	if (stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET))
 		complete(&dev->cmd_complete);
 
@@ -531,7 +531,7 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
 	}
 	i2c_dw_init(dev);
 
-	writew(0, dev->base + DW_IC_INTR_MASK); /* disable IRQ */
+	writel(0, dev->base + DW_IC_INTR_MASK); /* disable IRQ */
 	r = request_irq(dev->irq, i2c_dw_isr, 0, pdev->name, dev);
 	if (r) {
 		dev_err(&pdev->dev, "failure requesting irq %i\n", dev->irq);
@@ -587,7 +587,7 @@ static int __devexit dw_i2c_remove(struct platform_device *pdev)
 	clk_put(dev->clk);
 	dev->clk = NULL;
 
-	writeb(0, dev->base + DW_IC_ENABLE);
+	writel(0, dev->base + DW_IC_ENABLE);
 	free_irq(dev->irq, dev);
 	kfree(dev);
 
-- 
1.6.5.2

[PATCH v2 02/22] i2c-designware: Don't use the IC_CLR_INTR register to clear interrupts

[Shinya Kuribayashi]

We're strongly discouraged from using the IC_CLR_INTR register because
it clears all software-clearable interrupts asserted at the moment.

  stat = readl(IC_INTR_STAT);
    :
    :  <=== Interrupts asserted during this period will be lost
    :
  readl(IC_CLR_INTR);

Instead, use the separately-prepared IC_CLR_* registers.

At the same time, this patch adds all remaining interrupt definitions
available in the DesignWare I2C hardware.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
---

 v2: Preserve IC_TX_ABRT_SOURCE before clearing TX_ABRT interrupt.

 drivers/i2c/busses/i2c-designware.c |   84 ++++++++++++++++++++++++++++++++--
 1 files changed, 79 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index a4f928e..eeb1915 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -49,7 +49,18 @@
 #define DW_IC_FS_SCL_LCNT	0x20
 #define DW_IC_INTR_STAT		0x2c
 #define DW_IC_INTR_MASK		0x30
+#define DW_IC_RAW_INTR_STAT	0x34
 #define DW_IC_CLR_INTR		0x40
+#define DW_IC_CLR_RX_UNDER	0x44
+#define DW_IC_CLR_RX_OVER	0x48
+#define DW_IC_CLR_TX_OVER	0x4c
+#define DW_IC_CLR_RD_REQ	0x50
+#define DW_IC_CLR_TX_ABRT	0x54
+#define DW_IC_CLR_RX_DONE	0x58
+#define DW_IC_CLR_ACTIVITY	0x5c
+#define DW_IC_CLR_STOP_DET	0x60
+#define DW_IC_CLR_START_DET	0x64
+#define DW_IC_CLR_GEN_CALL	0x68
 #define DW_IC_ENABLE		0x6c
 #define DW_IC_STATUS		0x70
 #define DW_IC_TXFLR		0x74
@@ -64,9 +75,18 @@
 #define DW_IC_CON_RESTART_EN		0x20
 #define DW_IC_CON_SLAVE_DISABLE		0x40
 
-#define DW_IC_INTR_TX_EMPTY	0x10
-#define DW_IC_INTR_TX_ABRT	0x40
+#define DW_IC_INTR_RX_UNDER	0x001
+#define DW_IC_INTR_RX_OVER	0x002
+#define DW_IC_INTR_RX_FULL	0x004
+#define DW_IC_INTR_TX_OVER	0x008
+#define DW_IC_INTR_TX_EMPTY	0x010
+#define DW_IC_INTR_RD_REQ	0x020
+#define DW_IC_INTR_TX_ABRT	0x040
+#define DW_IC_INTR_RX_DONE	0x080
+#define DW_IC_INTR_ACTIVITY	0x100
 #define DW_IC_INTR_STOP_DET	0x200
+#define DW_IC_INTR_START_DET	0x400
+#define DW_IC_INTR_GEN_CALL	0x800
 
 #define DW_IC_STATUS_ACTIVITY	0x1
 
@@ -439,6 +459,61 @@ static void dw_i2c_pump_msg(unsigned long data)
 	writel(intr_mask, dev->base + DW_IC_INTR_MASK);
 }
 
+static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
+{
+	u32 stat;
+
+	/*
+	 * The IC_INTR_STAT register just indicates "enabled" interrupts.
+	 * Ths unmasked raw version of interrupt status bits are available
+	 * in the IC_RAW_INTR_STAT register.
+	 *
+	 * That is,
+	 *   stat = readl(IC_INTR_STAT);
+	 * equals to,
+	 *   stat = readl(IC_RAW_INTR_STAT) & readl(IC_INTR_MASK);
+	 *
+	 * The raw version might be useful for debugging purposes.
+	 */
+	stat = readl(dev->base + DW_IC_INTR_STAT);
+
+	/*
+	 * Do not use the IC_CLR_INTR register to clear interrupts, or
+	 * you'll miss some interrupts, triggered during the period from
+	 * readl(IC_INTR_STAT) to readl(IC_CLR_INTR).
+	 *
+	 * Instead, use the separately-prepared IC_CLR_* registers.
+	 */
+	if (stat & DW_IC_INTR_RX_UNDER)
+		readl(dev->base + DW_IC_CLR_RX_UNDER);
+	if (stat & DW_IC_INTR_RX_OVER)
+		readl(dev->base + DW_IC_CLR_RX_OVER);
+	if (stat & DW_IC_INTR_TX_OVER)
+		readl(dev->base + DW_IC_CLR_TX_OVER);
+	if (stat & DW_IC_INTR_RD_REQ)
+		readl(dev->base + DW_IC_CLR_RD_REQ);
+	if (stat & DW_IC_INTR_TX_ABRT) {
+		/*
+		 * The IC_TX_ABRT_SOURCE register is cleared whenever
+		 * the IC_CLR_TX_ABRT is read.  Preserve it beforehand.
+		 */
+		dev->abort_source = readl(dev->base + DW_IC_TX_ABRT_SOURCE);
+		readl(dev->base + DW_IC_CLR_TX_ABRT);
+	}
+	if (stat & DW_IC_INTR_RX_DONE)
+		readl(dev->base + DW_IC_CLR_RX_DONE);
+	if (stat & DW_IC_INTR_ACTIVITY)
+		readl(dev->base + DW_IC_CLR_ACTIVITY);
+	if (stat & DW_IC_INTR_STOP_DET)
+		readl(dev->base + DW_IC_CLR_STOP_DET);
+	if (stat & DW_IC_INTR_START_DET)
+		readl(dev->base + DW_IC_CLR_START_DET);
+	if (stat & DW_IC_INTR_GEN_CALL)
+		readl(dev->base + DW_IC_CLR_GEN_CALL);
+
+	return stat;
+}
+
 /*
  * Interrupt service routine. This gets called whenever an I2C interrupt
  * occurs.
@@ -448,16 +523,15 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 	struct dw_i2c_dev *dev = dev_id;
 	u32 stat;
 
-	stat = readl(dev->base + DW_IC_INTR_STAT);
+	stat = i2c_dw_read_clear_intrbits(dev);
 	dev_dbg(dev->dev, "%s: stat=0x%x\n", __func__, stat);
+
 	if (stat & DW_IC_INTR_TX_ABRT) {
-		dev->abort_source = readl(dev->base + DW_IC_TX_ABRT_SOURCE);
 		dev->cmd_err |= DW_IC_ERR_TX_ABRT;
 		dev->status = STATUS_IDLE;
 	} else if (stat & DW_IC_INTR_TX_EMPTY)
 		tasklet_schedule(&dev->pump_msg);
 
-	readl(dev->base + DW_IC_CLR_INTR);	/* clear interrupts */
 	writel(0, dev->base + DW_IC_INTR_MASK);	/* disable interrupts */
 	if (stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET))
 		complete(&dev->cmd_complete);
-- 
1.6.5.2

[PATCH v2 03/22] i2c-designware: Use platform_get_irq helper

[Shinya Kuribayashi]

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Acked-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index eeb1915..139f555 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -548,8 +548,8 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev;
 	struct i2c_adapter *adap;
-	struct resource *mem, *irq, *ioarea;
-	int r;
+	struct resource *mem, *ioarea;
+	int irq, r;
 
 	/* NOTE: driver uses the static register mapping */
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -558,10 +558,10 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!irq) {
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
 		dev_err(&pdev->dev, "no irq resource?\n");
-		return -EINVAL;
+		return irq; /* -ENXIO */
 	}
 
 	ioarea = request_mem_region(mem->start, resource_size(mem),
@@ -581,7 +581,7 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
 	tasklet_init(&dev->pump_msg, dw_i2c_pump_msg, (unsigned long) dev);
 	mutex_init(&dev->lock);
 	dev->dev = get_device(&pdev->dev);
-	dev->irq = irq->start;
+	dev->irq = irq;
 	platform_set_drvdata(pdev, dev);
 
 	dev->clk = clk_get(&pdev->dev, NULL);
-- 
1.6.5.2

[PATCH v2 04/22] i2c-designware: i2c_dw_read: Use "struct dw_i2c_dev" pointer

[Shinya Kuribayashi]

We don't have to use "struct i2c_adapter" pointer here.
Let's use a local "struct dw_i2c_dev" pointer, instead.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Acked-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 139f555..bb7b257 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -328,9 +328,8 @@ i2c_dw_xfer_msg(struct i2c_adapter *adap)
 }
 
 static void
-i2c_dw_read(struct i2c_adapter *adap)
+i2c_dw_read(struct dw_i2c_dev *dev)
 {
-	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
 	struct i2c_msg *msgs = dev->msgs;
 	int num = dev->msgs_num;
 	u32 addr = msgs[dev->msg_read_idx].addr;
@@ -416,7 +415,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	if (likely(!dev->cmd_err)) {
 		/* read rx fifo, and disable the adapter */
 		do {
-			i2c_dw_read(adap);
+			i2c_dw_read(dev);
 		} while (dev->status & STATUS_READ_IN_PROGRESS);
 		writel(0, dev->base + DW_IC_ENABLE);
 		ret = num;
@@ -450,7 +449,7 @@ static void dw_i2c_pump_msg(unsigned long data)
 	struct dw_i2c_dev *dev = (struct dw_i2c_dev *) data;
 	u32 intr_mask;
 
-	i2c_dw_read(&dev->adapter);
+	i2c_dw_read(dev);
 	i2c_dw_xfer_msg(&dev->adapter);
 
 	intr_mask = DW_IC_INTR_STOP_DET | DW_IC_INTR_TX_ABRT;
-- 
1.6.5.2

[PATCH v2 05/22] i2c-designware: i2c_dw_xfer_msg: Use "struct dw_i2c_dev" pointer

[Shinya Kuribayashi]

We don't have to use "struct i2c_adapter" pointer here.
Let's use a local "struct dw_i2c_dev" pointer, instead.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Acked-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index bb7b257..443f398 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -254,9 +254,8 @@ static int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
  * that is longer than the size of the TX FIFO.
  */
 static void
-i2c_dw_xfer_msg(struct i2c_adapter *adap)
+i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 {
-	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
 	struct i2c_msg *msgs = dev->msgs;
 	int num = dev->msgs_num;
 	u32 ic_con, intr_mask;
@@ -394,7 +393,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 		goto done;
 
 	/* start the transfers */
-	i2c_dw_xfer_msg(adap);
+	i2c_dw_xfer_msg(dev);
 
 	/* wait for tx to complete */
 	ret = wait_for_completion_interruptible_timeout(&dev->cmd_complete, HZ);
@@ -450,7 +449,7 @@ static void dw_i2c_pump_msg(unsigned long data)
 	u32 intr_mask;
 
 	i2c_dw_read(dev);
-	i2c_dw_xfer_msg(&dev->adapter);
+	i2c_dw_xfer_msg(dev);
 
 	intr_mask = DW_IC_INTR_STOP_DET | DW_IC_INTR_TX_ABRT;
 	if (dev->status & STATUS_WRITE_IN_PROGRESS)
-- 
1.6.5.2

[PATCH v2 06/22] i2c-designware: Remove an useless local variable "num"

[Shinya Kuribayashi]

We couldn't know the original intent for this variable, but at this
point it's useless.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Acked-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 443f398..fd1616b 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -257,7 +257,6 @@ static void
 i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 {
 	struct i2c_msg *msgs = dev->msgs;
-	int num = dev->msgs_num;
 	u32 ic_con, intr_mask;
 	int tx_limit = dev->tx_fifo_depth - readl(dev->base + DW_IC_TXFLR);
 	int rx_limit = dev->rx_fifo_depth - readl(dev->base + DW_IC_RXFLR);
@@ -283,7 +282,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 		writel(1, dev->base + DW_IC_ENABLE);
 	}
 
-	for (; dev->msg_write_idx < num; dev->msg_write_idx++) {
+	for (; dev->msg_write_idx < dev->msgs_num; dev->msg_write_idx++) {
 		/* if target address has changed, we need to
 		 * reprogram the target address in the i2c
 		 * adapter when we are done with this transfer
@@ -330,11 +329,10 @@ static void
 i2c_dw_read(struct dw_i2c_dev *dev)
 {
 	struct i2c_msg *msgs = dev->msgs;
-	int num = dev->msgs_num;
 	u32 addr = msgs[dev->msg_read_idx].addr;
 	int rx_valid = readl(dev->base + DW_IC_RXFLR);
 
-	for (; dev->msg_read_idx < num; dev->msg_read_idx++) {
+	for (; dev->msg_read_idx < dev->msgs_num; dev->msg_read_idx++) {
 		u32 len;
 		u8 *buf;
 
-- 
1.6.5.2

[PATCH v2 07/22] i2c-designware: Improved _HCNT/_LCNT calculation

[Shinya Kuribayashi]

* Calculate with accurate conditional expressions from DW manuals.
* Round ic_clk by adding 0.5 as it's important at high ic_clk rate.
* Take into account "tHD;STA" issue for _HCNT calculation.
* Take into account "tf" for _LCNT calculation.
* Add "cond" and "offset" fot further correction requirements.

For _HCNT calculation, there's one issue needs to be carefully
considered; DesignWare I2C core doesn't seem to have solid strategy
to meet the tHD;STA timing spec.  If you configure _HCNT based on the
tHIGH timing spec, it easily results in violation of the tHD;STA spec.

After many trials, we came to the conclusion that the tHD;STA period
is proportional to (_HCNT + 3).  For the safety's sake, this should be
selected by default.

As for _LCNT calculation, DW I2C core has one characteristic behavior;
he starts counting the SCL CNTs for the LOW period of the SCL clock
(tLOW) as soon as it pulls the SCL line.  At that time, he doesn't take
into account the fall time of SCL signal (tf), IOW, he starts counting
CNTs without confirming the SCL input voltage has dropped to below VIL.

This characteristics becomes a problem on some platforms where tf is
considerably long, and results in violation of the tLOW timing spec.

To make the driver configurable as much as possible for various cases,
we'd have separated arguments "tf" and "offset", and for safety default
values should be 0.3 us and 0, respectively.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Acked-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |   93 +++++++++++++++++++++++++++++++---
 1 files changed, 84 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index fd1616b..cdfd94e 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -196,6 +196,61 @@ struct dw_i2c_dev {
 	unsigned int		rx_fifo_depth;
 };
 
+static u32
+i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
+{
+	/*
+	 * DesignWare I2C core doesn't seem to have solid strategy to meet
+	 * the tHD;STA timing spec.  Configuring _HCNT based on tHIGH spec
+	 * will result in violation of the tHD;STA spec.
+	 */
+	if (cond)
+		/*
+		 * Conditional expression:
+		 *
+		 *   IC_[FS]S_SCL_HCNT + (1+4+3) >= IC_CLK * tHIGH
+		 *
+		 * This is based on the DW manuals, and represents an ideal
+		 * configuration.  The resulting I2C bus speed will be
+		 * faster than any of the others.
+		 *
+		 * If your hardware is free from tHD;STA issue, try this one.
+		 */
+		return (ic_clk * tSYMBOL + 5000) / 10000 - 8 + offset;
+	else
+		/*
+		 * Conditional expression:
+		 *
+		 *   IC_[FS]S_SCL_HCNT + 3 >= IC_CLK * (tHD;STA + tf)
+		 *
+		 * This is just experimental rule; the tHD;STA period turned
+		 * out to be proportinal to (_HCNT + 3).  With this setting,
+		 * we could meet both tHIGH and tHD;STA timing specs.
+		 *
+		 * If unsure, you'd better to take this alternative.
+		 *
+		 * The reason why we need to take into account "tf" here,
+		 * is the same as described in i2c_dw_scl_lcnt().
+		 */
+		return (ic_clk * (tSYMBOL + tf) + 5000) / 10000 - 3 + offset;
+}
+
+static u32 i2c_dw_scl_lcnt(u32 ic_clk, u32 tLOW, u32 tf, int offset)
+{
+	/*
+	 * Conditional expression:
+	 *
+	 *   IC_[FS]S_SCL_LCNT + 1 >= IC_CLK * (tLOW + tf)
+	 *
+	 * DW I2C core starts counting the SCL CNTs for the LOW period
+	 * of the SCL clock (tLOW) as soon as it pulls the SCL line.
+	 * In order to meet the tLOW timing spec, we need to take into
+	 * account the fall time of SCL signal (tf).  Default tf value
+	 * should be 0.3 us, for safety.
+	 */
+	return ((ic_clk * (tLOW + tf) + 5000) / 10000) - 1 + offset;
+}
+
 /**
  * i2c_dw_init() - initialize the designware i2c master hardware
  * @dev: device private data
@@ -207,20 +262,40 @@ struct dw_i2c_dev {
 static void i2c_dw_init(struct dw_i2c_dev *dev)
 {
 	u32 input_clock_khz = clk_get_rate(dev->clk) / 1000;
-	u32 ic_con;
+	u32 ic_con, hcnt, lcnt;
 
 	/* Disable the adapter */
 	writel(0, dev->base + DW_IC_ENABLE);
 
 	/* set standard and fast speed deviders for high/low periods */
-	writel((input_clock_khz * 40 / 10000)+1, /* std speed high, 4us */
-			dev->base + DW_IC_SS_SCL_HCNT);
-	writel((input_clock_khz * 47 / 10000)+1, /* std speed low, 4.7us */
-			dev->base + DW_IC_SS_SCL_LCNT);
-	writel((input_clock_khz *  6 / 10000)+1, /* fast speed high, 0.6us */
-			dev->base + DW_IC_FS_SCL_HCNT);
-	writel((input_clock_khz * 13 / 10000)+1, /* fast speed low, 1.3us */
-			dev->base + DW_IC_FS_SCL_LCNT);
+
+	/* Standard-mode */
+	hcnt = i2c_dw_scl_hcnt(input_clock_khz,
+				40,	/* tHD;STA = tHIGH = 4.0 us */
+				3,	/* tf = 0.3 us */
+				0,	/* 0: DW default, 1: Ideal */
+				0);	/* No offset */
+	lcnt = i2c_dw_scl_lcnt(input_clock_khz,
+				47,	/* tLOW = 4.7 us */
+				3,	/* tf = 0.3 us */
+				0);	/* No offset */
+	writel(hcnt, dev->base + DW_IC_SS_SCL_HCNT);
+	writel(lcnt, dev->base + DW_IC_SS_SCL_LCNT);
+	dev_dbg(dev->dev, "Standard-mode HCNT:LCNT = %d:%d\n", hcnt, lcnt);
+
+	/* Fast-mode */
+	hcnt = i2c_dw_scl_hcnt(input_clock_khz,
+				6,	/* tHD;STA = tHIGH = 0.6 us */
+				3,	/* tf = 0.3 us */
+				0,	/* 0: DW default, 1: Ideal */
+				0);	/* No offset */
+	lcnt = i2c_dw_scl_lcnt(input_clock_khz,
+				13,	/* tLOW = 1.3 us */
+				3,	/* tf = 0.3 us */
+				0);	/* No offset */
+	writel(hcnt, dev->base + DW_IC_FS_SCL_HCNT);
+	writel(lcnt, dev->base + DW_IC_FS_SCL_LCNT);
+	dev_dbg(dev->dev, "Fast-mode HCNT:LCNT = %d:%d\n", hcnt, lcnt);
 
 	/* configure the i2c master */
 	ic_con = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
-- 
1.6.5.2

[PATCH v2 08/22] i2c-designware: i2c_dw_xfer_msg: Fix i2c_msg search bug

[Shinya Kuribayashi]

In case a work-in-progress i2c_msg has more bytes to be written, we
need to set STATUS_WRITE_IN_PROGRESS and exit from the msg_write_idx-
searching loop.  Otherwise, we will overtake the current msg_write_idx
without waiting for its transmission to be processed.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
---

v2: The intr_mask handling, which stayed outside for-loop in the
    previous submission, is now folded into the for-loop.

 drivers/i2c/busses/i2c-designware.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index cdfd94e..bfb42f0 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -338,6 +338,8 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 	u32 addr = msgs[dev->msg_write_idx].addr;
 	u32 buf_len = dev->tx_buf_len;
 
+	intr_mask = DW_IC_INTR_STOP_DET | DW_IC_INTR_TX_ABRT;
+
 	if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
 		/* Disable the adapter */
 		writel(0, dev->base + DW_IC_ENABLE);
@@ -387,17 +389,19 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 						dev->base + DW_IC_DATA_CMD);
 			tx_limit--; buf_len--;
 		}
+
+		dev->tx_buf_len = buf_len;
+
+		if (buf_len > 0) {
+			/* more bytes to be written */
+			intr_mask |= DW_IC_INTR_TX_EMPTY;
+			dev->status |= STATUS_WRITE_IN_PROGRESS;
+			break;
+		} else
+			dev->status &= ~STATUS_WRITE_IN_PROGRESS;
 	}
 
-	intr_mask = DW_IC_INTR_STOP_DET | DW_IC_INTR_TX_ABRT;
-	if (buf_len > 0) { /* more bytes to be written */
-		intr_mask |= DW_IC_INTR_TX_EMPTY;
-		dev->status |= STATUS_WRITE_IN_PROGRESS;
-	} else
-		dev->status &= ~STATUS_WRITE_IN_PROGRESS;
 	writel(intr_mask, dev->base + DW_IC_INTR_MASK);
-
-	dev->tx_buf_len = buf_len;
 }
 
 static void
-- 
1.6.5.2

[PATCH v2 09/22] i2c-designware: Process i2c_msg messages in the interrupt handler

[Shinya Kuribayashi]

Symptom:
--------
When we're going to send/receive the longer size of data than the Tx
FIFO length, the I2C transaction will be divided into several separated
transactions, limited by the Tx FIFO length.

Details:
--------
As a hardware feature, DW I2C core generates a STOP condition whenever
the Tx FIFO becomes empty (strictly speaking, whenever the last byte in
the Tx FIFO is sent out), even if we have more bytes to be written.
Then, once a new transmit data is written to the Tx FIFO, DW I2C core
will initiate a new transaction, which leads to another START condition.

This explains how the transaction in question goes, and implies that
current tasklet-based dw_i2c_pump_msg() strategy couldn't meet the
timing constraint required for avoiding Tx FIFO underrun.

To avoid this scenario, we must keep providing new transmit data within
a given time period.  In case of Fast-mode + 32-byte Tx FIFO, for
instance, it takes about 22.5[us] to process single byte, and 720[us] in
total.

This patch removes the existing tasklet-based "pump" system, and move
its jobs into the interrupt handler.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Acked-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |   39 ++++++++++++----------------------
 1 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index bfb42f0..5fce1a0 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -149,7 +149,6 @@ static char *abort_sources[] = {
  * @dev: driver model device node
  * @base: IO registers pointer
  * @cmd_complete: tx completion indicator
- * @pump_msg: continue in progress transfers
  * @lock: protect this struct and IO registers
  * @clk: input reference clock
  * @cmd_err: run time hadware error code
@@ -175,7 +174,6 @@ struct dw_i2c_dev {
 	struct device		*dev;
 	void __iomem		*base;
 	struct completion	cmd_complete;
-	struct tasklet_struct	pump_msg;
 	struct mutex		lock;
 	struct clk		*clk;
 	int			cmd_err;
@@ -325,7 +323,7 @@ static int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
 /*
  * Initiate low level master read/write transaction.
  * This function is called from i2c_dw_xfer when starting a transfer.
- * This function is also called from dw_i2c_pump_msg to continue a transfer
+ * This function is also called from i2c_dw_isr to continue a transfer
  * that is longer than the size of the TX FIFO.
  */
 static void
@@ -489,10 +487,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 
 	/* no error */
 	if (likely(!dev->cmd_err)) {
-		/* read rx fifo, and disable the adapter */
-		do {
-			i2c_dw_read(dev);
-		} while (dev->status & STATUS_READ_IN_PROGRESS);
+		/* Disable the adapter */
 		writel(0, dev->base + DW_IC_ENABLE);
 		ret = num;
 		goto done;
@@ -520,20 +515,6 @@ static u32 i2c_dw_func(struct i2c_adapter *adap)
 	return I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR;
 }
 
-static void dw_i2c_pump_msg(unsigned long data)
-{
-	struct dw_i2c_dev *dev = (struct dw_i2c_dev *) data;
-	u32 intr_mask;
-
-	i2c_dw_read(dev);
-	i2c_dw_xfer_msg(dev);
-
-	intr_mask = DW_IC_INTR_STOP_DET | DW_IC_INTR_TX_ABRT;
-	if (dev->status & STATUS_WRITE_IN_PROGRESS)
-		intr_mask |= DW_IC_INTR_TX_EMPTY;
-	writel(intr_mask, dev->base + DW_IC_INTR_MASK);
-}
-
 static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
 {
 	u32 stat;
@@ -604,10 +585,19 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 	if (stat & DW_IC_INTR_TX_ABRT) {
 		dev->cmd_err |= DW_IC_ERR_TX_ABRT;
 		dev->status = STATUS_IDLE;
-	} else if (stat & DW_IC_INTR_TX_EMPTY)
-		tasklet_schedule(&dev->pump_msg);
+	}
+
+	if (stat & DW_IC_INTR_TX_EMPTY) {
+		i2c_dw_read(dev);
+		i2c_dw_xfer_msg(dev);
+	}
+
+	/*
+	 * No need to modify or disable the interrupt mask here.
+	 * i2c_dw_xfer_msg() will take care of it according to
+	 * the current transmit status.
+	 */
 
-	writel(0, dev->base + DW_IC_INTR_MASK);	/* disable interrupts */
 	if (stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET))
 		complete(&dev->cmd_complete);
 
@@ -653,7 +643,6 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
 	}
 
 	init_completion(&dev->cmd_complete);
-	tasklet_init(&dev->pump_msg, dw_i2c_pump_msg, (unsigned long) dev);
 	mutex_init(&dev->lock);
 	dev->dev = get_device(&pdev->dev);
 	dev->irq = irq;
-- 
1.6.5.2

[PATCH v2 10/22] i2c-designware: Set Tx/Rx FIFO threshold levels

[Shinya Kuribayashi]

As a hardware feature, DW I2C core generates a STOP condition whenever
the Tx FIFO becomes empty (strictly speaking, whenever the last byte in
the Tx FIFO is sent out), even if we have more bytes to be written.

In other words, we must never make "Tx FIFO underrun" happen during
a transaction, except for the last byte.  For the safety's sake, we'd
make TX_EMPTY interrupt get triggered every time one byte is processed.

The Rx FIFO threshold needs to be set as well.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Acked-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 5fce1a0..0eea0dd 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -50,6 +50,8 @@
 #define DW_IC_INTR_STAT		0x2c
 #define DW_IC_INTR_MASK		0x30
 #define DW_IC_RAW_INTR_STAT	0x34
+#define DW_IC_RX_TL		0x38
+#define DW_IC_TX_TL		0x3c
 #define DW_IC_CLR_INTR		0x40
 #define DW_IC_CLR_RX_UNDER	0x44
 #define DW_IC_CLR_RX_OVER	0x48
@@ -295,6 +297,10 @@ static void i2c_dw_init(struct dw_i2c_dev *dev)
 	writel(lcnt, dev->base + DW_IC_FS_SCL_LCNT);
 	dev_dbg(dev->dev, "Fast-mode HCNT:LCNT = %d:%d\n", hcnt, lcnt);
 
+	/* Configure Tx/Rx FIFO threshold levels */
+	writel(dev->tx_fifo_depth - 1, dev->base + DW_IC_TX_TL);
+	writel(0, dev->base + DW_IC_RX_TL);
+
 	/* configure the i2c master */
 	ic_con = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
 		DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST;
-- 
1.6.5.2

[PATCH v2 11/22] i2c-designware: Enable RX_FULL interrupt

[Shinya Kuribayashi]

Enable RX_FULL interrupt mask by default, and hook it in the interrupt
handler.  If requested amount of rx data (defined by IC_RX_TL) is not
available, we don't have to process i2c_dw_read().

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Acked-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 0eea0dd..940bbf3 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -342,7 +342,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 	u32 addr = msgs[dev->msg_write_idx].addr;
 	u32 buf_len = dev->tx_buf_len;
 
-	intr_mask = DW_IC_INTR_STOP_DET | DW_IC_INTR_TX_ABRT;
+	intr_mask = DW_IC_INTR_STOP_DET | DW_IC_INTR_TX_ABRT | DW_IC_INTR_RX_FULL;
 
 	if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
 		/* Disable the adapter */
@@ -593,10 +593,11 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 		dev->status = STATUS_IDLE;
 	}
 
-	if (stat & DW_IC_INTR_TX_EMPTY) {
+	if (stat & DW_IC_INTR_RX_FULL)
 		i2c_dw_read(dev);
+
+	if (stat & DW_IC_INTR_TX_EMPTY)
 		i2c_dw_xfer_msg(dev);
-	}
 
 	/*
 	 * No need to modify or disable the interrupt mask here.
-- 
1.6.5.2

[PATCH v2 12/22] i2c-designware: Divide i2c_dw_xfer_msg into two functions

[Shinya Kuribayashi]

We have some steps at the top of i2c_dw_xfer_msg() to set up a slave
address and enable DW I2C core.  And it's executed only when we don't
have STATUS_WRITE_IN_PROGRESS.

But we need to make sure that STATUS_WRITE_IN_PROGRESS only indicates
that we have a pending i2c_msg to process.  In other words, even if
STATUS_WRITE_IN_PROGRESS is not set, that doesn't mean we're at initial
state in the I2C transaction.

Since i2c_dw_xfer_msg() will be invoked again and again during a
transaction, those init steps have a possibility to be re-processed
needlessly.  For example, this issue easily takes place when processing
a combined transaction with a certain condition (the number of tx bytes
in the first i2c_msg, equals to the Tx FIFO depth).

Consequently we should not use STATUS_WRITE_IN_PROGRESS to determine
where we're at in an I2C transaction.  It would be better to separate
those initialization steps from i2c_dw_xfer_msg().

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Acked-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |   45 +++++++++++++++++++---------------
 1 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 940bbf3..da5612b 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -326,6 +326,29 @@ static int i2c_dw_wait_bus_not_busy(struct dw_i2c_dev *dev)
 	return 0;
 }
 
+static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
+{
+	struct i2c_msg *msgs = dev->msgs;
+	u32 ic_con;
+
+	/* Disable the adapter */
+	writel(0, dev->base + DW_IC_ENABLE);
+
+	/* set the slave (target) address */
+	writel(msgs[dev->msg_write_idx].addr, dev->base + DW_IC_TAR);
+
+	/* if the slave address is ten bit address, enable 10BITADDR */
+	ic_con = readl(dev->base + DW_IC_CON);
+	if (msgs[dev->msg_write_idx].flags & I2C_M_TEN)
+		ic_con |= DW_IC_CON_10BITADDR_MASTER;
+	else
+		ic_con &= ~DW_IC_CON_10BITADDR_MASTER;
+	writel(ic_con, dev->base + DW_IC_CON);
+
+	/* Enable the adapter */
+	writel(1, dev->base + DW_IC_ENABLE);
+}
+
 /*
  * Initiate low level master read/write transaction.
  * This function is called from i2c_dw_xfer when starting a transfer.
@@ -336,7 +359,7 @@ static void
 i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 {
 	struct i2c_msg *msgs = dev->msgs;
-	u32 ic_con, intr_mask;
+	u32 intr_mask;
 	int tx_limit = dev->tx_fifo_depth - readl(dev->base + DW_IC_TXFLR);
 	int rx_limit = dev->rx_fifo_depth - readl(dev->base + DW_IC_RXFLR);
 	u32 addr = msgs[dev->msg_write_idx].addr;
@@ -344,25 +367,6 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 
 	intr_mask = DW_IC_INTR_STOP_DET | DW_IC_INTR_TX_ABRT | DW_IC_INTR_RX_FULL;
 
-	if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
-		/* Disable the adapter */
-		writel(0, dev->base + DW_IC_ENABLE);
-
-		/* set the slave (target) address */
-		writel(msgs[dev->msg_write_idx].addr, dev->base + DW_IC_TAR);
-
-		/* if the slave address is ten bit address, enable 10BITADDR */
-		ic_con = readl(dev->base + DW_IC_CON);
-		if (msgs[dev->msg_write_idx].flags & I2C_M_TEN)
-			ic_con |= DW_IC_CON_10BITADDR_MASTER;
-		else
-			ic_con &= ~DW_IC_CON_10BITADDR_MASTER;
-		writel(ic_con, dev->base + DW_IC_CON);
-
-		/* Enable the adapter */
-		writel(1, dev->base + DW_IC_ENABLE);
-	}
-
 	for (; dev->msg_write_idx < dev->msgs_num; dev->msg_write_idx++) {
 		/* if target address has changed, we need to
 		 * reprogram the target address in the i2c
@@ -474,6 +478,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 		goto done;
 
 	/* start the transfers */
+	i2c_dw_xfer_init(dev);
 	i2c_dw_xfer_msg(dev);
 
 	/* wait for tx to complete */
-- 
1.6.5.2

[PATCH v2 13/22] i2c-designware: i2c_dw_xfer_msg: Introduce a local "buf" pointer

[Shinya Kuribayashi]

While we have a local variable "buf_len" for dev->tx_buf_len, we don't
have such local variable for dev->tx_buf pointer.  While "buf_len" is
restored at first then updated when we start processing a new i2c_msg
(determined by STATUS_WRITE_IN_PROGRESS flag), ->tx_buf is different.

Such inconsistency makes the code slightly hard to follow.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Acked-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index da5612b..e8e2212 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -364,6 +364,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 	int rx_limit = dev->rx_fifo_depth - readl(dev->base + DW_IC_RXFLR);
 	u32 addr = msgs[dev->msg_write_idx].addr;
 	u32 buf_len = dev->tx_buf_len;
+	u8 *buf = dev->tx_buf;;
 
 	intr_mask = DW_IC_INTR_STOP_DET | DW_IC_INTR_TX_ABRT | DW_IC_INTR_RX_FULL;
 
@@ -384,7 +385,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 
 		if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
 			/* new i2c_msg */
-			dev->tx_buf = msgs[dev->msg_write_idx].buf;
+			buf = msgs[dev->msg_write_idx].buf;
 			buf_len = msgs[dev->msg_write_idx].len;
 		}
 
@@ -393,11 +394,11 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 				writel(0x100, dev->base + DW_IC_DATA_CMD);
 				rx_limit--;
 			} else
-				writel(*(dev->tx_buf++),
-						dev->base + DW_IC_DATA_CMD);
+				writel(*buf++, dev->base + DW_IC_DATA_CMD);
 			tx_limit--; buf_len--;
 		}
 
+		dev->tx_buf = buf;
 		dev->tx_buf_len = buf_len;
 
 		if (buf_len > 0) {
-- 
1.6.5.2

[PATCH v2 14/22] i2c-designware: Initialize byte count variables just prior to being used

[Shinya Kuribayashi]

As the driver and hardware always process the given data in parallel,
then it would be better to initialize tx_limit, rx_limit and rx_valid
variables just prior to being used.

This will help us to send / receive as much data as possible.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
Acked-by: Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index e8e2212..c3702a8 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -360,8 +360,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 {
 	struct i2c_msg *msgs = dev->msgs;
 	u32 intr_mask;
-	int tx_limit = dev->tx_fifo_depth - readl(dev->base + DW_IC_TXFLR);
-	int rx_limit = dev->rx_fifo_depth - readl(dev->base + DW_IC_RXFLR);
+	int tx_limit, rx_limit;
 	u32 addr = msgs[dev->msg_write_idx].addr;
 	u32 buf_len = dev->tx_buf_len;
 	u8 *buf = dev->tx_buf;;
@@ -389,6 +388,9 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 			buf_len = msgs[dev->msg_write_idx].len;
 		}
 
+		tx_limit = dev->tx_fifo_depth - readl(dev->base + DW_IC_TXFLR);
+		rx_limit = dev->rx_fifo_depth - readl(dev->base + DW_IC_RXFLR);
+
 		while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
 			if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
 				writel(0x100, dev->base + DW_IC_DATA_CMD);
@@ -418,7 +420,7 @@ i2c_dw_read(struct dw_i2c_dev *dev)
 {
 	struct i2c_msg *msgs = dev->msgs;
 	u32 addr = msgs[dev->msg_read_idx].addr;
-	int rx_valid = readl(dev->base + DW_IC_RXFLR);
+	int rx_valid;
 
 	for (; dev->msg_read_idx < dev->msgs_num; dev->msg_read_idx++) {
 		u32 len;
@@ -439,6 +441,8 @@ i2c_dw_read(struct dw_i2c_dev *dev)
 			buf = dev->rx_buf;
 		}
 
+		rx_valid = readl(dev->base + DW_IC_RXFLR);
+
 		for (; len > 0 && rx_valid > 0; len--, rx_valid--)
 			*buf++ = readl(dev->base + DW_IC_DATA_CMD);
 
-- 
1.6.5.2

[PATCH v2 15/22] i2c-designware: i2c_dw_func: Set I2C_FUNC_SMBUS_foo bits

[Shinya Kuribayashi]

Set proper I2C_FUNC_SMBUS_* bits so that the driver could be used with
some utilities requiring SMBus functionalities, such as i2c-tools.

Note that DW I2C core doesn't support I2C_FUNC_SMBUS_QUICK, as it's not
capable of zero-length data transactions.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index c3702a8..5ba7e55 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -528,7 +528,12 @@ done:
 
 static u32 i2c_dw_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR;
+	return	I2C_FUNC_I2C |
+		I2C_FUNC_10BIT_ADDR |
+		I2C_FUNC_SMBUS_BYTE |
+		I2C_FUNC_SMBUS_BYTE_DATA |
+		I2C_FUNC_SMBUS_WORD_DATA |
+		I2C_FUNC_SMBUS_I2C_BLOCK;
 }
 
 static u32 i2c_dw_read_clear_intrbits(struct dw_i2c_dev *dev)
-- 
1.6.5.2

[PATCH v2 16/22] i2c-designware: i2c_dw_read: Remove redundant target address checker

[Shinya Kuribayashi]

I2c_dw_xfer_msg() also has the same target address inconsistency check,
and furthermore it checks across all i2c_msg messages, while
i2c_dw_read() walks through i2c_msg messages only with_ I2C_M_RD flag.
That is, target address check in i2c_dw_read() is redundant and useless.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 5ba7e55..5a3bd74 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -419,7 +419,6 @@ static void
 i2c_dw_read(struct dw_i2c_dev *dev)
 {
 	struct i2c_msg *msgs = dev->msgs;
-	u32 addr = msgs[dev->msg_read_idx].addr;
 	int rx_valid;
 
 	for (; dev->msg_read_idx < dev->msgs_num; dev->msg_read_idx++) {
@@ -429,10 +428,6 @@ i2c_dw_read(struct dw_i2c_dev *dev)
 		if (!(msgs[dev->msg_read_idx].flags & I2C_M_RD))
 			continue;
 
-		/* different i2c client, reprogram the i2c adapter */
-		if (msgs[dev->msg_read_idx].addr != addr)
-			return;
-
 		if (!(dev->status & STATUS_READ_IN_PROGRESS)) {
 			len = msgs[dev->msg_read_idx].len;
 			buf = msgs[dev->msg_read_idx].buf;
-- 
1.6.5.2

[PATCH v2 17/22] i2c-designware: Process all i2c_msg messages in the interrupt handler

[Shinya Kuribayashi]

Currently we process the first i2c_dw_xfer_msg() in i2c_dw_xfer(),
but in this case there is a possibility to be interrupted by certain
interrupts.  As described before in this patchset, we need to keep
providing new transmit data within a given time period, otherwise Tx
FIFO underrun takes place and STOP condition will be generated on the
bus, even if we have more bytes to be written.

In order to exclude all such possibilities, change TX_EMPTY interrupt
usage as below:

* DW_IC_INTR_DEFAULT_MASK: Define a default interrupt mask set, and
  put TX_EMPTY there.

* i2c_dw_xfer_init: Enable DW_IC_INTR_DEFAULT_MASK prior to initiating
  a new I2C transaction.  The first TX_EMPTY will be triggered shortly.
  With the help of it, we can make the first call to i2c_dw_xfer_msg()
  in the interrupt handler.

* i2c_dw_xfer_msg: Fixup intr_mask operation accordingly.  Make sure
  that TX_EMPTY operations need to be reversed.

* request_irq: Set IRQF_DISABLED so that we could load transmit data
  into Tx FIFO without being distracted by other interrupts.

* Remove i2c_dw_xfer_msg() in i2c_dw_xfer().

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |   26 +++++++++++++++++---------
 1 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 5a3bd74..f184d82 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -90,6 +90,11 @@
 #define DW_IC_INTR_START_DET	0x400
 #define DW_IC_INTR_GEN_CALL	0x800
 
+#define DW_IC_INTR_DEFAULT_MASK		(DW_IC_INTR_RX_FULL | \
+					 DW_IC_INTR_TX_EMPTY | \
+					 DW_IC_INTR_TX_ABRT | \
+					 DW_IC_INTR_STOP_DET)
+
 #define DW_IC_STATUS_ACTIVITY	0x1
 
 #define DW_IC_ERR_TX_ABRT	0x1
@@ -347,13 +352,16 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 
 	/* Enable the adapter */
 	writel(1, dev->base + DW_IC_ENABLE);
+
+	/* Enable interrupts */
+	writel(DW_IC_INTR_DEFAULT_MASK, dev->base + DW_IC_INTR_MASK);
 }
 
 /*
- * Initiate low level master read/write transaction.
- * This function is called from i2c_dw_xfer when starting a transfer.
- * This function is also called from i2c_dw_isr to continue a transfer
- * that is longer than the size of the TX FIFO.
+ * Initiate (and continue) low level master read/write transaction.
+ * This function is only called from i2c_dw_isr, and pumping i2c_msg
+ * messages into the tx buffer.  Even if the size of i2c_msg data is
+ * longer than the size of the tx buffer, it handles everything.
  */
 static void
 i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
@@ -365,7 +373,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 	u32 buf_len = dev->tx_buf_len;
 	u8 *buf = dev->tx_buf;;
 
-	intr_mask = DW_IC_INTR_STOP_DET | DW_IC_INTR_TX_ABRT | DW_IC_INTR_RX_FULL;
+	intr_mask = DW_IC_INTR_DEFAULT_MASK;
 
 	for (; dev->msg_write_idx < dev->msgs_num; dev->msg_write_idx++) {
 		/* if target address has changed, we need to
@@ -405,11 +413,12 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 
 		if (buf_len > 0) {
 			/* more bytes to be written */
-			intr_mask |= DW_IC_INTR_TX_EMPTY;
 			dev->status |= STATUS_WRITE_IN_PROGRESS;
 			break;
-		} else
+		} else {
 			dev->status &= ~STATUS_WRITE_IN_PROGRESS;
+			intr_mask &= ~DW_IC_INTR_TX_EMPTY;
+		}
 	}
 
 	writel(intr_mask, dev->base + DW_IC_INTR_MASK);
@@ -479,7 +488,6 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 
 	/* start the transfers */
 	i2c_dw_xfer_init(dev);
-	i2c_dw_xfer_msg(dev);
 
 	/* wait for tx to complete */
 	ret = wait_for_completion_interruptible_timeout(&dev->cmd_complete, HZ);
@@ -687,7 +695,7 @@ static int __devinit dw_i2c_probe(struct platform_device *pdev)
 	i2c_dw_init(dev);
 
 	writel(0, dev->base + DW_IC_INTR_MASK); /* disable IRQ */
-	r = request_irq(dev->irq, i2c_dw_isr, 0, pdev->name, dev);
+	r = request_irq(dev->irq, i2c_dw_isr, IRQF_DISABLED, pdev->name, dev);
 	if (r) {
 		dev_err(&pdev->dev, "failure requesting irq %i\n", dev->irq);
 		goto err_iounmap;
-- 
1.6.5.2

[PATCH v2 18/22] i2c-designware: Disable TX_EMPTY when all i2c_msg msgs has been processed

[Shinya Kuribayashi]

Currently we disable TX_EMPTY interrupt when buf_len is zero, but this
is wrong.  (buf_len == 0) means that all transmit data in the current
i2c_msg message has been sent out, but that doesn't necessarily mean
all i2c_msg messages have been processed.

TX_EMPTY interrupt is used as the driving force of DW I2C transactions,
so we need to keep it enabled as long as i2c_msg messages are available.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index f184d82..6acbe84 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -415,12 +415,17 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 			/* more bytes to be written */
 			dev->status |= STATUS_WRITE_IN_PROGRESS;
 			break;
-		} else {
+		} else
 			dev->status &= ~STATUS_WRITE_IN_PROGRESS;
-			intr_mask &= ~DW_IC_INTR_TX_EMPTY;
-		}
 	}
 
+	/*
+	 * If i2c_msg index search is completed, we don't need TX_EMPTY
+	 * interrupt any more.
+	 */
+	if (dev->msg_write_idx == dev->msgs_num)
+		intr_mask &= ~DW_IC_INTR_TX_EMPTY;
+
 	writel(intr_mask, dev->base + DW_IC_INTR_MASK);
 }
 
-- 
1.6.5.2

[PATCH v2 19/22] i2c-designware: i2c_dw_xfer_msg: Fix error handling procedures

[Shinya Kuribayashi]

Current error handling procedures are not good in two respects:

* Forgot to mark dev->cmd_complete as "completed" on errors

  Once an I2C transaction is initiated, wait_for_completion_
  interruptible_timeout() waits for dev->cmd_complete to be completed.
  We have to take care of it whenever an error is detected, otherwise
  we will have a needless HZ timeout.

* Forgot to disable interrupts

  In the previous patch, interrupt mask operations have been changed.
  We don't disable interrupts at the end of the interrupt handler any
  more, and try to keep RX_FULL (and TX_EMPTY if required) enabled
  during the transaction so that we can send longer data than the size
  of Tx/Rx FIFO.

  If an error is detected, we need to disable interrupts before
  quitting current transaction.

We can work around above points using dev->msg_err effectively.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 6acbe84..cb83671 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -380,14 +380,18 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 		 * reprogram the target address in the i2c
 		 * adapter when we are done with this transfer
 		 */
-		if (msgs[dev->msg_write_idx].addr != addr)
-			return;
+		if (msgs[dev->msg_write_idx].addr != addr) {
+			dev_err(dev->dev,
+				"%s: invalid target address\n", __func__);
+			dev->msg_err = -EINVAL;
+			break;
+		}
 
 		if (msgs[dev->msg_write_idx].len == 0) {
 			dev_err(dev->dev,
 				"%s: invalid message length\n", __func__);
 			dev->msg_err = -EINVAL;
-			return;
+			break;
 		}
 
 		if (!(dev->status & STATUS_WRITE_IN_PROGRESS)) {
@@ -426,6 +430,9 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 	if (dev->msg_write_idx == dev->msgs_num)
 		intr_mask &= ~DW_IC_INTR_TX_EMPTY;
 
+	if (dev->msg_err)
+		intr_mask = 0;
+
 	writel(intr_mask, dev->base + DW_IC_INTR_MASK);
 }
 
@@ -628,7 +635,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 	 * the current transmit status.
 	 */
 
-	if (stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET))
+	if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET)) || dev->msg_err)
 		complete(&dev->cmd_complete);
 
 	return IRQ_HANDLED;
-- 
1.6.5.2

[PATCH v2 20/22] i2c-designware: Skip RX_FULL and TX_EMPTY bits on tx abort errors

[Shinya Kuribayashi]

Suppose TX_ABRT occurs in the middle of processing i2c_msg msgs[], and
a STOP condition has already been generated on the bus.  In this case,
subsequent i2c_dw_xfer_msg() might initiate a new and unnecessary I2C
transaction, which we'd have to avoid.

Furthermore, anytime TX_ABRT is set, the contents of tx/rx buffers are
flushed, so we don't have to process RX_FULL and TX_EMPTY.

Disable interrupts, and skip them.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index cb83671..e6b1e6e 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -621,6 +621,13 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 	if (stat & DW_IC_INTR_TX_ABRT) {
 		dev->cmd_err |= DW_IC_ERR_TX_ABRT;
 		dev->status = STATUS_IDLE;
+
+		/*
+		 * Anytime TX_ABRT is set, the contents of the tx/rx
+		 * buffers are flushed.  Make sure to skip them.
+		 */
+		writel(0, dev->base + DW_IC_INTR_MASK);
+		goto tx_aborted;
 	}
 
 	if (stat & DW_IC_INTR_RX_FULL)
@@ -635,6 +642,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 	 * the current transmit status.
 	 */
 
+tx_aborted:
 	if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET)) || dev->msg_err)
 		complete(&dev->cmd_complete);
 
-- 
1.6.5.2

[PATCH v2 21/22] i2c-designware: Tx abort cleanups

[Shinya Kuribayashi]

* ABRT_MASTER_DIS: Fix a typo.

* i2c_dw_handle_tx_abort: Return an appropriate error number
  depending on abort_source.

* i2c_dw_xfer: Add a missing abort_source initialization.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |   47 +++++++++++++++++++++++++++++-----
 1 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index e6b1e6e..887aed6 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -123,9 +123,27 @@
 #define ABRT_SBYTE_ACKDET	7
 #define ABRT_SBYTE_NORSTRT	9
 #define ABRT_10B_RD_NORSTRT	10
-#define ARB_MASTER_DIS		11
+#define ABRT_MASTER_DIS		11
 #define ARB_LOST		12
 
+#define DW_IC_TX_ABRT_7B_ADDR_NOACK	(1UL << ABRT_7B_ADDR_NOACK)
+#define DW_IC_TX_ABRT_10ADDR1_NOACK	(1UL << ABRT_10ADDR1_NOACK)
+#define DW_IC_TX_ABRT_10ADDR2_NOACK	(1UL << ABRT_10ADDR2_NOACK)
+#define DW_IC_TX_ABRT_TXDATA_NOACK	(1UL << ABRT_TXDATA_NOACK)
+#define DW_IC_TX_ABRT_GCALL_NOACK	(1UL << ABRT_GCALL_NOACK)
+#define DW_IC_TX_ABRT_GCALL_READ	(1UL << ABRT_GCALL_READ)
+#define DW_IC_TX_ABRT_SBYTE_ACKDET	(1UL << ABRT_SBYTE_ACKDET)
+#define DW_IC_TX_ABRT_SBYTE_NORSTRT	(1UL << ABRT_SBYTE_NORSTRT)
+#define DW_IC_TX_ABRT_10B_RD_NORSTRT	(1UL << ABRT_10B_RD_NORSTRT)
+#define DW_IC_TX_ABRT_MASTER_DIS	(1UL << ABRT_MASTER_DIS)
+#define DW_IC_TX_ARB_LOST		(1UL << ARB_LOST)
+
+#define DW_IC_TX_ABRT_NOACK		(DW_IC_TX_ABRT_7B_ADDR_NOACK | \
+					 DW_IC_TX_ABRT_10ADDR1_NOACK | \
+					 DW_IC_TX_ABRT_10ADDR2_NOACK | \
+					 DW_IC_TX_ABRT_TXDATA_NOACK | \
+					 DW_IC_TX_ABRT_GCALL_NOACK)
+
 static char *abort_sources[] = {
 	[ABRT_7B_ADDR_NOACK]	=
 		"slave address not acknowledged (7bit mode)",
@@ -472,6 +490,24 @@ i2c_dw_read(struct dw_i2c_dev *dev)
 	}
 }
 
+static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
+{
+	unsigned long abort_source = dev->abort_source;
+	int i;
+
+	for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
+		dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
+
+	if (abort_source & DW_IC_TX_ARB_LOST)
+		return -EAGAIN;
+	else if (abort_source & DW_IC_TX_ABRT_NOACK)
+		return -EREMOTEIO;
+	else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
+		return -EINVAL; /* wrong msgs[] data */
+	else
+		return -EIO;
+}
+
 /*
  * Prepare controller for a transaction and call i2c_dw_xfer_msg
  */
@@ -493,6 +529,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	dev->msg_read_idx = 0;
 	dev->msg_err = 0;
 	dev->status = STATUS_IDLE;
+	dev->abort_source = 0;
 
 	ret = i2c_dw_wait_bus_not_busy(dev);
 	if (ret < 0)
@@ -526,12 +563,8 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 
 	/* We have an error */
 	if (dev->cmd_err == DW_IC_ERR_TX_ABRT) {
-		unsigned long abort_source = dev->abort_source;
-		int i;
-
-		for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources)) {
-		    dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
-		}
+		ret = i2c_dw_handle_tx_abort(dev);
+		goto done;
 	}
 	ret = -EIO;
 
-- 
1.6.5.2

[PATCH v2 22/22] i2c-designware: Cosmetic cleanups

[Shinya Kuribayashi]

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
---
 drivers/i2c/busses/i2c-designware.c |   27 ++++++++++++++-------------
 1 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 887aed6..4534d45 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -1,5 +1,5 @@
 /*
- * Synopsys Designware I2C adapter driver (master only).
+ * Synopsys DesignWare I2C adapter driver (master only).
  *
  * Based on the TI DAVINCI I2C adapter driver.
  *
@@ -145,27 +145,27 @@
 					 DW_IC_TX_ABRT_GCALL_NOACK)
 
 static char *abort_sources[] = {
-	[ABRT_7B_ADDR_NOACK]	=
+	[ABRT_7B_ADDR_NOACK] =
 		"slave address not acknowledged (7bit mode)",
-	[ABRT_10ADDR1_NOACK]	=
+	[ABRT_10ADDR1_NOACK] =
 		"first address byte not acknowledged (10bit mode)",
-	[ABRT_10ADDR2_NOACK]	=
+	[ABRT_10ADDR2_NOACK] =
 		"second address byte not acknowledged (10bit mode)",
-	[ABRT_TXDATA_NOACK]		=
+	[ABRT_TXDATA_NOACK] =
 		"data not acknowledged",
-	[ABRT_GCALL_NOACK]		=
+	[ABRT_GCALL_NOACK] =
 		"no acknowledgement for a general call",
-	[ABRT_GCALL_READ]		=
+	[ABRT_GCALL_READ] =
 		"read after general call",
-	[ABRT_SBYTE_ACKDET]		=
+	[ABRT_SBYTE_ACKDET] =
 		"start byte acknowledged",
-	[ABRT_SBYTE_NORSTRT]	=
+	[ABRT_SBYTE_NORSTRT] =
 		"trying to send start byte when restart is disabled",
-	[ABRT_10B_RD_NORSTRT]	=
+	[ABRT_10B_RD_NORSTRT] =
 		"trying to read when restart is disabled (10bit mode)",
-	[ARB_MASTER_DIS]		=
+	[ABRT_MASTER_DIS] =
 		"trying to use disabled adapter",
-	[ARB_LOST]			=
+	[ARB_LOST] =
 		"lost arbitration",
 };
 
@@ -394,7 +394,8 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
 	intr_mask = DW_IC_INTR_DEFAULT_MASK;
 
 	for (; dev->msg_write_idx < dev->msgs_num; dev->msg_write_idx++) {
-		/* if target address has changed, we need to
+		/*
+		 * if target address has changed, we need to
 		 * reprogram the target address in the i2c
 		 * adapter when we are done with this transfer
 		 */
-- 
1.6.5.2

Re: [PATCH v2] i2c-designware updates

[Ben Dooks]

On Fri, Nov 06, 2009 at 09:42:30PM +0900, Shinya Kuribayashi wrote:
> Hi Baruch and Ben,
>
> here's v2 patchset of DesignWare I2C driver (i2c-designware.c).
> I did all test I can do for now (including arbitration tests), and
> fixed more issues left in the v1 patchset.  I think now the driver is
> in reasonable shape for the mainline, and hope gets merged.
>
> The patches which already have Baruch's Acked-by: line are logically
> same as the previous v1 patches.  For the rest are revised version of
> v1 patches, or newly added in v2.

thanks, the first pass seems ok, will give a thorough review by next
week and get a branch with them on for -next submission.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Re: [PATCH v2] i2c-designware updates

[Ben Dooks]

On Fri, Nov 06, 2009 at 09:42:30PM +0900, Shinya Kuribayashi wrote:
> Hi Baruch and Ben,
>
> here's v2 patchset of DesignWare I2C driver (i2c-designware.c).
> I did all test I can do for now (including arbitration tests), and
> fixed more issues left in the v1 patchset.  I think now the driver is
> in reasonable shape for the mainline, and hope gets merged.
>
> The patches which already have Baruch's Acked-by: line are logically
> same as the previous v1 patches.  For the rest are revised version of
> v1 patches, or newly added in v2.

thanks, the first pass seems ok, will give a thorough review by next
week and get a branch with them on for -next submission.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

[PATCH v2] i2c-designware updates

[Ben Dooks]

On Fri, Nov 06, 2009 at 09:42:30PM +0900, Shinya Kuribayashi wrote:
> Hi Baruch and Ben,
>
> here's v2 patchset of DesignWare I2C driver (i2c-designware.c).
> I did all test I can do for now (including arbitration tests), and
> fixed more issues left in the v1 patchset.  I think now the driver is
> in reasonable shape for the mainline, and hope gets merged.
>
> The patches which already have Baruch's Acked-by: line are logically
> same as the previous v1 patches.  For the rest are revised version of
> v1 patches, or newly added in v2.

thanks, the first pass seems ok, will give a thorough review by next
week and get a branch with them on for -next submission.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Re: [PATCH v2 21/22] i2c-designware: Tx abort cleanups

[Shinya Kuribayashi]

Baruch,

Shinya Kuribayashi wrote:
> * ABRT_MASTER_DIS: Fix a typo.
> 
> * i2c_dw_handle_tx_abort: Return an appropriate error number
>   depending on abort_source.
> 
> * i2c_dw_xfer: Add a missing abort_source initialization.
> 
> Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>

[ ... ]

> @@ -472,6 +490,24 @@ i2c_dw_read(struct dw_i2c_dev *dev)
>  	}
>  }
>  
> +static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
> +{
> +	unsigned long abort_source = dev->abort_source;
> +	int i;
> +
> +	for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
> +		dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);

Dev_err() might be annoying when we use sort of misc utilities
such as i2c-tools.  In case of no ACKs, we sometimes don't want
to see error messages in the console, because such tools are
sometimes capable of generating human-friendly console output
like,

 root@localhost:/root> i2cdetect -y -r 2
      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
 00:          -- -- -- -- -- -- -- -- -- -- -- -- --
 10: -- -- -- -- -- -- -- -- ...

 root@localhost:/root> i2cdump -y 0 0x49 b
      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
 00: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX    XXXXXXXXXXXXXXXX
 10: XX XX XX XX XX XX  ....

Then how do I change dev_err() here?  I'm not sure dev_dbg() or
other variants are acceptable for no ACKs or arbitration cases.
Could someone give me a comment?  I'll prepare patches for it.

Thanks in advance,

> +	if (abort_source & DW_IC_TX_ARB_LOST)
> +		return -EAGAIN;
> +	else if (abort_source & DW_IC_TX_ABRT_NOACK)
> +		return -EREMOTEIO;
> +	else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
> +		return -EINVAL; /* wrong msgs[] data */
> +	else
> +		return -EIO;
> +}
> +
>  /*
>   * Prepare controller for a transaction and call i2c_dw_xfer_msg
>   */

-- 
Shinya Kuribayashi
NEC Electronics

Re: [PATCH v2 21/22] i2c-designware: Tx abort cleanups

[Ben Dooks]

On Thu, Nov 12, 2009 at 01:29:46PM +0900, Shinya Kuribayashi wrote:
> Baruch,
> 
> Shinya Kuribayashi wrote:
> >* ABRT_MASTER_DIS: Fix a typo.
> >
> >* i2c_dw_handle_tx_abort: Return an appropriate error number
> >  depending on abort_source.
> >
> >* i2c_dw_xfer: Add a missing abort_source initialization.
> >
> >Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
> 
> [ ... ]
> 
> >@@ -472,6 +490,24 @@ i2c_dw_read(struct dw_i2c_dev *dev)
> > 	}
> > }
> >+static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
> >+{
> >+	unsigned long abort_source = dev->abort_source;
> >+	int i;
> >+
> >+	for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
> >+		dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
> 
> Dev_err() might be annoying when we use sort of misc utilities
> such as i2c-tools.  In case of no ACKs, we sometimes don't want
> to see error messages in the console, because such tools are
> sometimes capable of generating human-friendly console output
> like,
> 
> root@localhost:/root> i2cdetect -y -r 2
>      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
> 00:          -- -- -- -- -- -- -- -- -- -- -- -- --
> 10: -- -- -- -- -- -- -- -- ...
> 
> root@localhost:/root> i2cdump -y 0 0x49 b
>      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
> 00: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX    XXXXXXXXXXXXXXXX
> 10: XX XX XX XX XX XX  ....
> 
> Then how do I change dev_err() here?  I'm not sure dev_dbg() or
> other variants are acceptable for no ACKs or arbitration cases.
> Could someone give me a comment?  I'll prepare patches for it.

Maybe in the case of no-ack, this could be a debug message and erro
for the case where you have an arbitration problem or other error that
you wouldn't normally find on an I2C bus?
 
> Thanks in advance,
> 
> >+	if (abort_source & DW_IC_TX_ARB_LOST)
> >+		return -EAGAIN;
> >+	else if (abort_source & DW_IC_TX_ABRT_NOACK)
> >+		return -EREMOTEIO;
> >+	else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
> >+		return -EINVAL; /* wrong msgs[] data */
> >+	else
> >+		return -EIO;
> >+}
> >+
> > /*
> >  * Prepare controller for a transaction and call i2c_dw_xfer_msg
> >  */
> 
> -- 
> Shinya Kuribayashi
> NEC Electronics
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

Re: [PATCH v2 21/22] i2c-designware: Tx abort cleanups

[Shinya Kuribayashi]

Ben Dooks wrote:
>>> +	for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
>>> +		dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
>> Dev_err() might be annoying when we use sort of misc utilities
>> such as i2c-tools.  In case of no ACKs, we sometimes don't want
>> to see error messages in the console, because such tools are
>> sometimes capable of generating human-friendly console output
>> like,
>>
>> root@localhost:/root> i2cdetect -y -r 2
>>      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
>> 00:          -- -- -- -- -- -- -- -- -- -- -- -- --
>> 10: -- -- -- -- -- -- -- -- ...
>>
>> root@localhost:/root> i2cdump -y 0 0x49 b
>>      0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f    0123456789abcdef
>> 00: XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX    XXXXXXXXXXXXXXXX
>> 10: XX XX XX XX XX XX  ....
>>
>> Then how do I change dev_err() here?  I'm not sure dev_dbg() or
>> other variants are acceptable for no ACKs or arbitration cases.
>> Could someone give me a comment?  I'll prepare patches for it.
> 
> Maybe in the case of no-ack, this could be a debug message and erro
> for the case where you have an arbitration problem or other error that
> you wouldn't normally find on an I2C bus?

Thanks, I'll send an additional patch shortly.
-- 
Shinya Kuribayashi
NEC Electronics

[PATCH 23/22] i2c-designware: i2c_dw_handle_tx_abort: Use dev_dbg() for NOACK cases

[Shinya Kuribayashi]

In the case of no-ACKs, we don't want to see dev_err() messages in the
console, because some utilities like i2c-tools are capable of printing
decorated console output.  This patch will ease such situations.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi-jaWZhaxaiAMAvxtiuMwx3w@public.gmane.org>
---

Hi Ben,

 This patch can be applied on the top of the v2 patchset (as 23/22).
 I've tested the patch with both no-ACK cases and arbitration case.
 As for errors other than NOACKs, it's worth doing dev_err().

 drivers/i2c/busses/i2c-designware.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 4534d45..9e18ef9 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -496,13 +496,18 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 	unsigned long abort_source = dev->abort_source;
 	int i;
 
+	if (abort_source & DW_IC_TX_ABRT_NOACK) {
+		for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
+			dev_dbg(dev->dev,
+				"%s: %s\n", __func__, abort_sources[i]);
+		return -EREMOTEIO;
+	}
+
 	for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
 		dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
 
 	if (abort_source & DW_IC_TX_ARB_LOST)
 		return -EAGAIN;
-	else if (abort_source & DW_IC_TX_ABRT_NOACK)
-		return -EREMOTEIO;
 	else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
 		return -EINVAL; /* wrong msgs[] data */
 	else
-- 
1.6.5.2

[PATCH 23/22] i2c-designware: i2c_dw_handle_tx_abort: Use dev_dbg() for NOACK cases

[Shinya Kuribayashi]

In the case of no-ACKs, we don't want to see dev_err() messages in the
console, because some utilities like i2c-tools are capable of printing
decorated console output.  This patch will ease such situations.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi@necel.com>
---

Hi Ben,

 This patch can be applied on the top of the v2 patchset (as 23/22).
 I've tested the patch with both no-ACK cases and arbitration case.
 As for errors other than NOACKs, it's worth doing dev_err().

 drivers/i2c/busses/i2c-designware.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 4534d45..9e18ef9 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -496,13 +496,18 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 	unsigned long abort_source = dev->abort_source;
 	int i;
 
+	if (abort_source & DW_IC_TX_ABRT_NOACK) {
+		for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
+			dev_dbg(dev->dev,
+				"%s: %s\n", __func__, abort_sources[i]);
+		return -EREMOTEIO;
+	}
+
 	for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
 		dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
 
 	if (abort_source & DW_IC_TX_ARB_LOST)
 		return -EAGAIN;
-	else if (abort_source & DW_IC_TX_ABRT_NOACK)
-		return -EREMOTEIO;
 	else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
 		return -EINVAL; /* wrong msgs[] data */
 	else
-- 
1.6.5.2

[PATCH 23/22] i2c-designware: i2c_dw_handle_tx_abort: Use dev_dbg() for NOACK cases

[Shinya Kuribayashi]

In the case of no-ACKs, we don't want to see dev_err() messages in the
console, because some utilities like i2c-tools are capable of printing
decorated console output.  This patch will ease such situations.

Signed-off-by: Shinya Kuribayashi <shinya.kuribayashi@necel.com>
---

Hi Ben,

 This patch can be applied on the top of the v2 patchset (as 23/22).
 I've tested the patch with both no-ACK cases and arbitration case.
 As for errors other than NOACKs, it's worth doing dev_err().

 drivers/i2c/busses/i2c-designware.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware.c b/drivers/i2c/busses/i2c-designware.c
index 4534d45..9e18ef9 100644
--- a/drivers/i2c/busses/i2c-designware.c
+++ b/drivers/i2c/busses/i2c-designware.c
@@ -496,13 +496,18 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
 	unsigned long abort_source = dev->abort_source;
 	int i;
 
+	if (abort_source & DW_IC_TX_ABRT_NOACK) {
+		for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
+			dev_dbg(dev->dev,
+				"%s: %s\n", __func__, abort_sources[i]);
+		return -EREMOTEIO;
+	}
+
 	for_each_bit(i, &abort_source, ARRAY_SIZE(abort_sources))
 		dev_err(dev->dev, "%s: %s\n", __func__, abort_sources[i]);
 
 	if (abort_source & DW_IC_TX_ARB_LOST)
 		return -EAGAIN;
-	else if (abort_source & DW_IC_TX_ABRT_NOACK)
-		return -EREMOTEIO;
 	else if (abort_source & DW_IC_TX_ABRT_GCALL_READ)
 		return -EINVAL; /* wrong msgs[] data */
 	else
-- 
1.6.5.2

Re: [PATCH v2] i2c-designware updates

[Ben Dooks]

On Fri, Nov 06, 2009 at 09:42:30PM +0900, Shinya Kuribayashi wrote:
> Hi Baruch and Ben,
> 
> here's v2 patchset of DesignWare I2C driver (i2c-designware.c).
> I did all test I can do for now (including arbitration tests), and
> fixed more issues left in the v1 patchset.  I think now the driver is
> in reasonable shape for the mainline, and hope gets merged.
> 
> The patches which already have Baruch's Acked-by: line are logically
> same as the previous v1 patches.  For the rest are revised version of
> v1 patches, or newly added in v2.
> 
> There are 22 patches in total and might be slightly annoying.  I'll
> send subsequent patches only to linux-i2c.

These are all queued at:

	git://git.fluff.org/bjdooks/linux.git next-i2c-designware

-- 
Ben (ben-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

Re: [PATCH v2] i2c-designware updates

[Ben Dooks]

On Fri, Nov 06, 2009 at 09:42:30PM +0900, Shinya Kuribayashi wrote:
> Hi Baruch and Ben,
> 
> here's v2 patchset of DesignWare I2C driver (i2c-designware.c).
> I did all test I can do for now (including arbitration tests), and
> fixed more issues left in the v1 patchset.  I think now the driver is
> in reasonable shape for the mainline, and hope gets merged.
> 
> The patches which already have Baruch's Acked-by: line are logically
> same as the previous v1 patches.  For the rest are revised version of
> v1 patches, or newly added in v2.
> 
> There are 22 patches in total and might be slightly annoying.  I'll
> send subsequent patches only to linux-i2c.

These are all queued at:

	git://git.fluff.org/bjdooks/linux.git next-i2c-designware

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

[PATCH v2] i2c-designware updates

[Ben Dooks]

On Fri, Nov 06, 2009 at 09:42:30PM +0900, Shinya Kuribayashi wrote:
> Hi Baruch and Ben,
> 
> here's v2 patchset of DesignWare I2C driver (i2c-designware.c).
> I did all test I can do for now (including arbitration tests), and
> fixed more issues left in the v1 patchset.  I think now the driver is
> in reasonable shape for the mainline, and hope gets merged.
> 
> The patches which already have Baruch's Acked-by: line are logically
> same as the previous v1 patches.  For the rest are revised version of
> v1 patches, or newly added in v2.
> 
> There are 22 patches in total and might be slightly annoying.  I'll
> send subsequent patches only to linux-i2c.

These are all queued at:

	git://git.fluff.org/bjdooks/linux.git next-i2c-designware

-- 
Ben (ben at fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'