[RFC 0/5] i2c: omap: new fixes 2

[RFC 0/5] i2c: omap: new fixes 2

[Alexander Kochetkov]

This pacth series intended for fixing problem reported
by Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> here[1]

One of first four patched could fix the problem.
Last patch provide event trace so I could resolve problem.
It could be applied using 'git am' or 'patch -p1 ...'

Patches are rebased on branch 'i2c/for-next' of
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux
(6e79807443cba7397cd855ed29d6faba51d4c893)

Tony, could you check, does the series fix the problem reported[1]?
If yes, could you bisect and point commit that solve.
If no, could you provide trace output (with or without the patches
from series).
If no, could you check does i2c-omap.c from commit ca1f8da9ac5ce6e63d8f6933f83fabc1f3f961f4.
(the commit before my changes to kernel) work for you?

Thank you.

Regards,
Alexander.

[1] http://www.spinics.net/lists/linux-i2c/msg17811.html

Alexander Kochetkov (5):
  i2c: omap: ack only reported events
  i2c: omap: simplify i462 errata handling for NACK and AL cases
  i2c: omap: move STP generation logic into ISR thread
  i2c: omap: reimpelement STP hack via 2-phases transfer
  i2c: omap: add trace

 drivers/i2c/busses/i2c-omap.c |  162 ++++++++++++++++++++++++-----------------
 1 file changed, 95 insertions(+), 67 deletions(-)

-- 
1.7.9.5

[RFC 1/5] i2c: omap: ack only reported events

[Alexander Kochetkov]

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
 drivers/i2c/busses/i2c-omap.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 4563200..53b4234 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1059,7 +1059,11 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 
 		if (stat & (OMAP_I2C_STAT_ARDY | OMAP_I2C_STAT_NACK |
 					OMAP_I2C_STAT_AL)) {
-			omap_i2c_ack_stat(dev, (OMAP_I2C_STAT_RRDY |
+			/*
+			 * These are pending events not handled in time
+			 * due to ISR latency.
+			 */
+			omap_i2c_ack_stat(dev, stat & (OMAP_I2C_STAT_RRDY |
 						OMAP_I2C_STAT_RDR |
 						OMAP_I2C_STAT_XRDY |
 						OMAP_I2C_STAT_XDR |
-- 
1.7.9.5

[RFC 2/5] i2c: omap: simplify i462 errata handling for NACK and AL cases

[Alexander Kochetkov]

Carry out NACK and AL handling to main event loop as it should be.

The change affects omap3530 and early boards.
Tested and simulated on omap3730 (Beagleboard XM C).

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
 drivers/i2c/busses/i2c-omap.c |   39 ++++++---------------------------------
 1 file changed, 6 insertions(+), 33 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 53b4234..8591c0c 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -907,22 +907,8 @@ static int errata_omap3_i462(struct omap_i2c_dev *dev)
 		if (stat & OMAP_I2C_STAT_XUDF)
 			break;
 
-		if (stat & (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
-			omap_i2c_ack_stat(dev, (OMAP_I2C_STAT_XRDY |
-							OMAP_I2C_STAT_XDR));
-			if (stat & OMAP_I2C_STAT_NACK) {
-				dev->cmd_err |= OMAP_I2C_STAT_NACK;
-				omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK);
-			}
-
-			if (stat & OMAP_I2C_STAT_AL) {
-				dev_err(dev->dev, "Arbitration lost\n");
-				dev->cmd_err |= OMAP_I2C_STAT_AL;
-				omap_i2c_ack_stat(dev, OMAP_I2C_STAT_AL);
-			}
-
+		if (stat & (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL))
 			return -EIO;
-		}
 
 		cpu_relax();
 	} while (--timeout);
@@ -956,7 +942,7 @@ static void omap_i2c_receive_data(struct omap_i2c_dev *dev, u8 num_bytes,
 	}
 }
 
-static int omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8 num_bytes,
+static void omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8 num_bytes,
 		bool is_xdr)
 {
 	u16		w;
@@ -975,17 +961,12 @@ static int omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8 num_bytes,
 		}
 
 		if (dev->errata & I2C_OMAP_ERRATA_I462) {
-			int ret;
-
-			ret = errata_omap3_i462(dev);
-			if (ret < 0)
-				return ret;
+			if (errata_omap3_i462(dev))
+				break;
 		}
 
 		omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
 	}
-
-	return 0;
 }
 
 static irqreturn_t
