* [RFC 0/2] i2c: omap: new fixes for driver
@ 2014-11-29 21:00 Alexander Kochetkov
2014-11-29 21:00 ` [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing Alexander Kochetkov
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Alexander Kochetkov @ 2014-11-29 21:00 UTC (permalink / raw)
To: Kevin Hilman, Tony Lindgren, Felipe Balbi, Wolfram Sang,
Alexander Kochetkov
Cc: linux-omap, linux-i2c, linux-kernel
The first patch fix i2c-omap driver for omap2420, found by code review and
later reported by Tony Lindgren <tony@atomide.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread* [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing 2014-11-29 21:00 [RFC 0/2] i2c: omap: new fixes for driver Alexander Kochetkov @ 2014-11-29 21:00 ` Alexander Kochetkov 2014-12-01 19:58 ` Tony Lindgren 2014-11-29 21:00 ` [RFC 2/2] i2c: omap: show that the reason of system lockup is an unhandled ISR event Alexander Kochetkov ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Alexander Kochetkov @ 2014-11-29 21:00 UTC (permalink / raw) To: Kevin Hilman, Tony Lindgren, Felipe Balbi, Wolfram Sang, Alexander Kochetkov Cc: linux-omap, linux-i2c, linux-kernel 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 <al.kochet@gmail.com> Fixes: dd74548ddece4b9d68e5528287a272fa552c81d0 "i2c: omap: resize fifos before each message" Reported-by: Tony Lindgren <tony@atomide.com> --- 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing 2014-11-29 21:00 ` [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing Alexander Kochetkov @ 2014-12-01 19:58 ` Tony Lindgren 2014-12-02 11:17 ` Alexander Kochetkov 0 siblings, 1 reply; 10+ messages in thread From: Tony Lindgren @ 2014-12-01 19:58 UTC (permalink / raw) To: Alexander Kochetkov Cc: Kevin Hilman, Felipe Balbi, Wolfram Sang, linux-omap, linux-i2c, linux-kernel * Alexander Kochetkov <al.kochet@gmail.com> [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 <al.kochet@gmail.com> > Fixes: dd74548ddece4b9d68e5528287a272fa552c81d0 "i2c: omap: resize fifos before each message" > Reported-by: Tony Lindgren <tony@atomide.com> 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 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing 2014-12-01 19:58 ` Tony Lindgren @ 2014-12-02 11:17 ` Alexander Kochetkov 2014-12-04 18:09 ` Tony Lindgren 0 siblings, 1 reply; 10+ messages in thread From: Alexander Kochetkov @ 2014-12-02 11:17 UTC (permalink / raw) To: Tony Lindgren Cc: Kevin Hilman, Felipe Balbi, Wolfram Sang, linux-omap, linux-i2c, linux-kernel 01 дек. 2014 г., в 22:58, Tony Lindgren <tony@atomide.com> написал(а): > 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing 2014-12-02 11:17 ` Alexander Kochetkov @ 2014-12-04 18:09 ` Tony Lindgren 2014-12-05 17:18 ` Alexander Kochetkov 0 siblings, 1 reply; 10+ messages in thread From: Tony Lindgren @ 2014-12-04 18:09 UTC (permalink / raw) To: Alexander Kochetkov Cc: Kevin Hilman, Felipe Balbi, Wolfram Sang, linux-omap, linux-i2c, linux-kernel * Alexander Kochetkov <al.kochet@gmail.com> [141202 03:19]: > > 01 дек. 2014 г., в 22:58, Tony Lindgren <tony@atomide.com> написал(а): > > > 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 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing 2014-12-04 18:09 ` Tony Lindgren @ 2014-12-05 17:18 ` Alexander Kochetkov 0 siblings, 0 replies; 10+ messages in thread From: Alexander Kochetkov @ 2014-12-05 17:18 UTC (permalink / raw) To: Tony Lindgren Cc: Kevin Hilman, Felipe Balbi, Wolfram Sang, linux-omap, linux-i2c, linux-kernel 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC 2/2] i2c: omap: show that the reason of system lockup is an unhandled ISR event 2014-11-29 21:00 [RFC 0/2] i2c: omap: new fixes for driver Alexander Kochetkov 2014-11-29 21:00 ` [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing Alexander Kochetkov @ 2014-11-29 21:00 ` Alexander Kochetkov 2014-12-01 20:04 ` [RFC 0/2] i2c: omap: new fixes for driver Kevin Hilman 2015-01-13 10:04 ` Wolfram Sang 3 siblings, 0 replies; 10+ messages in thread From: Alexander Kochetkov @ 2014-11-29 21:00 UTC (permalink / raw) To: Kevin Hilman, Tony Lindgren, Felipe Balbi, Wolfram Sang, Alexander Kochetkov Cc: linux-omap, linux-i2c, linux-kernel 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 <al.kochet@gmail.com> 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC 0/2] i2c: omap: new fixes for driver 2014-11-29 21:00 [RFC 0/2] i2c: omap: new fixes for driver Alexander Kochetkov 2014-11-29 21:00 ` [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing Alexander Kochetkov 2014-11-29 21:00 ` [RFC 2/2] i2c: omap: show that the reason of system lockup is an unhandled ISR event Alexander Kochetkov @ 2014-12-01 20:04 ` Kevin Hilman 2014-12-02 11:21 ` Alexander Kochetkov 2015-01-13 10:04 ` Wolfram Sang 3 siblings, 1 reply; 10+ messages in thread From: Kevin Hilman @ 2014-12-01 20:04 UTC (permalink / raw) To: Alexander Kochetkov Cc: Tony Lindgren, Felipe Balbi, Wolfram Sang, linux-omap, linux-i2c, linux-kernel Alexander Kochetkov <al.kochet@gmail.com> writes: > The first patch fix i2c-omap driver for omap2420, found by code review and > later reported by Tony Lindgren <tony@atomide.com> 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 0/2] i2c: omap: new fixes for driver 2014-12-01 20:04 ` [RFC 0/2] i2c: omap: new fixes for driver Kevin Hilman @ 2014-12-02 11:21 ` Alexander Kochetkov 0 siblings, 0 replies; 10+ messages in thread From: Alexander Kochetkov @ 2014-12-02 11:21 UTC (permalink / raw) To: Kevin Hilman Cc: Tony Lindgren, Felipe Balbi, Wolfram Sang, linux-omap, linux-i2c, linux-kernel 01 дек. 2014 г., в 23:04, Kevin Hilman <khilman@kernel.org> написал(а): > 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 0/2] i2c: omap: new fixes for driver 2014-11-29 21:00 [RFC 0/2] i2c: omap: new fixes for driver Alexander Kochetkov ` (2 preceding siblings ...) 2014-12-01 20:04 ` [RFC 0/2] i2c: omap: new fixes for driver Kevin Hilman @ 2015-01-13 10:04 ` Wolfram Sang 3 siblings, 0 replies; 10+ messages in thread From: Wolfram Sang @ 2015-01-13 10:04 UTC (permalink / raw) To: Alexander Kochetkov Cc: Kevin Hilman, Tony Lindgren, Felipe Balbi, linux-omap, linux-i2c, linux-kernel [-- Attachment #1: Type: text/plain, Size: 847 bytes --] On Sun, Nov 30, 2014 at 01:00:01AM +0400, Alexander Kochetkov wrote: > The first patch fix i2c-omap driver for omap2420, found by code review and > later reported by Tony Lindgren <tony@atomide.com> 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. 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 --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-01-13 10:04 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-29 21:00 [RFC 0/2] i2c: omap: new fixes for driver Alexander Kochetkov 2014-11-29 21:00 ` [RFC 1/2] i2c: omap: fix buffer overruns during RX/TX data processing Alexander Kochetkov 2014-12-01 19:58 ` Tony Lindgren 2014-12-02 11:17 ` Alexander Kochetkov 2014-12-04 18:09 ` Tony Lindgren 2014-12-05 17:18 ` Alexander Kochetkov 2014-11-29 21:00 ` [RFC 2/2] i2c: omap: show that the reason of system lockup is an unhandled ISR event Alexander Kochetkov 2014-12-01 20:04 ` [RFC 0/2] i2c: omap: new fixes for driver Kevin Hilman 2014-12-02 11:21 ` Alexander Kochetkov 2015-01-13 10:04 ` Wolfram Sang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox