[armel/orion5x] I2C: mv64xxx: fix race between FSM/interrupt and process context (Closes: #622325)

svn path=/dists/sid/linux/; revision=20495
This commit is contained in:
Ben Hutchings 2013-08-13 18:36:33 +00:00
parent c16ce90abc
commit cc3748ad46
5 changed files with 290 additions and 0 deletions

2
debian/changelog vendored
View File

@ -2,6 +2,8 @@ linux (3.10.5-2) UNRELEASED; urgency=low
[ Ben Hutchings ]
* [x86] Enable ASUS_OLED as module (Closes: #680016)
* [armel/orion5x] I2C: mv64xxx: fix race between FSM/interrupt and process
context (Closes: #622325)
[ Thorsten Glaser ]
* [m68k] debian/patches/bugfix/m68k/atari-irqs.patch: patch from mailing list

View File

@ -0,0 +1,57 @@
From: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Thu, 16 May 2013 21:37:11 +0100
Subject: [1/3] I2C: mv64xxx: remove I2C_M_NOSTART code
Origin: https://git.kernel.org/linus/aa6bce5319a54c050af26e095287472854abccfd
Bug-Debian: http://bugs.debian.org/622325
As this driver does not advertise protocol mangling support
(I2C_FUNC_PROTOCOL_MANGLING is not set), having code to act on
I2C_M_NOSTART is illogical, and in any case isn't supportable on
anything but the first message - which makes no sense. Remove
the I2C_M_NOSTART code.
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: Mark A. Greer <mgreer@animalcreek.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
drivers/i2c/busses/i2c-mv64xxx.c | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index 4b45c73..f11cb25 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -419,28 +419,12 @@ mv64xxx_i2c_execute_msg(struct mv64xxx_i2c_data *drv_data, struct i2c_msg *msg,
spin_lock_irqsave(&drv_data->lock, flags);
mv64xxx_i2c_prepare_for_io(drv_data, msg);
- if (unlikely(msg->flags & I2C_M_NOSTART)) { /* Skip start/addr phases */
- if (drv_data->msg->flags & I2C_M_RD) {
- /* No action to do, wait for slave to send a byte */
- drv_data->action = MV64XXX_I2C_ACTION_CONTINUE;
- drv_data->state =
- MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_DATA;
- } else {
- drv_data->action = MV64XXX_I2C_ACTION_SEND_DATA;
- drv_data->state =
- MV64XXX_I2C_STATE_WAITING_FOR_SLAVE_ACK;
- drv_data->bytes_left--;
- }
+ if (is_first) {
+ drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
+ drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_START_COND;
} else {
- if (is_first) {
- drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
- drv_data->state =
- MV64XXX_I2C_STATE_WAITING_FOR_START_COND;
- } else {
- drv_data->action = MV64XXX_I2C_ACTION_SEND_ADDR_1;
- drv_data->state =
- MV64XXX_I2C_STATE_WAITING_FOR_ADDR_1_ACK;
- }
+ drv_data->action = MV64XXX_I2C_ACTION_SEND_ADDR_1;
+ drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_ADDR_1_ACK;
}
drv_data->send_stop = is_last;

View File

@ -0,0 +1,141 @@
From: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Thu, 16 May 2013 21:39:12 +0100
Subject: [3/3] I2C: mv64xxx: fix race between FSM/interrupt and process
context
Origin: https://git.kernel.org/linus/4243fa0bad551b8c8d4ff7104e8fd557ae848845
Bug-Debian: http://bugs.debian.org/622325
Asking for a multi-part message to be handled by this driver is racy; it
has been observed that the following sequence is possible with this
driver:
- send start
- send address + write
- send data
- send (repeated) start
- send address + write
- send (repeated) start
- send address + read
- unrecoverable bus hang (except by system reset)
The problem is that the interrupt handling sees the next event after the
first repeated start is sent - the IFLG bit is set in the register even
though INTEN is disabled.
Let's fix this by moving all of the message processing into interrupt
context, rather than having it partly in IRQ and partly in process
context. This allows us to move immediately to the next message in the
interrupt handler and get on with the transfer, rather than incuring a
couple of scheduling switches to get the next message.
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: Mark A. Greer <mgreer@animalcreek.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
drivers/i2c/busses/i2c-mv64xxx.c | 54 +++++++++++++++++++++++++---------------
1 file changed, 34 insertions(+), 20 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index bb37e14..6356439 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -86,6 +86,8 @@ enum {
};
struct mv64xxx_i2c_data {
+ struct i2c_msg *msgs;
+ int num_msgs;
int irq;
u32 state;
u32 action;
@@ -194,7 +196,7 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status)
if ((drv_data->bytes_left == 0)
|| (drv_data->aborting
&& (drv_data->byte_posn != 0))) {
- if (drv_data->send_stop) {
+ if (drv_data->send_stop || drv_data->aborting) {
drv_data->action = MV64XXX_I2C_ACTION_SEND_STOP;
drv_data->state = MV64XXX_I2C_STATE_IDLE;
} else {
@@ -271,12 +273,25 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
{
switch(drv_data->action) {
case MV64XXX_I2C_ACTION_SEND_RESTART:
+ /* We should only get here if we have further messages */
+ BUG_ON(drv_data->num_msgs == 0);
+
drv_data->cntl_bits |= MV64XXX_I2C_REG_CONTROL_START;
- drv_data->cntl_bits &= ~MV64XXX_I2C_REG_CONTROL_INTEN;
writel(drv_data->cntl_bits,
drv_data->reg_base + MV64XXX_I2C_REG_CONTROL);
- drv_data->block = 0;
- wake_up(&drv_data->waitq);
+
+ drv_data->msgs++;
+ drv_data->num_msgs--;
+
+ /* Setup for the next message */
+ mv64xxx_i2c_prepare_for_io(drv_data, drv_data->msgs);
+
+ /*
+ * We're never at the start of the message here, and by this
+ * time it's already too late to do any protocol mangling.
+ * Thankfully, do not advertise support for that feature.
+ */
+ drv_data->send_stop = drv_data->num_msgs == 1;
break;
case MV64XXX_I2C_ACTION_CONTINUE:
@@ -412,20 +427,15 @@ mv64xxx_i2c_wait_for_completion(struct mv64xxx_i2c_data *drv_data)
static int
mv64xxx_i2c_execute_msg(struct mv64xxx_i2c_data *drv_data, struct i2c_msg *msg,
- int is_first, int is_last)
+ int is_last)
{
unsigned long flags;
spin_lock_irqsave(&drv_data->lock, flags);
mv64xxx_i2c_prepare_for_io(drv_data, msg);
- if (is_first) {
- drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
- drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_START_COND;
- } else {
- drv_data->action = MV64XXX_I2C_ACTION_SEND_ADDR_1;
- drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_ADDR_1_ACK;
- }
+ drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
+ drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_START_COND;
drv_data->send_stop = is_last;
drv_data->block = 1;
@@ -453,16 +463,20 @@ static int
mv64xxx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
{
struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap);
- int i, rc;
+ int rc, ret = num;
- for (i = 0; i < num; i++) {
- rc = mv64xxx_i2c_execute_msg(drv_data, &msgs[i],
- i == 0, i + 1 == num);
- if (rc < 0)
- return rc;
- }
+ BUG_ON(drv_data->msgs != NULL);
+ drv_data->msgs = msgs;
+ drv_data->num_msgs = num;
+
+ rc = mv64xxx_i2c_execute_msg(drv_data, &msgs[0], num == 1);
+ if (rc < 0)
+ ret = rc;
+
+ drv_data->num_msgs = 0;
+ drv_data->msgs = NULL;
- return num;
+ return ret;
}
static const struct i2c_algorithm mv64xxx_i2c_algo = {

View File

@ -0,0 +1,86 @@
From: Russell King <rmk+kernel@arm.linux.org.uk>
Date: Thu, 16 May 2013 21:38:11 +0100
Subject: [2/3] I2C: mv64xxx: move mv64xxx_i2c_prepare_for_io()
Origin: https://git.kernel.org/linus/3420afbc06058c9c13f7d69cf48b9d5429db6bd9
Bug-Debian: http://bugs.debian.org/622325
Move mv64xxx_i2c_prepare_for_io() higher up in the driver to avoid a
future forward declaration for this function.
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: Mark A. Greer <mgreer@animalcreek.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
drivers/i2c/busses/i2c-mv64xxx.c | 52 ++++++++++++++++++++--------------------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index f11cb25..bb37e14 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -110,6 +110,32 @@ struct mv64xxx_i2c_data {
struct i2c_adapter adapter;
};
+static void
+mv64xxx_i2c_prepare_for_io(struct mv64xxx_i2c_data *drv_data,
+ struct i2c_msg *msg)
+{
+ u32 dir = 0;
+
+ drv_data->msg = msg;
+ drv_data->byte_posn = 0;
+ drv_data->bytes_left = msg->len;
+ drv_data->aborting = 0;
+ drv_data->rc = 0;
+ drv_data->cntl_bits = MV64XXX_I2C_REG_CONTROL_ACK |
+ MV64XXX_I2C_REG_CONTROL_INTEN | MV64XXX_I2C_REG_CONTROL_TWSIEN;
+
+ if (msg->flags & I2C_M_RD)
+ dir = 1;
+
+ if (msg->flags & I2C_M_TEN) {
+ drv_data->addr1 = 0xf0 | (((u32)msg->addr & 0x300) >> 7) | dir;
+ drv_data->addr2 = (u32)msg->addr & 0xff;
+ } else {
+ drv_data->addr1 = ((u32)msg->addr & 0x7f) << 1 | dir;
+ drv_data->addr2 = 0;
+ }
+}
+
/*
*****************************************************************************
*
@@ -347,32 +373,6 @@ mv64xxx_i2c_intr(int irq, void *dev_id)
*****************************************************************************
*/
static void
-mv64xxx_i2c_prepare_for_io(struct mv64xxx_i2c_data *drv_data,
- struct i2c_msg *msg)
-{
- u32 dir = 0;
-
- drv_data->msg = msg;
- drv_data->byte_posn = 0;
- drv_data->bytes_left = msg->len;
- drv_data->aborting = 0;
- drv_data->rc = 0;
- drv_data->cntl_bits = MV64XXX_I2C_REG_CONTROL_ACK |
- MV64XXX_I2C_REG_CONTROL_INTEN | MV64XXX_I2C_REG_CONTROL_TWSIEN;
-
- if (msg->flags & I2C_M_RD)
- dir = 1;
-
- if (msg->flags & I2C_M_TEN) {
- drv_data->addr1 = 0xf0 | (((u32)msg->addr & 0x300) >> 7) | dir;
- drv_data->addr2 = (u32)msg->addr & 0xff;
- } else {
- drv_data->addr1 = ((u32)msg->addr & 0x7f) << 1 | dir;
- drv_data->addr2 = 0;
- }
-}
-
-static void
mv64xxx_i2c_wait_for_completion(struct mv64xxx_i2c_data *drv_data)
{
long time_left;

View File

@ -120,3 +120,7 @@ bugfix/m68k/atari-irqs.patch
# m68k workaround for div64 called with wrong type args
bugfix/m68k/type-fix-div64.patch
bugfix/arm/I2C-I2C-mv64xxx-remove-I2C_M_NOSTART-code.patch
bugfix/arm/I2C-mv64xxx-move-mv64xxx_i2c_prepare_for_io.patch
bugfix/arm/I2C-mv64xxx-fix-race-between-FSM-interrupt-and-proce.patch