@@ -1101,30 +1082,22 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 
 		if (stat & OMAP_I2C_STAT_XDR) {
 			u8 num_bytes = 1;
-			int ret;
 
 			if (dev->fifo_size)
 				num_bytes = dev->buf_len;
 
-			ret = omap_i2c_transmit_data(dev, num_bytes, true);
-			if (ret < 0)
-				break;
-
+			omap_i2c_transmit_data(dev, num_bytes, true);
 			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XDR);
 			continue;
 		}
 
 		if (stat & OMAP_I2C_STAT_XRDY) {
 			u8 num_bytes = 1;
-			int ret;
 
 			if (dev->threshold)
 				num_bytes = dev->threshold;
 
-			ret = omap_i2c_transmit_data(dev, num_bytes, false);
-			if (ret < 0)
-				break;
-
+			omap_i2c_transmit_data(dev, num_bytes, false);
 			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_XRDY);
 			continue;
 		}
-- 
1.7.9.5

[RFC 3/5] i2c: omap: move STP generation logic into ISR thread

[Alexander Kochetkov]

The main reason is to avoid CON register access by omap_i2c_xfer_msg()
function, after transfer was submitted to IP.

Also changed NACK logic to wait ARDY so we can be sure what STP has
been sent on back to back transfers.

Tested on dm3730 (Beagleboard XM C).

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
 drivers/i2c/busses/i2c-omap.c |   26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 8591c0c..9e0d359 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -725,12 +725,9 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 		return -EAGAIN;
 
 	if (dev->cmd_err & OMAP_I2C_STAT_NACK) {
+		/* NOTE: IP always send stop on NACK */
 		if (msg->flags & I2C_M_IGNORE_NAK)
 			return 0;
-
-		w = omap_i2c_read_reg(dev, OMAP_I2C_CON_REG);
-		w |= OMAP_I2C_CON_STP;
-		omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, w);
 		return -EREMOTEIO;
 	}
 	return -EIO;
@@ -1022,8 +1019,24 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 		}
 
 		if (stat & OMAP_I2C_STAT_NACK) {
-			err |= OMAP_I2C_STAT_NACK;
+			u16 con;
+
+			dev->cmd_err |= OMAP_I2C_STAT_NACK;
 			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK);
+
+			/*
+			 * According to the TRM, IP *always* send STP and reset
+			 * MST in NACK cases. But for NACK in the middle of
+			 * transfer IP doesn't do it. So, here is workaround.
+			 */
+			con = omap_i2c_read_reg(dev, OMAP_I2C_CON_REG);
+			if (con & OMAP_I2C_CON_MST) {
+				con |= OMAP_I2C_CON_STP;
+				omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, con);
+			}
+
+			/* Wait for ARDY */
+			continue;
 		}
 
 		if (stat & OMAP_I2C_STAT_AL) {
@@ -1038,8 +1051,7 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 		if (stat & OMAP_I2C_STAT_ARDY)
 			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_ARDY);
 
