2014-12-03 14:34:12

by Alexander Kochetkov

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

This pacth series intended for fixing problem reported
by Tony Lindgren <[email protected]> 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


2014-12-03 14:34:17

by Alexander Kochetkov

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

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 <[email protected]>
---
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

2014-12-03 14:34:29

by Alexander Kochetkov

[permalink] [raw]
Subject: [RFC 5/5] i2c: omap: add trace

Signed-off-by: Alexander Kochetkov <[email protected]>
---
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

2014-12-03 14:34:48

by Alexander Kochetkov

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

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 <[email protected]>
---
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

2014-12-03 14:35:16

by Alexander Kochetkov

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

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 <[email protected]>
---
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

2014-12-03 14:35:42

by Alexander Kochetkov

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

Signed-off-by: Alexander Kochetkov <[email protected]>
---
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

2014-12-03 14:49:38

by Felipe Balbi

[permalink] [raw]
Subject: Re: [RFC 5/5] i2c: omap: add trace

On Wed, Dec 03, 2014 at 06:34:02PM +0400, Alexander Kochetkov wrote:
> Signed-off-by: Alexander Kochetkov <[email protected]>
> ---
> 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


Attachments:
(No filename) (1.24 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-12-03 14:52:26

by Alexander Kochetkov

[permalink] [raw]
Subject: Re: [RFC 5/5] i2c: omap: add trace


03 ???. 2014 ?., ? 17:49, Felipe Balbi <[email protected]> ???????(?):

> 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.

2014-12-03 14:55:46

by Felipe Balbi

[permalink] [raw]
Subject: Re: [RFC 5/5] i2c: omap: add trace

On Wed, Dec 03, 2014 at 05:52:18PM +0300, Alexander Kochetkov wrote:
>
> 03 дек. 2014 г., в 17:49, Felipe Balbi <[email protected]> написал(а):
>
> > 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


Attachments:
(No filename) (671.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-12-04 18:25:16

by Tony Lindgren

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

* Alexander Kochetkov <[email protected]> [141203 06:36]:
> This pacth series intended for fixing problem reported
> by Tony Lindgren <[email protected]> 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
>