Re: [PATCH 2/4] i2c: omap: implement workaround for handling invalid BB-bit values
Hi,
On Fri, Nov 21, 2014 at 01:28:43AM +0400, Alexander Kochetkov wrote:
In a multimaster environment, after IP software reset, BB-bit value doesn't
correspond to the current bus state. It may happen what BB-bit will be 0,
while the bus is busy due to another I2C master activity.
Any transfer started when BB=0 and bus is busy wouldn't be completed by IP
and results in controller timeout. More over, in some cases IP could
interrupt another master's transfer and corrupt data on wire.
The commit implement method allowing to prevent IP from entering into
controller timeout state and from data corruption state.
The one drawback is the need to wait for 10ms before the first transfer.
Tested on Beagleboard XM C.
Signed-off-by: Alexander Kochetkov al.koc...@gmail.com
we have a report from Kevin Hilman that this commit breaks OMAP3530, see
[1]
[1]
http://storage.armcloud.us/kernel-ci/next/next-20141124/arm-omap2plus_defconfig/boot-omap3-overo-tobi.log
---
drivers/i2c/busses/i2c-omap.c | 103
+
1 file changed, 103 insertions(+)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index a021733..3ffb9c0 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -58,6 +58,9 @@
/* timeout for pm runtime autosuspend */
#define OMAP_I2C_PM_TIMEOUT 1000/* ms */
+/* timeout for making decision on bus free status */
+#define OMAP_I2C_BUS_FREE_TIMEOUT (msecs_to_jiffies(10))
+
/* For OMAP3 I2C_IV has changed to I2C_WE (wakeup enable) */
enum {
OMAP_I2C_REV_REG = 0,
@@ -210,6 +213,9 @@ struct omap_i2c_dev {
*/
u32 rev;
unsignedb_hw:1; /* bad h/w fixes */
+ unsignedbb_valid:1; /* true when BB-bit reflects
+ * the I2C bus state
+ */
unsignedreceiver:1; /* true when we're in receiver
mode */
u16 iestate;/* Saved interrupt register */
u16 pscstate;
@@ -336,7 +342,10 @@ static int omap_i2c_reset(struct omap_i2c_dev *dev)
/* SYSC register is cleared by the reset; rewrite it */
omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, sysc);
+ /* Schedule I2C-bus monitoring on the next transfer */
+ dev-bb_valid = 0;
}
+
return 0;
}
@@ -449,6 +458,11 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
dev-scllstate = scll;
dev-sclhstate = sclh;
+ if (dev-rev OMAP_I2C_OMAP1_REV_2) {
+ /* Not implemented */
+ dev-bb_valid = 1;
+ }
+
__omap_i2c_init(dev);
return 0;
@@ -473,6 +487,91 @@ static int omap_i2c_wait_for_bb(struct omap_i2c_dev *dev)
return 0;
}
+/*
+ * Wait while BB-bit doesn't reflect the I2C bus state
+ *
+ * In a multimaster environment, after IP software reset, BB-bit value
doesn't
+ * correspond to the current bus state. It may happen what BB-bit will be 0,
+ * while the bus is busy due to another I2C master activity.
+ * Here are BB-bit values after reset:
+ * SDA SCL BB NOTES
+ * 0 00 1, 2
+ * 1 00 1, 2
+ * 0 11
+ * 1 10 3
+ * Later, if IP detect SDA=0 and SCL=1 (ACK) or SDA 1-0 while SCL=1 (START)
+ * combinations on the bus, it set BB-bit to 1.
+ * If IP detect SDA 0-1 while SCL=1 (STOP) combination on the bus,
+ * it set BB-bit to 0 and BF to 1.
+ * BB and BF bits correctly tracks the bus state while IP is suspended
+ * BB bit became valid on the next FCLK clock after CON_EN bit set
+ *
+ * NOTES:
+ * 1. Any transfer started when BB=0 and bus is busy wouldn't be
+ *completed by IP and results in controller timeout.
+ * 2. Any transfer started when BB=0 and SCL=0 results in IP
+ *starting to drive SDA low. In that case IP corrupt data
+ *on the bus.
+ * 3. Any transfer started in the middle of another master's transfer
+ *results in unpredictable results and data corruption
+ */
+static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *dev)
+{
+ unsigned long bus_free_timeout = 0;
+ unsigned long timeout;
+ int bus_free = 0;
+ u16 stat, systest;
+
+ if (dev-bb_valid)
+ return 0;
+
+ timeout = jiffies + OMAP_I2C_TIMEOUT;
+ while (1) {
+ stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
+ /*
+ * We will see BB or BF event in a case IP had detected any
+ * activity on the I2C bus. Now IP correctly tracks the bus
+ * state. BB-bit value is valid.
+ */
+ if (stat (OMAP_I2C_STAT_BB | OMAP_I2C_STAT_BF))
+ break;
+
+
Re: [PATCH 2/4] i2c: omap: implement workaround for handling invalid BB-bit values
On Sun, Nov 23, 2014 at 04:18:40PM +0300, Alexander Kochetkov wrote: 23 нояб. 2014 г., в 7:43, Felipe Balbi ba...@ti.com написал(а): maybe there was a typo? I tested on v3.18-rc3 :-) I do my tests on kernel from angstrom with almost all i2c-omap patches backported from linux/master. Then I rebased them to wrong (old) kernel version and posted to the list. Angstrom kernel is from meta-ti layer. It's the same kernel as for arago. http://arago-project.org/wiki/index.php/Main_Page I would really like to switch to the recent kernel, but I don't know how good codec engine (CE) is supported on it. Initially all ti drivers for codec engine was done for 2.6.x kernels and later some of them was ported for 3.2.x. Felipe, do you know how CE is supported on v3.18-rc3? what's CE ? -- balbi signature.asc Description: Digital signature
Re: [PATCH 2/4] i2c: omap: implement workaround for handling invalid BB-bit values
23 нояб. 2014 г., в 7:43, Felipe Balbi ba...@ti.com написал(а): maybe there was a typo? I tested on v3.18-rc3 :-) I do my tests on kernel from angstrom with almost all i2c-omap patches backported from linux/master. Then I rebased them to wrong (old) kernel version and posted to the list. Angstrom kernel is from meta-ti layer. It's the same kernel as for arago. http://arago-project.org/wiki/index.php/Main_Page I would really like to switch to the recent kernel, but I don't know how good codec engine (CE) is supported on it. Initially all ti drivers for codec engine was done for 2.6.x kernels and later some of them was ported for 3.2.x. Felipe, do you know how CE is supported on v3.18-rc3? Regards, Alexander. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] i2c: omap: implement workaround for handling invalid BB-bit values
On Fri, Nov 21, 2014 at 10:08:08AM -0600, Felipe Balbi wrote: On Fri, Nov 21, 2014 at 01:28:43AM +0400, Alexander Kochetkov wrote: In a multimaster environment, after IP software reset, BB-bit value doesn't correspond to the current bus state. It may happen what BB-bit will be 0, while the bus is busy due to another I2C master activity. Any transfer started when BB=0 and bus is busy wouldn't be completed by IP and results in controller timeout. More over, in some cases IP could interrupt another master's transfer and corrupt data on wire. The commit implement method allowing to prevent IP from entering into controller timeout state and from data corruption state. The one drawback is the need to wait for 10ms before the first transfer. Tested on Beagleboard XM C. Signed-off-by: Alexander Kochetkov al.koc...@gmail.com Tested on BBB and AM437x Starter Kit Tested-by: Felipe Balbi ba...@ti.com Reviewed-by: Felipe Balbi ba...@ti.com Huh, I can't apply this one? Which kernel version is this based on? signature.asc Description: Digital signature
Re: [PATCH 2/4] i2c: omap: implement workaround for handling invalid BB-bit values
22 нояб. 2014 г., в 16:23, Wolfram Sang w...@the-dreams.de написал(а): Huh, I can't apply this one? Which kernel version is this based on? v.3.13-rc8 It depends on PATCH 1/4 alexander@ubuntu:git.kernel.org.pub.scm.linux.kernel.git.stable.linux-stable$ git log --pretty=oneline --abbrev-commit -6 204c058 i2c: omap: add notes related to i2c multimaster mode 2b15417 i2c: omap: don't reset controller if Arbitration Lost detected 38ef30e i2c: omap: implement workaround for handling invalid BB-bit values 9b15135 i2c: omap: cleanup register definitions a7483d8 Fixes: 1d7afc9 i2c: omap: ack IRQ in parts 7e22e91 Linux 3.13-rc8 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] i2c: omap: implement workaround for handling invalid BB-bit values
On Sat, Nov 22, 2014 at 05:06:10PM +0300, Alexander Kochetkov wrote: 22 нояб. 2014 г., в 16:23, Wolfram Sang w...@the-dreams.de написал(а): Huh, I can't apply this one? Which kernel version is this based on? v.3.13-rc8 Wow, that's old. Can you please rebase the series on top of 3.18-rc4? Or my i2c/for-next? Or at the very least 3.17? @Felipe: With which kernel did you test the series? Also, 3.13? signature.asc Description: Digital signature
Re: [PATCH 2/4] i2c: omap: implement workaround for handling invalid BB-bit values
On Sat, Nov 22, 2014 at 07:02:22PM +0100, Wolfram Sang wrote: On Sat, Nov 22, 2014 at 05:06:10PM +0300, Alexander Kochetkov wrote: 22 нояб. 2014 г., в 16:23, Wolfram Sang w...@the-dreams.de написал(а): Huh, I can't apply this one? Which kernel version is this based on? v.3.13-rc8 Wow, that's old. Can you please rebase the series on top of 3.18-rc4? Or my i2c/for-next? Or at the very least 3.17? @Felipe: With which kernel did you test the series? Also, 3.13? maybe there was a typo? I tested on v3.18-rc3 :-) -- balbi signature.asc Description: Digital signature
Re: [PATCH 2/4] i2c: omap: implement workaround for handling invalid BB-bit values
On Fri, Nov 21, 2014 at 01:28:43AM +0400, Alexander Kochetkov wrote:
In a multimaster environment, after IP software reset, BB-bit value doesn't
correspond to the current bus state. It may happen what BB-bit will be 0,
while the bus is busy due to another I2C master activity.
Any transfer started when BB=0 and bus is busy wouldn't be completed by IP
and results in controller timeout. More over, in some cases IP could
interrupt another master's transfer and corrupt data on wire.
The commit implement method allowing to prevent IP from entering into
controller timeout state and from data corruption state.
The one drawback is the need to wait for 10ms before the first transfer.
Tested on Beagleboard XM C.
Signed-off-by: Alexander Kochetkov al.koc...@gmail.com
Tested on BBB and AM437x Starter Kit
Tested-by: Felipe Balbi ba...@ti.com
Reviewed-by: Felipe Balbi ba...@ti.com
---
drivers/i2c/busses/i2c-omap.c | 103
+
1 file changed, 103 insertions(+)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index a021733..3ffb9c0 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -58,6 +58,9 @@
/* timeout for pm runtime autosuspend */
#define OMAP_I2C_PM_TIMEOUT 1000/* ms */
+/* timeout for making decision on bus free status */
+#define OMAP_I2C_BUS_FREE_TIMEOUT (msecs_to_jiffies(10))
+
/* For OMAP3 I2C_IV has changed to I2C_WE (wakeup enable) */
enum {
OMAP_I2C_REV_REG = 0,
@@ -210,6 +213,9 @@ struct omap_i2c_dev {
*/
u32 rev;
unsignedb_hw:1; /* bad h/w fixes */
+ unsignedbb_valid:1; /* true when BB-bit reflects
+ * the I2C bus state
+ */
unsignedreceiver:1; /* true when we're in receiver
mode */
u16 iestate;/* Saved interrupt register */
u16 pscstate;
@@ -336,7 +342,10 @@ static int omap_i2c_reset(struct omap_i2c_dev *dev)
/* SYSC register is cleared by the reset; rewrite it */
omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, sysc);
+ /* Schedule I2C-bus monitoring on the next transfer */
+ dev-bb_valid = 0;
}
+
return 0;
}
@@ -449,6 +458,11 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
dev-scllstate = scll;
dev-sclhstate = sclh;
+ if (dev-rev OMAP_I2C_OMAP1_REV_2) {
+ /* Not implemented */
+ dev-bb_valid = 1;
+ }
+
__omap_i2c_init(dev);
return 0;
@@ -473,6 +487,91 @@ static int omap_i2c_wait_for_bb(struct omap_i2c_dev *dev)
return 0;
}
+/*
+ * Wait while BB-bit doesn't reflect the I2C bus state
+ *
+ * In a multimaster environment, after IP software reset, BB-bit value
doesn't
+ * correspond to the current bus state. It may happen what BB-bit will be 0,
+ * while the bus is busy due to another I2C master activity.
+ * Here are BB-bit values after reset:
+ * SDA SCL BB NOTES
+ * 0 00 1, 2
+ * 1 00 1, 2
+ * 0 11
+ * 1 10 3
+ * Later, if IP detect SDA=0 and SCL=1 (ACK) or SDA 1-0 while SCL=1 (START)
+ * combinations on the bus, it set BB-bit to 1.
+ * If IP detect SDA 0-1 while SCL=1 (STOP) combination on the bus,
+ * it set BB-bit to 0 and BF to 1.
+ * BB and BF bits correctly tracks the bus state while IP is suspended
+ * BB bit became valid on the next FCLK clock after CON_EN bit set
+ *
+ * NOTES:
+ * 1. Any transfer started when BB=0 and bus is busy wouldn't be
+ *completed by IP and results in controller timeout.
+ * 2. Any transfer started when BB=0 and SCL=0 results in IP
+ *starting to drive SDA low. In that case IP corrupt data
+ *on the bus.
+ * 3. Any transfer started in the middle of another master's transfer
+ *results in unpredictable results and data corruption
+ */
+static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *dev)
+{
+ unsigned long bus_free_timeout = 0;
+ unsigned long timeout;
+ int bus_free = 0;
+ u16 stat, systest;
+
+ if (dev-bb_valid)
+ return 0;
+
+ timeout = jiffies + OMAP_I2C_TIMEOUT;
+ while (1) {
+ stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
+ /*
+ * We will see BB or BF event in a case IP had detected any
+ * activity on the I2C bus. Now IP correctly tracks the bus
+ * state. BB-bit value is valid.
+ */
+ if (stat (OMAP_I2C_STAT_BB | OMAP_I2C_STAT_BF))
+ break;
+
+ /*
+ * Otherwise, we must look signals on the bus to make
+
[PATCH 2/4] i2c: omap: implement workaround for handling invalid BB-bit values
In a multimaster environment, after IP software reset, BB-bit value doesn't
correspond to the current bus state. It may happen what BB-bit will be 0,
while the bus is busy due to another I2C master activity.
Any transfer started when BB=0 and bus is busy wouldn't be completed by IP
and results in controller timeout. More over, in some cases IP could
interrupt another master's transfer and corrupt data on wire.
The commit implement method allowing to prevent IP from entering into
controller timeout state and from data corruption state.
The one drawback is the need to wait for 10ms before the first transfer.
Tested on Beagleboard XM C.
Signed-off-by: Alexander Kochetkov al.koc...@gmail.com
---
drivers/i2c/busses/i2c-omap.c | 103 +
1 file changed, 103 insertions(+)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index a021733..3ffb9c0 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -58,6 +58,9 @@
/* timeout for pm runtime autosuspend */
#define OMAP_I2C_PM_TIMEOUT1000/* ms */
+/* timeout for making decision on bus free status */
+#define OMAP_I2C_BUS_FREE_TIMEOUT (msecs_to_jiffies(10))
+
/* For OMAP3 I2C_IV has changed to I2C_WE (wakeup enable) */
enum {
OMAP_I2C_REV_REG = 0,
@@ -210,6 +213,9 @@ struct omap_i2c_dev {
*/
u32 rev;
unsignedb_hw:1; /* bad h/w fixes */
+ unsignedbb_valid:1; /* true when BB-bit reflects
+* the I2C bus state
+*/
unsignedreceiver:1; /* true when we're in receiver
mode */
u16 iestate;/* Saved interrupt register */
u16 pscstate;
@@ -336,7 +342,10 @@ static int omap_i2c_reset(struct omap_i2c_dev *dev)
/* SYSC register is cleared by the reset; rewrite it */
omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, sysc);
+ /* Schedule I2C-bus monitoring on the next transfer */
+ dev-bb_valid = 0;
}
+
return 0;
}
@@ -449,6 +458,11 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
dev-scllstate = scll;
dev-sclhstate = sclh;
+ if (dev-rev OMAP_I2C_OMAP1_REV_2) {
+ /* Not implemented */
+ dev-bb_valid = 1;
+ }
+
__omap_i2c_init(dev);
return 0;
@@ -473,6 +487,91 @@ static int omap_i2c_wait_for_bb(struct omap_i2c_dev *dev)
return 0;
}
+/*
+ * Wait while BB-bit doesn't reflect the I2C bus state
+ *
+ * In a multimaster environment, after IP software reset, BB-bit value doesn't
+ * correspond to the current bus state. It may happen what BB-bit will be 0,
+ * while the bus is busy due to another I2C master activity.
+ * Here are BB-bit values after reset:
+ * SDA SCL BB NOTES
+ * 0 00 1, 2
+ * 1 00 1, 2
+ * 0 11
+ * 1 10 3
+ * Later, if IP detect SDA=0 and SCL=1 (ACK) or SDA 1-0 while SCL=1 (START)
+ * combinations on the bus, it set BB-bit to 1.
+ * If IP detect SDA 0-1 while SCL=1 (STOP) combination on the bus,
+ * it set BB-bit to 0 and BF to 1.
+ * BB and BF bits correctly tracks the bus state while IP is suspended
+ * BB bit became valid on the next FCLK clock after CON_EN bit set
+ *
+ * NOTES:
+ * 1. Any transfer started when BB=0 and bus is busy wouldn't be
+ *completed by IP and results in controller timeout.
+ * 2. Any transfer started when BB=0 and SCL=0 results in IP
+ *starting to drive SDA low. In that case IP corrupt data
+ *on the bus.
+ * 3. Any transfer started in the middle of another master's transfer
+ *results in unpredictable results and data corruption
+ */
+static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *dev)
+{
+ unsigned long bus_free_timeout = 0;
+ unsigned long timeout;
+ int bus_free = 0;
+ u16 stat, systest;
+
+ if (dev-bb_valid)
+ return 0;
+
+ timeout = jiffies + OMAP_I2C_TIMEOUT;
+ while (1) {
+ stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
+ /*
+* We will see BB or BF event in a case IP had detected any
+* activity on the I2C bus. Now IP correctly tracks the bus
+* state. BB-bit value is valid.
+*/
+ if (stat (OMAP_I2C_STAT_BB | OMAP_I2C_STAT_BF))
+ break;
+
+ /*
+* Otherwise, we must look signals on the bus to make
+* the right decision.
+*/
+ systest = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+ if ((systest OMAP_I2C_SYSTEST_SCL_I_FUNC)
+ (systest
