Hi Andy, I like your patch and I just did some testing on it. Unluckily, it doesn't solve the lock-up problem.
I haven't investigated any further but I suspect that on top of the cases I observed when debugging this (interrupts after initialisation of dev, easy to prove) there are more obscure cases in which interrupts are generated in an unexpected manner after errors. The interrupt-driven transfer state machine of the driver implicitly relies on the fact that all updates of dev which are related to the same transfer are performed between the mutex_lock and mutex_unlock calls in i2c_dw_xfer. Thus, I decided it was safer to disable the block before releasing the mutex when I wrote my patch. That said, I think the sequencing at transfer initialisation is more logical with your patch and I wonder if it is still worth applying. Any other opinions on this? Greetings, Christian On Fri, Jun 07, 2013 at 12:30:01PM +0300, Andy Shevchenko wrote: > Signed-off-by: Andy Shevchenko <[email protected]> > --- > drivers/i2c/busses/i2c-designware-core.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-core.c > b/drivers/i2c/busses/i2c-designware-core.c > index c41ca63..a1d3d95 100644 > --- a/drivers/i2c/busses/i2c-designware-core.c > +++ b/drivers/i2c/busses/i2c-designware-core.c > @@ -366,9 +366,6 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev) > struct i2c_msg *msgs = dev->msgs; > u32 ic_con; > > - /* Disable the adapter */ > - __i2c_dw_enable(dev, false); > - > /* set the slave (target) address */ > dw_writel(dev, msgs[dev->msg_write_idx].addr, DW_IC_TAR); > > @@ -561,6 +558,13 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg > msgs[], int num) > mutex_lock(&dev->lock); > pm_runtime_get_sync(dev->dev); > > + ret = i2c_dw_wait_bus_not_busy(dev); > + if (ret < 0) > + goto done; > + > + /* Disable the adapter */ > + __i2c_dw_enable(dev, false); > + > INIT_COMPLETION(dev->cmd_complete); > dev->msgs = msgs; > dev->msgs_num = num; > @@ -572,10 +576,6 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg > msgs[], int num) > dev->abort_source = 0; > dev->rx_outstanding = 0; > > - ret = i2c_dw_wait_bus_not_busy(dev); > - if (ret < 0) > - goto done; > - > /* start the transfers */ > i2c_dw_xfer_init(dev); > > -- > 1.8.2.rc0.22.gb3600c3 > -- Christian Ruppert , <[email protected]> /| Tel: +41/(0)22 816 19-42 //| 3, Chemin du Pré-Fleuri _// | bilis Systems CH-1228 Plan-les-Ouates -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
