2012-06-13 08:30:17

by Sonic Zhang

[permalink] [raw]
Subject: [PATCH 1/8 v2] i2c: i2c-bfin-twi: Illegal i2c bus lock upon certain transfer scenarios.

From: Michael Hennerich <[email protected]>

For transfer counts > 255 bytes i2c-bfin-twi sets the data
transfer counter DCNT to 0xFF indicating unlimited transfers.
It then uses a flag iface->manual_stop to manually issue the STOP
condition, once the required amount of bytes are received.

We found that on I2C receive operation issuing the STOP condition
together with a FULL RCV FIFO (2bytes) will cause SDA and SCL be
constantly driven low.

Temporary workaround until further investigation:
Discard the RCV FIFO before issuing the STOP condition.

Signed-off-by: Michael Hennerich <[email protected]>
Signed-off-by: Sonic Zhang <[email protected]>
---
drivers/i2c/busses/i2c-bfin-twi.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index cdb59e5..33031f0 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -131,6 +131,10 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
iface->transPtr++;
iface->readNum--;
} else if (iface->manual_stop) {
+ /* Temporary workaround to avoid possible bus stall -
+ * Flush FIFO before issuing the STOP condition
+ */
+ read_RCV_DATA16(iface);
write_MASTER_CTL(iface,
read_MASTER_CTL(iface) | STOP);
} else if (iface->cur_mode == TWI_I2C_MODE_REPEAT &&
--
1.7.0.4


2012-06-13 08:28:15

by Sonic Zhang

[permalink] [raw]
Subject: [PATCH 5/8 v2] i2c:i2c-bfin-twi: TWI fails to restart next transfer in high system load.

From: Sonic Zhang <[email protected]>

Current driver was developed based on BF537 0.2 HRM. In high system load, BUFRDERR error
interrupt may be raised if XMTSERV interrupt of last TX byte is not served in time
(set RSTART bit), which breaks restart tranfer as expected.

"Buffer Read Error (BUFRDERR)" description in Blackfin HRM only applys to BF537
rev. < 0.3. In later rev. and later announced Blackfin chips, such as BF527 and
BF548, a new TWI master feature "Clock Stretching" is added into the TWI controller,
BUFRDERR interrupt is not triggered after TX FIFO is empty.

This patch sets RSTART bit at the beginning of the first transfer. The SCL and SDA
is hold till XMTSERV interrupt of last TX byte is served. Restart transfer is not broken
in high system load.

Signed-off-by: Sonic Zhang <[email protected]>
---
drivers/i2c/busses/i2c-bfin-twi.c | 23 +++++++++++++----------
1 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index 2e59bbd..e75ee91 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -99,7 +99,7 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
*/
else if (iface->cur_mode == TWI_I2C_MODE_COMBINED)
write_MASTER_CTL(iface,
- read_MASTER_CTL(iface) | MDIR | RSTART);
+ read_MASTER_CTL(iface) | MDIR);
else if (iface->manual_stop)
write_MASTER_CTL(iface,
read_MASTER_CTL(iface) | STOP);
@@ -107,10 +107,10 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
iface->cur_msg + 1 < iface->msg_num) {
if (iface->pmsg[iface->cur_msg + 1].flags & I2C_M_RD)
write_MASTER_CTL(iface,
- read_MASTER_CTL(iface) | RSTART | MDIR);
+ read_MASTER_CTL(iface) | MDIR);
else
write_MASTER_CTL(iface,
- (read_MASTER_CTL(iface) | RSTART) & ~MDIR);
+ read_MASTER_CTL(iface) & ~MDIR);
}
}
if (twi_int_status & RCVSERV) {
@@ -144,10 +144,10 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
iface->cur_msg + 1 < iface->msg_num) {
if (iface->pmsg[iface->cur_msg + 1].flags & I2C_M_RD)
write_MASTER_CTL(iface,
- read_MASTER_CTL(iface) | RSTART | MDIR);
+ read_MASTER_CTL(iface) | MDIR);
else
write_MASTER_CTL(iface,
- (read_MASTER_CTL(iface) | RSTART) & ~MDIR);
+ read_MASTER_CTL(iface) & ~MDIR);
}
}
}
@@ -262,9 +262,10 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
(0xff << 6)));
iface->manual_stop = 1;
}
- /* remove restart bit and enable master receive */
- write_MASTER_CTL(iface,
- read_MASTER_CTL(iface) & ~RSTART);
+ /* remove restart bit before last message */
+ if (iface->cur_msg+1 == iface->msg_num)
+ write_MASTER_CTL(iface,
+ read_MASTER_CTL(iface) & ~RSTART);
} else {
iface->result = 1;
write_INT_MASK(iface, 0);
@@ -321,7 +322,8 @@ static int bfin_twi_do_master_xfer(struct i2c_adapter *adap,
return -EINVAL;
}

- iface->cur_mode = TWI_I2C_MODE_REPEAT;
+ if (iface->msg_num > 1)
+ iface->cur_mode = TWI_I2C_MODE_REPEAT;
iface->manual_stop = 0;
iface->transPtr = pmsg->buf;
iface->writeNum = iface->readNum = pmsg->len;
@@ -366,6 +368,7 @@ static int bfin_twi_do_master_xfer(struct i2c_adapter *adap,

/* Master enable */
write_MASTER_CTL(iface, read_MASTER_CTL(iface) | MEN |
+ (iface->msg_num > 1 ? RSTART : 0) |
((iface->read_write == I2C_SMBUS_READ) ? MDIR : 0) |
((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ > 100) ? FAST : 0));
SSYNC();
@@ -530,7 +533,7 @@ int bfin_twi_do_smbus_xfer(struct i2c_adapter *adap, u16 addr,
else
write_MASTER_CTL(iface, 0x1 << 6);
/* Master enable */
- write_MASTER_CTL(iface, read_MASTER_CTL(iface) | MEN |
+ write_MASTER_CTL(iface, read_MASTER_CTL(iface) | MEN | RSTART |
((CONFIG_I2C_BLACKFIN_TWI_CLK_KHZ>100) ? FAST : 0));
break;
default:
--
1.7.0.4

2012-06-13 08:28:12

by Sonic Zhang

[permalink] [raw]
Subject: [PATCH 2/8 v2] i2c: i2c-bfin-twi: Improve the patch for bug "Illegal i2c bus lock upon certain transfer scenarios".

From: Sonic Zhang <[email protected]>

For transfer counts > 255 bytes i2c-bfin-twi sets the data
transfer counter DCNT to 0xFF indicating unlimited transfers.
It then uses a flag iface->manual_stop to manually issue the STOP
condition, once the required amount of bytes are received.

We found that on I2C receive operation issuing the STOP condition
together with a FULL RCV FIFO (2bytes) will cause SDA and SCL be
constantly driven low.

This patch stops receiving operation immediately in last rx interrupt.
This patch also wakes up waiting process when transfer completes.
Signed-off-by: Sonic Zhang <[email protected]>
---
drivers/i2c/busses/i2c-bfin-twi.c | 43 ++++++++++++++++++++----------------
1 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index 33031f0..dbe8fa0 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -130,21 +130,25 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
}
iface->transPtr++;
iface->readNum--;
- } else if (iface->manual_stop) {
- /* Temporary workaround to avoid possible bus stall -
- * Flush FIFO before issuing the STOP condition
- */
- read_RCV_DATA16(iface);
- write_MASTER_CTL(iface,
- read_MASTER_CTL(iface) | STOP);
- } else if (iface->cur_mode == TWI_I2C_MODE_REPEAT &&
- iface->cur_msg + 1 < iface->msg_num) {
- if (iface->pmsg[iface->cur_msg + 1].flags & I2C_M_RD)
- write_MASTER_CTL(iface,
- read_MASTER_CTL(iface) | RSTART | MDIR);
- else
+ }
+
+ if (iface->readNum == 0) {
+ if (iface->manual_stop) {
+ /* Temporary workaround to avoid possible bus stall -
+ * Flush FIFO before issuing the STOP condition
+ */
+ read_RCV_DATA16(iface);
write_MASTER_CTL(iface,
- (read_MASTER_CTL(iface) | RSTART) & ~MDIR);
+ read_MASTER_CTL(iface) | STOP);
+ } else if (iface->cur_mode == TWI_I2C_MODE_REPEAT &&
+ iface->cur_msg + 1 < iface->msg_num) {
+ if (iface->pmsg[iface->cur_msg + 1].flags & I2C_M_RD)
+ write_MASTER_CTL(iface,
+ read_MASTER_CTL(iface) | RSTART | MDIR);
+ else
+ write_MASTER_CTL(iface,
+ (read_MASTER_CTL(iface) | RSTART) & ~MDIR);
+ }
}
}
if (twi_int_status & MERR) {
@@ -245,12 +249,13 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
}
}