-		if (stat & (OMAP_I2C_STAT_ARDY | OMAP_I2C_STAT_NACK |
-					OMAP_I2C_STAT_AL)) {
+		if (stat & (OMAP_I2C_STAT_ARDY | OMAP_I2C_STAT_AL)) {
 			/*
 			 * These are pending events not handled in time
 			 * due to ISR latency.
-- 
1.7.9.5

[RFC 4/5] i2c: omap: reimpelement STP hack via 2-phases transfer

[Alexander Kochetkov]

The main reason is to avoid CON register access by omap_i2c_xfer_msg()
function, after transfer was submitted to IP.

The change takes into account comment from the code:
"Don't write stt and stp together on some hardware."

That mean, what some hardware doesn't support "Full transfers",
transfers with STT and STP bits set together.

While this is doesn't correspond errata and TRM, assume it's true.
According to TRM, "Full transfer" could be expessed with "2 phases transfer":
first start with STT and STP bits set to 1 and 0, second with
0 and 1.

The change affects omap3530 and early boards.
Tested and simulated on omap3730 (Beagleboard XM C).

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
 drivers/i2c/busses/i2c-omap.c |   46 ++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 9e0d359..66506db 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -213,6 +213,7 @@ struct omap_i2c_dev {
 						 * the I2C bus state
 						 */
 	unsigned		receiver:1;	/* true when we're in receiver mode */
+	unsigned		stop:1;		/* ISR send STP after xfer */
 	u16			iestate;	/* Saved interrupt register */
 	u16			pscstate;
 	u16			scllstate;
@@ -669,6 +670,11 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	if (!dev->b_hw && stop)
 		w |= OMAP_I2C_CON_STP;
 	/*
+	 * Don't write stt and stp together on some hardware.
+	 */
+	dev->stop = (dev->b_hw && stop);
+
+	/*
 	 * NOTE: STAT_BB bit could became 1 here if another master occupy
 	 * the bus. IP successfully complete transfer when the bus will be
 	 * free again (BB reset to 0).
@@ -676,29 +682,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, w);
 
 	/*
-	 * Don't write stt and stp together on some hardware.
-	 */
-	if (dev->b_hw && stop) {
-		unsigned long delay = jiffies + OMAP_I2C_TIMEOUT;
-		u16 con = omap_i2c_read_reg(dev, OMAP_I2C_CON_REG);
-		while (con & OMAP_I2C_CON_STT) {
-			con = omap_i2c_read_reg(dev, OMAP_I2C_CON_REG);
-
-			/* Let the user know if i2c is in a bad state */
-			if (time_after(jiffies, delay)) {
-				dev_err(dev->dev, "controller timed out "
-				"waiting for start condition to finish\n");
-				return -ETIMEDOUT;
-			}
-			cpu_relax();
-		}
-
-		w |= OMAP_I2C_CON_STP;
-		w &= ~OMAP_I2C_CON_STT;
-		omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, w);
-	}
-
-	/*
 	 * REVISIT: We should abort the transfer on signals, but the bus goes
 	 * into arbitration and we're currently unable to recover from it.
 	 */
@@ -1021,6 +1004,7 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 		if (stat & OMAP_I2C_STAT_NACK) {
 			u16 con;
 
+			dev->stop = 0;
 			dev->cmd_err |= OMAP_I2C_STAT_NACK;
 			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_NACK);
 
@@ -1041,6 +1025,7 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 
 		if (stat & OMAP_I2C_STAT_AL) {
 			dev_err(dev->dev, "Arbitration lost\n");
+			dev->stop = 0;
 			err |= OMAP_I2C_STAT_AL;
 			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_AL);
 		}
@@ -1051,6 +1036,21 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 		if (stat & OMAP_I2C_STAT_ARDY)
 			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_ARDY);
 
+		if ((stat & OMAP_I2C_STAT_ARDY) && dev->stop) {
+			u16 con;
+			/* Second phase transfer with STT:STP=01 */
+			omap_i2c_ack_stat(dev, OMAP_I2C_STAT_ARDY);
+			dev->stop = 0;
+
+			con = omap_i2c_read_reg(dev, OMAP_I2C_CON_REG);
+			con |= OMAP_I2C_CON_STP;
+			con &= ~OMAP_I2C_CON_STT;
+			omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, con);
+
+			/* Wait for the next ARDY */
+			continue;
+		}
+
 		if (stat & (OMAP_I2C_STAT_ARDY | OMAP_I2C_STAT_AL)) {
 			/*
 			 * These are pending events not handled in time
-- 
1.7.9.5

[RFC 5/5] i2c: omap: add trace

[Alexander Kochetkov]

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
---
 drivers/i2c/busses/i2c-omap.c |   45 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 66506db..d2dfabe 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -283,6 +283,23 @@ static inline u16 omap_i2c_read_reg(struct omap_i2c_dev *i2c_dev, int reg)
 				(i2c_dev->regs[reg] << i2c_dev->reg_shift));
 }
 
+#ifdef dev_dbg
+#undef dev_dbg
+#endif
+#define dev_dbg dev_info
+
+static inline void omap_i2c_dump_state(const char *func, int line,
+	struct omap_i2c_dev *dev, const char *msg)
+{
+	dev_dbg(dev->dev, "%s: STAT=0x%04x; IE=0x%04x; CON=0x%04x; (%s:%d)\n",
+		msg,
+		omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG),
+		omap_i2c_read_reg(dev, OMAP_I2C_IE_REG),
+		omap_i2c_read_reg(dev, OMAP_I2C_CON_REG),
+		func, line);
+}
+#define OMAP_I2C_DUMP_STATE(dev, msg) omap_i2c_dump_state(__func__, __LINE__, (dev), (msg))
+
 static void __omap_i2c_init(struct omap_i2c_dev *dev)
 {
 
@@ -393,6 +410,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
 		 */
 		if (fclk_rate > 12000000)
 			psc = fclk_rate / 12000000;
+
+		dev_info(dev->dev, "ARMXOR_CLK: fclk_rate=%lu\n", fclk_rate);
 	}
 
 	if (!(dev->flags & OMAP_I2C_FLAG_SIMPLE_CLOCK)) {
@@ -445,6 +464,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
 		}
 		scll = (hsscll << OMAP_I2C_SCLL_HSSCLL) | fsscll;
 		sclh = (hssclh << OMAP_I2C_SCLH_HSSCLH) | fssclh;
