I2C: mv64xxx: fix race between FSM/interrupt and process context
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>
This commit is contained in:
parent
3420afbc06
commit
4243fa0bad
|
@ -86,6 +86,8 @@ enum {
|
||||||
};
|
};
|
||||||
|
|
||||||
struct mv64xxx_i2c_data {
|
struct mv64xxx_i2c_data {
|
||||||
|
struct i2c_msg *msgs;
|
||||||
|
int num_msgs;
|
||||||
int irq;
|
int irq;
|
||||||
u32 state;
|
u32 state;
|
||||||
u32 action;
|
u32 action;
|
||||||
|
@ -194,7 +196,7 @@ mv64xxx_i2c_fsm(struct mv64xxx_i2c_data *drv_data, u32 status)
|
||||||
if ((drv_data->bytes_left == 0)
|
if ((drv_data->bytes_left == 0)
|
||||||
|| (drv_data->aborting
|
|| (drv_data->aborting
|
||||||
&& (drv_data->byte_posn != 0))) {
|
&& (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->action = MV64XXX_I2C_ACTION_SEND_STOP;
|
||||||
drv_data->state = MV64XXX_I2C_STATE_IDLE;
|
drv_data->state = MV64XXX_I2C_STATE_IDLE;
|
||||||
} else {
|
} else {
|
||||||
|
@ -271,12 +273,25 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
|
||||||
{
|
{
|
||||||
switch(drv_data->action) {
|
switch(drv_data->action) {
|
||||||
case MV64XXX_I2C_ACTION_SEND_RESTART:
|
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_START;
|
||||||
drv_data->cntl_bits &= ~MV64XXX_I2C_REG_CONTROL_INTEN;
|
|
||||||
writel(drv_data->cntl_bits,
|
writel(drv_data->cntl_bits,
|
||||||
drv_data->reg_base + MV64XXX_I2C_REG_CONTROL);
|
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;
|
break;
|
||||||
|
|
||||||
case MV64XXX_I2C_ACTION_CONTINUE:
|
case MV64XXX_I2C_ACTION_CONTINUE:
|
||||||
|
@ -412,20 +427,15 @@ mv64xxx_i2c_wait_for_completion(struct mv64xxx_i2c_data *drv_data)
|
||||||
|
|
||||||
static int
|
static int
|
||||||
mv64xxx_i2c_execute_msg(struct mv64xxx_i2c_data *drv_data, struct i2c_msg *msg,
|
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;
|
unsigned long flags;
|
||||||
|
|
||||||
spin_lock_irqsave(&drv_data->lock, flags);
|
spin_lock_irqsave(&drv_data->lock, flags);
|
||||||
mv64xxx_i2c_prepare_for_io(drv_data, msg);
|
mv64xxx_i2c_prepare_for_io(drv_data, msg);
|
||||||
|
|
||||||
if (is_first) {
|
drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
|
||||||
drv_data->action = MV64XXX_I2C_ACTION_SEND_START;
|
drv_data->state = MV64XXX_I2C_STATE_WAITING_FOR_START_COND;
|
||||||
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->send_stop = is_last;
|
drv_data->send_stop = is_last;
|
||||||
drv_data->block = 1;
|
drv_data->block = 1;
|
||||||
|
@ -453,16 +463,20 @@ static int
|
||||||
mv64xxx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
|
mv64xxx_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
|
||||||
{
|
{
|
||||||
struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap);
|
struct mv64xxx_i2c_data *drv_data = i2c_get_adapdata(adap);
|
||||||
int i, rc;
|
int rc, ret = num;
|
||||||
|
|
||||||
for (i = 0; i < num; i++) {
|
BUG_ON(drv_data->msgs != NULL);
|
||||||
rc = mv64xxx_i2c_execute_msg(drv_data, &msgs[i],
|
drv_data->msgs = msgs;
|
||||||
i == 0, i + 1 == num);
|
drv_data->num_msgs = num;
|
||||||
if (rc < 0)
|
|
||||||
return rc;
|
|
||||||
}
|
|
||||||
|
|
||||||
return 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 ret;
|
||||||
}
|
}
|
||||||
|
|
||||||
static const struct i2c_algorithm mv64xxx_i2c_algo = {
|
static const struct i2c_algorithm mv64xxx_i2c_algo = {
|
||||||
|
|
Loading…
Reference in New Issue