- if (iface->pmsg[iface->cur_msg].len <= 255)
- write_MASTER_CTL(iface,
+ if (iface->pmsg[iface->cur_msg].len <= 255) {
+ write_MASTER_CTL(iface,
(read_MASTER_CTL(iface) &
(~(0xff << 6))) |
- (iface->pmsg[iface->cur_msg].len << 6));
- else {
+ (iface->pmsg[iface->cur_msg].len << 6));
+ iface->manual_stop = 0;
+ } else {
write_MASTER_CTL(iface,
(read_MASTER_CTL(iface) |
(0xff << 6)));
@@ -264,8 +269,8 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
write_INT_MASK(iface, 0);
write_MASTER_CTL(iface, 0);
}
+ complete(&iface->complete);
}
- complete(&iface->complete);
}

/* Interrupt handler */
--
1.7.0.4

2012-06-13 08:29:40

by Sonic Zhang

[permalink] [raw]
Subject: [PATCH 6/8 v2] i2c:i2c-bfin-twi: include twi head file

From: Sonic Zhang <[email protected]>

TWI bit mask macros are moved to twi head file.
Depend on commit 61c16b5c7414b6d0511dc384e0ea994e250e6339

Signed-off-by: Sonic Zhang <[email protected]>
Signed-off-by: Bob Liu <[email protected]>
---
drivers/i2c/busses/i2c-bfin-twi.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index e75ee91..5be1dc2 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -25,6 +25,7 @@
#include <asm/blackfin.h>
#include <asm/portmux.h>
#include <asm/irq.h>
+#include <asm/bfin_twi.h>