+
+		dev_info(dev->dev, "!SIMPLE_CLOCK: fclk_rate=%lu; internal_clk=%lu; speed %u\n", fclk_rate, internal_clk, dev->speed);
 	} else {
 		/* Program desired operating rate */
 		fclk_rate /= (psc + 1) * 1000;
@@ -452,6 +473,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
 			psc = 2;
 		scll = fclk_rate / (dev->speed * 2) - 7 + psc;
 		sclh = fclk_rate / (dev->speed * 2) - 7 + psc;
+
+		dev_info(dev->dev, "SIMPLE_CLOCK: fclk_rate=%lu; speed %u\n", fclk_rate, dev->speed);
 	}
 
 	dev->iestate = (OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY |
@@ -679,7 +702,9 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	 * the bus. IP successfully complete transfer when the bus will be
 	 * free again (BB reset to 0).
 	 */
+	OMAP_I2C_DUMP_STATE(dev, "XFER0");
 	omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, w);
+	OMAP_I2C_DUMP_STATE(dev, "XFER1");
 
 	/*
 	 * REVISIT: We should abort the transfer on signals, but the bus goes
@@ -688,12 +713,16 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	timeout = wait_for_completion_timeout(&dev->cmd_complete,
 						OMAP_I2C_TIMEOUT);
 	if (timeout == 0) {
+		OMAP_I2C_DUMP_STATE(dev, "XFER TIMEOUT");
 		dev_err(dev->dev, "controller timed out\n");
 		omap_i2c_reset(dev);
 		__omap_i2c_init(dev);
 		return -ETIMEDOUT;
 	}
 
+	OMAP_I2C_DUMP_STATE(dev, "XFER DONE");
+	dev_info(dev->dev, "cmd_err=%04x\n", dev->cmd_err);
+
 	if (likely(!dev->cmd_err))
 		return 0;
 
@@ -984,6 +1013,8 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 		stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
 		stat &= bits;
 
+		OMAP_I2C_DUMP_STATE(dev, "THR");
+
 		/* If we're in receiver mode, ignore XDR/XRDY */
 		if (dev->receiver)
 			stat &= ~(OMAP_I2C_STAT_XDR | OMAP_I2C_STAT_XRDY);
@@ -998,7 +1029,7 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 		dev_dbg(dev->dev, "IRQ (ISR = 0x%04x)\n", stat);
 		if (count++ == 100) {
 			dev_warn(dev->dev, "Too much work in one IRQ\n");
-			break;
+			goto out;
 		}
 
 		if (stat & OMAP_I2C_STAT_NACK) {
@@ -1129,10 +1160,12 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
 		}
 	} while (stat);
 
+	OMAP_I2C_DUMP_STATE(dev, "COMPLETE");
 	omap_i2c_complete_cmd(dev, err);
 
 out:
 	spin_unlock_irqrestore(&dev->lock, flags);
+	OMAP_I2C_DUMP_STATE(dev, "THR DONE");
 
 	return IRQ_HANDLED;
 }
@@ -1238,6 +1271,9 @@ omap_i2c_probe(struct platform_device *pdev)
 		of_property_read_u32(node, "clock-frequency", &freq);
 		/* convert DT freq value in Hz into kHz for speed */
 		dev->speed = freq / 1000;
