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 <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> 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 , <christian.ruppert@xxxxxxxxxx>
/|
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
