2014-11-29 21:00:12

by Alexander Kochetkov

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

The first patch fix i2c-omap driver for omap2420, found by code review and
later reported by Tony Lindgren <[email protected]> here[1].
Candidate for stable?

The second patch unhide the reson of system lockup.

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

Tony, could you check, what the patches fix the problem reported[1]?
Kevin, could you run tests for patches on all omap boards?

Regards,
Alexander.

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

Alexander Kochetkov (2):
i2c: omap: fix buffer overruns during RX/TX data processing
i2c: omap: show that the reason of system lockup is an unhandled ISR
event

drivers/i2c/busses/i2c-omap.c | 64 ++++++++++++++++++++++++++++++++---------
1 file changed, 50 insertions(+), 14 deletions(-)

--
1.7.9.5


2014-11-29 21:00:24

by Alexander Kochetkov

[permalink] [raw]
Subject: [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing

commit dd74548ddece4b9d68e5528287a272fa552c81d0 ("i2c: omap:
resize fifos before each message") dropped check for dev->buf_len.
As result, data processing loop cause dev->buf overruns for
devices with 16-bit data register (omap2420).

In the dd74548ddece4b9d68 code, for each loop iteration if the
flag OMAP_I2C_FLAG_16BIT_DATA_REG is set (omap2420), dev->buf
is incremented twice, and dev->buf_len decremented twice.

Also buffer overrun could happen (in theory) due to wrong
ISR handling (bug).

The commit fix data processing for omap2420 and add guard checks
in the data processing loops do disallow accesses to the buffer,
when dev->buf_len is zero. Also added warnings to unhide the bug.

Found by code review.

Signed-off-by: Alexander Kochetkov <[email protected]>
Fixes: dd74548ddece4b9d68e5528287a272fa552c81d0 "i2c: omap: resize fifos before each message"
Reported-by: Tony Lindgren <[email protected]>
---
drivers/i2c/busses/i2c-omap.c | 36 ++++++++++++++++++++++++++++--------
1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 4563200..e890295 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -938,20 +938,30 @@ static int errata_omap3_i462(struct omap_i2c_dev *dev)
static void omap_i2c_receive_data(struct omap_i2c_dev *dev, u8 num_bytes,
bool is_rdr)
{
- u16 w;
+ u16 w;
+
+ if (unlikely(num_bytes > dev->buf_len)) {
+ dev_err(dev->dev, "%s interrupt can't receive %u byte(s)\n",
+ is_rdr ? "RDR" : "RRDY", (num_bytes - dev->buf_len));
+ num_bytes = dev->buf_len;
+ }

- while (num_bytes--) {
+ while (num_bytes) {
w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG);
*dev->buf++ = w;
dev->buf_len--;
+ num_bytes--;

/*
* Data reg in 2430, omap3 and
* omap4 is 8 bit wide
*/
if (dev->flags & OMAP_I2C_FLAG_16BIT_DATA_REG) {
- *dev->buf++ = w >> 8;
- dev->buf_len--;
+ if (num_bytes) {
+ *dev->buf++ = w >> 8;
+ dev->buf_len--;
+ num_bytes--;
+ }
}
}
}
@@ -959,19 +969,29 @@ 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,
bool is_xdr)
{
- u16 w;
+ u16 w;
+
+ if (unlikely(num_bytes > dev->buf_len)) {
+ dev_err(dev->dev, "%s interrupt can't transmit %u byte(s)\n",
+ is_xdr ? "XDR" : "XRDY", (num_bytes - dev->buf_len));
+ num_bytes = dev->buf_len;
+ }

- while (num_bytes--) {
+ while (num_bytes) {
w = *dev->buf++;
dev->buf_len--;
+ num_bytes--;

/*
* Data reg in 2430, omap3 and
* omap4 is 8 bit wide
*/
if (dev->flags & OMAP_I2C_FLAG_16BIT_DATA_REG) {
- w |= *dev->buf++ << 8;
- dev->buf_len--;
+ if (num_bytes) {
+ w |= *dev->buf++ << 8;
+ dev->buf_len--;
+ num_bytes--;
+ }
}

if (dev->errata & I2C_OMAP_ERRATA_I462) {
--
1.7.9.5

2014-11-29 21:00:44

by Alexander Kochetkov

[permalink] [raw]
Subject: [RFC 2/2] i2c: omap: show that the reason of system lockup is an unhandled ISR event

Commit 079d8af24b948261e1dae5d7df6b31b7bf205cb4 ("i2c: omap: bus:
add a receiver flag") changed ISR loop to silently ignore unwanted
XDR/XRDY or RDR/RRDY events without processing. That unwanted events
will fire interrupt again and most likely they will be ignored again.
As a result, system enters lockup state. CPU loops through omap_i2c_isr()
and omap_i2c_isr_thread(). The lockup is signaled with the message
"sched: RT throttling activated" shown on console.

The commit allow unwanted events to enter into processing loop and
to be acked (at least) and processed (show error message).
Sometimes if unwannted event is acked, the ISR can continue it's work.
Sometimes system still remains in the lockup, but at least the reason
is obvious.

That unwanted events I tested with are events from IP slave receiver,
fired when General Call (GC) command is detected on the bus. As after
every master transfer IP enter into slave receiver mode. ISR must be
ready for events from slave receiver or slave receiver must be
disabled after master transfer to avoid lockups.

Signed-off-by: Alexander Kochetkov <[email protected]>
Fixes 079d8af24b948261e1dae5d7df6b31b7bf205cb4 "i2c: omap: bus: add a receiver flag"
---
drivers/i2c/busses/i2c-omap.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index e890295..192f9ce 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -939,6 +939,22 @@ static void omap_i2c_receive_data(struct omap_i2c_dev *dev, u8 num_bytes,
bool is_rdr)
{
u16 w;
+ int warn = 0;
+
+ if (unlikely(!dev->receiver)) {
+ warn = 1;
+ /* REVISIT: errata i207 during transmission? */
+ if (dev->errata & I2C_OMAP_ERRATA_I207) {
+ if (is_rdr & !num_bytes)
+ warn = 0;
+ }
+ num_bytes = 0;
+ }
+
+ if (unlikely(warn)) {
+ dev_err(dev->dev, "%s interrupt while sending\n",
+ is_rdr ? "RDR" : "RRDY");
+ }

if (unlikely(num_bytes > dev->buf_len)) {
dev_err(dev->dev, "%s interrupt can't receive %u byte(s)\n",
@@ -971,6 +987,12 @@ static int omap_i2c_transmit_data(struct omap_i2c_dev *dev, u8 num_bytes,
{
u16 w;

+ if (unlikely(dev->receiver)) {
+ dev_err(dev->dev, "%s interrupt while receiving\n",
+ is_xdr ? "XDR" : "XRDY");
+ num_bytes = 0;
+ }
+
if (unlikely(num_bytes > dev->buf_len)) {
dev_err(dev->dev, "%s interrupt can't transmit %u byte(s)\n",
is_xdr ? "XDR" : "XRDY", (num_bytes - dev->buf_len));
@@ -1043,12 +1065,6 @@ omap_i2c_isr_thread(int this_irq, void *dev_id)
stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
stat &= bits;

- /* If we're in receiver mode, ignore XDR/XRDY */
- if (dev->receiver)
- stat &= ~(OMAP_I2C_STAT_XDR | OMAP_I2C_STAT_XRDY);
- else
- stat &= ~(OMAP_I2C_STAT_RDR | OMAP_I2C_STAT_RRDY);
-
if (!stat) {
/* my work here is done */
goto out;
--
1.7.9.5

2014-12-01 20:00:59

by Tony Lindgren

[permalink] [raw]
Subject: Re: [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing

* Alexander Kochetkov <[email protected]> [141129 13:02]:
> commit dd74548ddece4b9d68e5528287a272fa552c81d0 ("i2c: omap:
> resize fifos before each message") dropped check for dev->buf_len.
> As result, data processing loop cause dev->buf overruns for
> devices with 16-bit data register (omap2420).
>
> In the dd74548ddece4b9d68 code, for each loop iteration if the
> flag OMAP_I2C_FLAG_16BIT_DATA_REG is set (omap2420), dev->buf
> is incremented twice, and dev->buf_len decremented twice.
>
> Also buffer overrun could happen (in theory) due to wrong
> ISR handling (bug).
>
> The commit fix data processing for omap2420 and add guard checks
> in the data processing loops do disallow accesses to the buffer,
> when dev->buf_len is zero. Also added warnings to unhide the bug.
>
> Found by code review.
>
> Signed-off-by: Alexander Kochetkov <[email protected]>
> Fixes: dd74548ddece4b9d68e5528287a272fa552c81d0 "i2c: omap: resize fifos before each message"
> Reported-by: Tony Lindgren <[email protected]>

I think this is a different issue than what I'm seeing.

Not sure if I've seen what you're describing.. The $subject patch
should be reviewed by Felipe and Aaro, but this does not help
things on 2430.

Regards,

Tony


> drivers/i2c/busses/i2c-omap.c | 36 ++++++++++++++++++++++++++++--------
> 1 file changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 4563200..e890295 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -938,20 +938,30 @@ static int errata_omap3_i462(struct omap_i2c_dev *dev)
> static void omap_i2c_receive_data(struct omap_i2c_dev *dev, u8 num_bytes,
> bool is_rdr)
> {
> - u16 w;
> + u16 w;
> +
> + if (unlikely(num_bytes > dev->buf_len)) {
> + dev_err(dev->dev, "%s interrupt can't receive %u byte(s)\n",
> + is_rdr ? "RDR" : "RRDY", (num_bytes - dev->buf_len));
> + num_bytes = dev->buf_len;
> + }
>
> - while (num_bytes--) {
> + while (num_bytes) {
> w = omap_i2c_read_reg(dev, OMAP_I2C_DATA_REG);
> *dev->buf++ = w;
> dev->buf_len--;
> + num_bytes--;
>
> /*
> * Data reg in 2430, omap3 and
> * omap4 is 8 bit wide
> */
> if (dev->flags & OMAP_I2C_FLAG_16BIT_DATA_REG) {
> - *dev->buf++ = w >> 8;
> - dev->buf_len--;
> + if (num_bytes) {
> + *dev->buf++ = w >> 8;
> + dev->buf_len--;
> + num_bytes--;
> + }
> }
> }
> }
> @@ -959,19 +969,29 @@ 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,
> bool is_xdr)
> {
> - u16 w;
> + u16 w;
> +
> + if (unlikely(num_bytes > dev->buf_len)) {
> + dev_err(dev->dev, "%s interrupt can't transmit %u byte(s)\n",
> + is_xdr ? "XDR" : "XRDY", (num_bytes - dev->buf_len));
> + num_bytes = dev->buf_len;
> + }
>
> - while (num_bytes--) {
> + while (num_bytes) {
> w = *dev->buf++;
> dev->buf_len--;
> + num_bytes--;
>
> /*
> * Data reg in 2430, omap3 and
> * omap4 is 8 bit wide
> */
> if (dev->flags & OMAP_I2C_FLAG_16BIT_DATA_REG) {
> - w |= *dev->buf++ << 8;
> - dev->buf_len--;
> + if (num_bytes) {
> + w |= *dev->buf++ << 8;
> + dev->buf_len--;
> + num_bytes--;
> + }
> }
>
> if (dev->errata & I2C_OMAP_ERRATA_I462) {
> --
> 1.7.9.5
>

2014-12-01 20:05:03

by Kevin Hilman

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

Alexander Kochetkov <[email protected]> writes:

> The first patch fix i2c-omap driver for omap2420, found by code review and
> later reported by Tony Lindgren <[email protected]> here[1].
> Candidate for stable?
>
> The second patch unhide the reson of system lockup.
>
> The patch is rebased on branch 'i2c/for-next' of
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux
> (6e79807443cba7397cd855ed29d6faba51d4c893)
>
> Tony, could you check, what the patches fix the problem reported[1]?
> Kevin, could you run tests for patches on all omap boards?

Done. Built v3.18-rc7 + these 2 patches using omap2plus_defconfig. Boot
logs here for your amusement:

http://people.linaro.org/~khilman/tmp/v3.18-rc7-2-gdf84e23f2864/arm-omap2plus_defconfig/

Kevin

2014-12-02 11:17:19

by Alexander Kochetkov

[permalink] [raw]
Subject: Re: [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing


01 ???. 2014 ?., ? 22:58, Tony Lindgren <[email protected]> ???????(?):

> I think this is a different issue than what I'm seeing.

Hello, Tony!

Thank you for testing!

Could check i2c-omap.c from commit ca1f8da9ac5ce6e63d8f6933f83fabc1f3f961f4.
As all my changes comes after it[1]. So I can understand was the problem before my work.

I there was no problems, then try with my first commit:
27caca9d2e01c92b26d0690f065aad093fea01c7

The problems you talk about is this?
[ 9.675994] omap_i2c 48072000.i2c: controller timed out
[ 10.704010] omap_i2c 48072000.i2c: controller timed out
[ 11.734069] omap_i2c 48072000.i2c: controller timed out
root@omap2430sdp:/# [ 12.823638] omap_i2c 48072000.i2c: controller timed out

And how it is possible to switch from ti,omap2430-i2c to ti,omap2420-i2c? They are so different IP, from
the driver point of view. They have different data bus width.

Alexander.

[1]
alexander@ubuntu:busses$ git log --pretty=oneline --reverse ca1f8da9ac5ce6e63d8f6933f83fabc1f3f961f4^..HEAD -- i2c-omap.c
ca1f8da9ac5ce6e63d8f6933f83fabc1f3f961f4 i2c: remove FSF address
27caca9d2e01c92b26d0690f065aad093fea01c7 i2c: omap: fix NACK and Arbitration Lost irq handling
854a59425a0b9600ee974b113aae081c873163f6 i2c: omap: cleanup register definitions
903c3859f77f9b0aace551da03267ef7a211dbc4 i2c: omap: implement workaround for handling invalid BB-bit values
80cc361f14e8fa97119afa3324c2c913915e7252 i2c: omap: don't reset controller if Arbitration Lost detected
39370ab406933efdedb425910f0a36c16667c45f i2c: omap: add notes related to i2c multimaster mode
ccfc866356674cb3a61829d239c685af6e85f197 i2c: omap: fix i207 errata handling
7d168dc7ed384e50bb7bff4920b73550fd2e9fcb Merge branch 'i2c/for-3.19' into i2c/for-next
2f769d173f0e6a2e85d75fe396f18f794fc4a615 omap: i2c: don't check bus state IP rev3.3 and earlier
2b6f66d87b44aaf1f34f071e6f6430c3ccaa8812 i2c: omap: fix buffer overruns during RX/TX data processing
30c52545106785405856c7e7e40b683b79c8084a i2c: omap: show that the reason of system lockup is an unhandled ISR event

2014-12-02 11:21:13

by Alexander Kochetkov

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


01 ???. 2014 ?., ? 23:04, Kevin Hilman <[email protected]> ???????(?):

> Done. Built v3.18-rc7 + these 2 patches using omap2plus_defconfig. Boot
> logs here for your amusement:

Hello, Kevin!

Thank you so much for doing tests.
What a pity you don't have omap2430sdp board in the tests.

Alexander.

2014-12-04 18:11:58

by Tony Lindgren

[permalink] [raw]
Subject: Re: [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing

* Alexander Kochetkov <[email protected]> [141202 03:19]:
>
> 01 дек. 2014 г., в 22:58, Tony Lindgren <[email protected]> написал(а):
>
> > I think this is a different issue than what I'm seeing.
>
> Hello, Tony!
>
> Thank you for testing!
>
> Could check i2c-omap.c from commit ca1f8da9ac5ce6e63d8f6933f83fabc1f3f961f4.
> As all my changes comes after it[1]. So I can understand was the problem before my work.

The issue I'm seeing on 2430sdp is some earlier regression that
has started happening over the past year or so. No idea when
though as it sometimes works and sometimes does not. So it does
not have anything to do with your patches.

> I there was no problems, then try with my first commit:
> 27caca9d2e01c92b26d0690f065aad093fea01c7
>
> The problems you talk about is this?
> [ 9.675994] omap_i2c 48072000.i2c: controller timed out
> [ 10.704010] omap_i2c 48072000.i2c: controller timed out
> [ 11.734069] omap_i2c 48072000.i2c: controller timed out
> root@omap2430sdp:/# [ 12.823638] omap_i2c 48072000.i2c: controller timed out

No, I'm seeing just this:

omap_i2c 48070000.i2c: bus 0 rev3.3 at 100 kHz
omap_i2c 48072000.i2c: bus 1 rev3.3 at 100 kHz
twl 1-0048: PIH (irq 216) chaining IRQs 217..225
twl 1-0048: power (irq 222) chaining IRQs 225..232

And the system just hangs. With i2c-omap debug enabled, the lines
around the twl are:

omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x0, stop: 0
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x1, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
twl 1-0048: PIH (irq 216) chaining IRQs 217..225
twl 1-0048: power (irq 222) chaining IRQs 225..232
omap_i2c 48072000.i2c: addr: 0x0049, len: 1, flags: 0x0, stop: 0
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x0049, len: 1, flags: 0x1, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x0049, len: 2, flags: 0x0, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x0, stop: 0
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x1, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x0049, len: 1, flags: 0x0, stop: 0
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x0049, len: 1, flags: 0x1, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 2, flags: 0x0, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)

And then it hangs. In the working case, the output continues with:

omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x0, stop: 0
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x1, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
twl 1-0048: PIH (irq 216) chaining IRQs 217..225
twl 1-0048: power (irq 222) chaining IRQs 225..232
omap_i2c 48072000.i2c: addr: 0x0049, len: 1, flags: 0x0, stop: 0
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x0049, len: 1, flags: 0x1, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x0049, len: 2, flags: 0x0, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x0, stop: 0
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x1, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 2, flags: 0x0, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x0, stop: 0
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x1, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x0, stop: 0
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x1, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 2, flags: 0x0, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x0, stop: 0
omap_i2c 48072000.i2c: IRQ (ISR = 0x0010)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
omap_i2c 48072000.i2c: addr: 0x004b, len: 1, flags: 0x1, stop: 1
omap_i2c 48072000.i2c: IRQ (ISR = 0x0008)
omap_i2c 48072000.i2c: IRQ (ISR = 0x0004)
...



> And how it is possible to switch from ti,omap2430-i2c to ti,omap2420-i2c?
> They are so different IP, from the driver point of view. They have different
> data bus width.

Yes sorry, you're right. Downgrading it to ti,omap2420-i2c just
seems to quiet down the errors as the I2C then does not work
at all :)

Of course the issue I'm seeing could be caused by hung twl4030
PMIC too.

Regards,

Tony


> Alexander.
>
> [1]
> alexander@ubuntu:busses$ git log --pretty=oneline --reverse ca1f8da9ac5ce6e63d8f6933f83fabc1f3f961f4^..HEAD -- i2c-omap.c
> ca1f8da9ac5ce6e63d8f6933f83fabc1f3f961f4 i2c: remove FSF address
> 27caca9d2e01c92b26d0690f065aad093fea01c7 i2c: omap: fix NACK and Arbitration Lost irq handling
> 854a59425a0b9600ee974b113aae081c873163f6 i2c: omap: cleanup register definitions
> 903c3859f77f9b0aace551da03267ef7a211dbc4 i2c: omap: implement workaround for handling invalid BB-bit values
> 80cc361f14e8fa97119afa3324c2c913915e7252 i2c: omap: don't reset controller if Arbitration Lost detected
> 39370ab406933efdedb425910f0a36c16667c45f i2c: omap: add notes related to i2c multimaster mode
> ccfc866356674cb3a61829d239c685af6e85f197 i2c: omap: fix i207 errata handling
> 7d168dc7ed384e50bb7bff4920b73550fd2e9fcb Merge branch 'i2c/for-3.19' into i2c/for-next
> 2f769d173f0e6a2e85d75fe396f18f794fc4a615 omap: i2c: don't check bus state IP rev3.3 and earlier
> 2b6f66d87b44aaf1f34f071e6f6430c3ccaa8812 i2c: omap: fix buffer overruns during RX/TX data processing
> 30c52545106785405856c7e7e40b683b79c8084a i2c: omap: show that the reason of system lockup is an unhandled ISR event
>
>

2014-12-05 17:19:00

by Alexander Kochetkov

[permalink] [raw]
Subject: Re: [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing

Hello, Tony!

> The issue I'm seeing on 2430sdp is some earlier regression that
> has started happening over the past year or so.

I don't have 2430sdp. And I couldn't find 2430sdp schematics.
Looks, like this is NDA stuff.

> Of course the issue I'm seeing could be caused by hung twl4030
> PMIC too.

This could be switching twl4030 into inappropriate for 2430sdp mode.

For example, SMARTREFLEX_ENABLE bit is dangerous, because
it switch twl-pads from VMODE to i2c.
And when you set SMARTREFLEX_ENABLE, twl automatically drop
VDD1 and VDD2 to 1.2V (if VDD1_SR_CONTROL was not initialized through
BYPASS register early in the boot loader).

What mode is used in the 2430sdp? VMODE or Smartreflex?

And, that is my assumption, may be it wrong again.

I'd added trace in the twl_i2c_read_u8/twl_i2c_write_u8 functions to
easy find concrete line of code, what cause problem (by register address).

Hang happens on access to 0x004b address group.
That could be: BACKUP_REG, INT, PM_MASTER, PM_RECEIVER, RTC or
SECURED_REG. It hard to tell which one cause hang without trace.

Hope this helps.

Regards,
Alexander.