+
+		dev_info(&pdev->dev, "of: compatible=%s; speed=%d; flags=%x\n",
+			match->compatible, dev->speed, dev->flags);
 	} else if (pdata != NULL) {
 		dev->speed = pdata->clkrate;
 		dev->flags = pdata->flags;
@@ -1356,8 +1392,8 @@ omap_i2c_probe(struct platform_device *pdev)
 		goto err_unuse_clocks;
 	}
 
-	dev_info(dev->dev, "bus %d rev%d.%d at %d kHz\n", adap->nr,
-		 major, minor, dev->speed);
+	dev_info(dev->dev, "bus %d rev%d.%d at %d kHz (rev %08x)\n", adap->nr,
+		 major, minor, dev->speed, dev->rev);
 
 	pm_runtime_mark_last_busy(dev->dev);
 	pm_runtime_put_autosuspend(dev->dev);
@@ -1396,6 +1432,8 @@ static int omap_i2c_runtime_suspend(struct device *dev)
 	struct platform_device *pdev = to_platform_device(dev);
 	struct omap_i2c_dev *_dev = platform_get_drvdata(pdev);
 
+	OMAP_I2C_DUMP_STATE(_dev, "SUSPEND");
+
 	_dev->iestate = omap_i2c_read_reg(_dev, OMAP_I2C_IE_REG);
 
 	if (_dev->scheme == OMAP_I2C_SCHEME_0)
@@ -1425,6 +1463,7 @@ static int omap_i2c_runtime_resume(struct device *dev)
 		return 0;
 
 	__omap_i2c_init(_dev);
+	OMAP_I2C_DUMP_STATE(_dev, "RESUME");
 
 	return 0;
 }
-- 
1.7.9.5

Re: [RFC 5/5] i2c: omap: add trace

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]

On Wed, Dec 03, 2014 at 06:34:02PM +0400, Alexander Kochetkov wrote:
> Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-omap.c |   45 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 66506db..d2dfabe 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -283,6 +283,23 @@ static inline u16 omap_i2c_read_reg(struct omap_i2c_dev *i2c_dev, int reg)
>  				(i2c_dev->regs[reg] << i2c_dev->reg_shift));
>  }
>  
> +#ifdef dev_dbg
> +#undef dev_dbg
> +#endif
> +#define dev_dbg dev_info
> +
> +static inline void omap_i2c_dump_state(const char *func, int line,
> +	struct omap_i2c_dev *dev, const char *msg)
> +{
> +	dev_dbg(dev->dev, "%s: STAT=0x%04x; IE=0x%04x; CON=0x%04x; (%s:%d)\n",
> +		msg,
> +		omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG),
> +		omap_i2c_read_reg(dev, OMAP_I2C_IE_REG),
> +		omap_i2c_read_reg(dev, OMAP_I2C_CON_REG),
> +		func, line);
> +}
> +#define OMAP_I2C_DUMP_STATE(dev, msg) omap_i2c_dump_state(__func__, __LINE__, (dev), (msg))

NAK, if you want to add traces, use, well, tracepoints :-) It makes no
sense to hack stuff like this.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC 5/5] i2c: omap: add trace

[Alexander Kochetkov]

03 дек. 2014 г., в 17:49, Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> написал(а):

> NAK, if you want to add traces, use, well, tracepoints :-) It makes no
> sense to hack stuff like this.

It's for testing only the problem reported not for upstream :)
The hack stuff like this is to allow for me to see what happens on the Tony's board.
I don't have omap2430 board.

Regards,
Alexander.

Re: [RFC 5/5] i2c: omap: add trace

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 688 bytes --]

On Wed, Dec 03, 2014 at 05:52:18PM +0300, Alexander Kochetkov wrote:
> 
> 03 дек. 2014 г., в 17:49, Felipe Balbi <balbi@ti.com> написал(а):
> 
> > NAK, if you want to add traces, use, well, tracepoints :-) It makes no
> > sense to hack stuff like this.
> 
> It's for testing only the problem reported not for upstream :)
> The hack stuff like this is to allow for me to see what happens on the Tony's board.
> I don't have omap2430 board.

oh alright, you might want to make it clear by adding "not for merging"
somewhere in the patch. Also, tracepoints would be something that can be
merged upstream *and* can be used for testing when necessary.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC 0/5] i2c: omap: new fixes 2