/* SMBus mode*/
#define TWI_I2C_MODE_STANDARD 1
--
1.7.0.4

2012-06-13 08:30:23

by Sonic Zhang

[permalink] [raw]
Subject: [PATCH 8/8 v2] i2c: i2c-bfin-twi: Move blackfin TWI register access Macro to head file.

From: Sonic Zhang <[email protected]>

Depend on 1e92bf6d80b5a0a137455c96bf6cdd9c1a5b531e

Signed-off-by: Sonic Zhang <[email protected]>
---
drivers/i2c/busses/i2c-bfin-twi.c | 45 -------------------------------------
1 files changed, 0 insertions(+), 45 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index 5c57623..e1abf50 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -33,51 +33,6 @@
#define TWI_I2C_MODE_COMBINED 3
#define TWI_I2C_MODE_REPEAT 4

-struct bfin_twi_iface {
- int irq;
- spinlock_t lock;
- char read_write;
- u8 command;
- u8 *transPtr;
- int readNum;
- int writeNum;
- int cur_mode;
- int manual_stop;
- int result;
- struct i2c_adapter adap;
- struct completion complete;
- struct i2c_msg *pmsg;
- int msg_num;
- int cur_msg;
- u16 saved_clkdiv;
- u16 saved_control;
- void __iomem *regs_base;
-};
-
-
-#define DEFINE_TWI_REG(reg, off) \
-static inline u16 read_##reg(struct bfin_twi_iface *iface) \
- { return bfin_read16(iface->regs_base + (off)); } \
-static inline void write_##reg(struct bfin_twi_iface *iface, u16 v) \
- { bfin_write16(iface->regs_base + (off), v); }
-
-DEFINE_TWI_REG(CLKDIV, 0x00)
-DEFINE_TWI_REG(CONTROL, 0x04)
-DEFINE_TWI_REG(SLAVE_CTL, 0x08)
-DEFINE_TWI_REG(SLAVE_STAT, 0x0C)
-DEFINE_TWI_REG(SLAVE_ADDR, 0x10)
-DEFINE_TWI_REG(MASTER_CTL, 0x14)
-DEFINE_TWI_REG(MASTER_STAT, 0x18)
-DEFINE_TWI_REG(MASTER_ADDR, 0x1C)
-DEFINE_TWI_REG(INT_STAT, 0x20)
-DEFINE_TWI_REG(INT_MASK, 0x24)
-DEFINE_TWI_REG(FIFO_CTL, 0x28)
-DEFINE_TWI_REG(FIFO_STAT, 0x2C)
-DEFINE_TWI_REG(XMT_DATA8, 0x80)
-DEFINE_TWI_REG(XMT_DATA16, 0x84)
-DEFINE_TWI_REG(RCV_DATA8, 0x88)
-DEFINE_TWI_REG(RCV_DATA16, 0x8C)
-
static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
unsigned short twi_int_status)
{
--
1.7.0.4

2012-06-13 08:30:20

by Sonic Zhang

[permalink] [raw]
Subject: [PATCH 3/8 v2] i2c: i2c-bfin-twi: Break dead waiting loop if i2c device misbehaves.

From: Sonic Zhang <[email protected]>

Some fault i2c device may hold the sda/scl line and cause i2c driver
wait in the BUS busy loop. The I2C framework already retry the
transfer loop before timeout. Return -EAGAIN instead of pull BUSBUSY
in the other loop.

Signed-off-by: Sonic Zhang <[email protected]>
---
drivers/i2c/busses/i2c-bfin-twi.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index dbe8fa0..a5ab454 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -307,8 +307,8 @@ static int bfin_twi_do_master_xfer(struct i2c_adapter *adap,
if (!(read_CONTROL(iface) & TWI_ENA))
return -ENXIO;

- while (read_MASTER_STAT(iface) & BUSBUSY)
- yield();
+ if (read_MASTER_STAT(iface) & BUSBUSY)
+ return -EAGAIN;

iface->pmsg = msgs;
iface->msg_num = num;
@@ -407,8 +407,8 @@ int bfin_twi_do_smbus_xfer(struct i2c_adapter *adap, u16 addr,
if (!(read_CONTROL(iface) & TWI_ENA))
return -ENXIO;

- while (read_MASTER_STAT(iface) & BUSBUSY)
- yield();
+ if (read_MASTER_STAT(iface) & BUSBUSY)
+ return -EAGAIN;

iface->writeNum = 0;
iface->readNum = 0;
--
1.7.0.4

2012-06-13 08:30:14

by Sonic Zhang

[permalink] [raw]
Subject: [PATCH 4/8 v2] i2c: i2c-bfin-twi: Tighten condition when failing I2C transfer if MEN bit is reset unexpectedly.

From: Sonic Zhang <[email protected]>

In order to mark I2C transfer fail when MEN bit in I2C controller is reset unexpeced
in MCOMP interrupt, interrupt status bits XMTSERV or RCVSERV should be checked.

Master Transfer Complete (MCOMP).
[1] The initiated master transfer has completed. In the absence of a
repeat start, the bus has been released.
[0] The completion of a transfer has not been detected.

Signed-off-by: Sonic Zhang <[email protected]>
---
drivers/i2c/busses/i2c-bfin-twi.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index a5ab454..2e59bbd 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -201,7 +201,8 @@ static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
return;
}
if (twi_int_status & MCOMP) {
- if ((read_MASTER_CTL(iface) & MEN) == 0 &&
+ if (twi_int_status & (XMTSERV|RCVSERV) &&
+ (read_MASTER_CTL(iface) & MEN) == 0 &&
(iface->cur_mode == TWI_I2C_MODE_REPEAT ||
iface->cur_mode == TWI_I2C_MODE_COMBINED)) {
iface->result = -1;
--
1.7.0.4

2012-06-13 08:30:12

by Sonic Zhang

[permalink] [raw]
Subject: [PATCH 7/8 v2] i2c: i2c-bfin-twi: Move TWI peripheral pin request array to platform data.

From: Sonic Zhang <[email protected]>

Depend on commit cf93feb3a0dee97c7896016a352a3226139fbcf4

Signed-off-by: Sonic Zhang <[email protected]>
---
drivers/i2c/busses/i2c-bfin-twi.c | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-bfin-twi.c b/drivers/i2c/busses/i2c-bfin-twi.c
index 5be1dc2..5c57623 100644
--- a/drivers/i2c/busses/i2c-bfin-twi.c
+++ b/drivers/i2c/busses/i2c-bfin-twi.c
@@ -78,11 +78,6 @@ DEFINE_TWI_REG(XMT_DATA16, 0x84)
DEFINE_TWI_REG(RCV_DATA8, 0x88)
DEFINE_TWI_REG(RCV_DATA16, 0x8C)

-static const u16 pin_req[2][3] = {
- {P_TWI0_SCL, P_TWI0_SDA, 0},
- {P_TWI1_SCL, P_TWI1_SDA, 0},
-};
-
static void bfin_twi_handle_interrupt(struct bfin_twi_iface *iface,
unsigned short twi_int_status)
{
@@ -709,7 +704,8 @@ static int i2c_bfin_twi_probe(struct platform_device *pdev)
p_adap->timeout = 5 * HZ;
p_adap->retries = 3;

- rc = peripheral_request_list(pin_req[pdev->id], "i2c-bfin-twi");
+ rc = peripheral_request_list((unsigned short *)pdev->dev.platform_data,
+ "i2c-bfin-twi");
if (rc) {
dev_err(&pdev->dev, "Can't setup pin mux!\n");
goto out_error_pin_mux;
@@ -756,7 +752,7 @@ out_error_add_adapter:
free_irq(iface->irq, iface);
out_error_req_irq:
out_error_no_irq:
- peripheral_free_list(pin_req[pdev->id]);
+ peripheral_free_list((unsigned short *)pdev->dev.platform_data);
out_error_pin_mux:
iounmap(iface->regs_base);
out_error_ioremap:
@@ -774,7 +770,7 @@ static int i2c_bfin_twi_remove(struct platform_device *pdev)

i2c_del_adapter(&(iface->adap));
free_irq(iface->irq, iface);
- peripheral_free_list(pin_req[pdev->id]);
+ peripheral_free_list((unsigned short *)pdev->dev.platform_data);
iounmap(iface->regs_base);
kfree(iface);

--
1.7.0.4