[Tony Lindgren]

* Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [141203 06:36]:
> This pacth series intended for fixing problem reported
> by Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> here[1]
> 
> One of first four patched could fix the problem.
> Last patch provide event trace so I could resolve problem.
> It could be applied using 'git am' or 'patch -p1 ...'
> 
> Patches are rebased on branch 'i2c/for-next' of
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux
> (6e79807443cba7397cd855ed29d6faba51d4c893)
> 
> Tony, could you check, does the series fix the problem reported[1]?
> If yes, could you bisect and point commit that solve.
> If no, could you provide trace output (with or without the patches
> from series).
> If no, could you check does i2c-omap.c from commit ca1f8da9ac5ce6e63d8f6933f83fabc1f3f961f4.
> (the commit before my changes to kernel) work for you?

It seems this is not related to your patches. Applying these
cause the following though on my 2430sdp:

omap_i2c 48072000.i2c: addr: 0x004b, len: 2, flags: 0x0, stop: 1
omap_i2c 48072000.i2c: XFER0: STAT=0x0000; IE=0x601f; CON=0x8000; (omap_i2c_xfer_msg:705)
omap_i2c 48072000.i2c: XFER1: STAT=0x0010; IE=0x601f; CON=0x8600; (omap_i2c_xfer_msg:707)
omap_i2c 48072000.i2c: THR: STAT=0x1410; IE=0x601f; CON=0x8600; (omap_i2c_isr_thread:1016)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: THR: STAT=0x1000; IE=0x601f; CON=0x8600; (omap_i2c_isr_thread:1016)
omap_i2c 48072000.i2c: THR DONE: STAT=0x1004; IE=0x601f; CON=0x8600; (omap_i2c_isr_thread:1168)
omap_i2c 48072000.i2c: THR: STAT=0x1004; IE=0x601f; CON=0x8600; (omap_i2c_isr_thread:1016)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: THR: STAT=0x1000; IE=0x601f; CON=0x8602; (omap_i2c_isr_thread:1016)
omap_i2c 48072000.i2c: THR DONE: STAT=0x1000; IE=0x601f; CON=0x8602; (omap_i2c_isr_thread:1168)
omap_i2c 48072000.i2c: XFER TIMEOUT: STAT=0x1000; IE=0x601f; CON=0x8602; (omap_i2c_xfer_msg:716)
omap_i2c 48072000.i2c: controller timed out
twl: Write failed (mod 3, reg 0x0e count 1)

Regards,

Tony

> [1] http://www.spinics.net/lists/linux-i2c/msg17811.html
> 
> Alexander Kochetkov (5):
>   i2c: omap: ack only reported events
>   i2c: omap: simplify i462 errata handling for NACK and AL cases
>   i2c: omap: move STP generation logic into ISR thread
>   i2c: omap: reimpelement STP hack via 2-phases transfer
>   i2c: omap: add trace
> 
>  drivers/i2c/busses/i2c-omap.c |  162 ++++++++++++++++++++++++-----------------
>  1 file changed, 95 insertions(+), 67 deletions(-)
> 
> -- 
> 1.7.9.5
> 

Re: [RFC 0/5] i2c: omap: new fixes 2

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1182 bytes --]

On Wed, Dec 03, 2014 at 06:33:57PM +0400, Alexander Kochetkov wrote:
> This pacth series intended for fixing problem reported
> by Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> here[1]
> 
> One of first four patched could fix the problem.
> Last patch provide event trace so I could resolve problem.
> It could be applied using 'git am' or 'patch -p1 ...'
> 
> Patches are rebased on branch 'i2c/for-next' of
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux
> (6e79807443cba7397cd855ed29d6faba51d4c893)
> 
> Tony, could you check, does the series fix the problem reported[1]?
> If yes, could you bisect and point commit that solve.
> If no, could you provide trace output (with or without the patches
> from series).
> If no, could you check does i2c-omap.c from commit ca1f8da9ac5ce6e63d8f6933f83fabc1f3f961f4.
> (the commit before my changes to kernel) work for you?
> 
> Thank you.
> 
> Regards,
> Alexander.

Thanks for working hard on this driver. I'll mark these patches as RFC,
since they did not get any additional tags from other people. If they
are intended for upstream, please resend a rebased version.

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]