Skip to content

Commit

Permalink
Revert "i2c: designware: do not disable adapter after transfer"
Browse files Browse the repository at this point in the history 
This reverts commit 0317e6c.

Srinivas reported recently touchscreen and touchpad stopped working in
Haswell based machine in Linux 4.9-rc series with timeout errors from
i2c_designware:

[   16.508013] i2c_designware INT33C3:00: controller timed out
[   16.508302] i2c_hid i2c-MSFT0001:02: failed to change power setting.
[   17.532016] i2c_designware INT33C3:00: controller timed out
[   18.556022] i2c_designware INT33C3:00: controller timed out
[   18.556315] i2c_hid i2c-ATML1000:00: failed to retrieve report from device.

I managed to reproduce similar errors on another Haswell based machine
where touchscreen initialization fails maybe in every 1/5 - 1/2 boots.
Since root cause for these errors is not clear yet and debugging is
ongoing it's better to revert this commit as we are near to release.

Reported-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
  • Loading branch information
@jhnikula
jhnikula authored and Wolfram Sang committed Nov 25, 2016
1 parent 4d6d5f1 commit 89119f0
Showing 1 changed file with 18 additions and 37 deletions.
55 changes: 18 additions & 37 deletions drivers/i2c/busses/i2c-designware-core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,7 @@
DW_IC_INTR_TX_ABRT | \
DW_IC_INTR_STOP_DET)

#define DW_IC_STATUS_ACTIVITY 0x1
#define DW_IC_STATUS_TFE BIT(2)
#define DW_IC_STATUS_MST_ACTIVITY BIT(5)
#define DW_IC_STATUS_ACTIVITY 0x1

#define DW_IC_SDA_HOLD_RX_SHIFT 16
#define DW_IC_SDA_HOLD_RX_MASK GENMASK(23, DW_IC_SDA_HOLD_RX_SHIFT)
Expand Down Expand Up @@ -478,25 +476,9 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
{
struct i2c_msg *msgs = dev->msgs;
u32 ic_tar = 0;
bool enabled;

enabled = dw_readl(dev, DW_IC_ENABLE_STATUS) & 1;

if (enabled) {
u32 ic_status;

/*
* Only disable adapter if ic_tar and ic_con can't be
* dynamically updated
*/
ic_status = dw_readl(dev, DW_IC_STATUS);
if (!dev->dynamic_tar_update_enabled ||
(ic_status & DW_IC_STATUS_MST_ACTIVITY) ||
!(ic_status & DW_IC_STATUS_TFE)) {
__i2c_dw_enable_and_wait(dev, false);
enabled = false;
}
}
/* Disable the adapter */
__i2c_dw_enable_and_wait(dev, false);

/* if the slave address is ten bit address, enable 10BITADDR */
if (dev->dynamic_tar_update_enabled) {
Expand Down Expand Up @@ -526,8 +508,8 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
/* enforce disabled interrupts (due to HW issues) */
i2c_dw_disable_int(dev);

if (!enabled)
__i2c_dw_enable(dev, true);
/* Enable the adapter */
__i2c_dw_enable(dev, true);

/* Clear and enable interrupts */
dw_readl(dev, DW_IC_CLR_INTR);
Expand Down Expand Up @@ -708,8 +690,7 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
}

/*
* Prepare controller for a transaction and start transfer by calling
* i2c_dw_xfer_init()
* Prepare controller for a transaction and call i2c_dw_xfer_msg
*/
static int
i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
Expand Down Expand Up @@ -752,6 +733,16 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
goto done;
}

/*
* We must disable the adapter before returning and signaling the end
* of the current transfer. Otherwise the hardware might continue
* generating interrupts which in turn causes a race condition with
* the following transfer. Needs some more investigation if the
* additional interrupts are a hardware bug or this driver doesn't
* handle them correctly yet.
*/
__i2c_dw_enable(dev, false);

if (dev->msg_err) {
ret = dev->msg_err;
goto done;
Expand Down Expand Up @@ -893,19 +884,9 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
*/

tx_aborted:
if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET))
|| dev->msg_err) {
/*
* We must disable interruts before returning and signaling
* the end of the current transfer. Otherwise the hardware
* might continue generating interrupts for non-existent
* transfers.
*/
i2c_dw_disable_int(dev);
dw_readl(dev, DW_IC_CLR_INTR);

if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET)) || dev->msg_err)
complete(&dev->cmd_complete);
} else if (unlikely(dev->accessor_flags & ACCESS_INTR_MASK)) {
else if (unlikely(dev->accessor_flags & ACCESS_INTR_MASK)) {
/* workaround to trigger pending interrupt */
stat = dw_readl(dev, DW_IC_INTR_MASK);
i2c_dw_disable_int(dev);
Expand Down

0 comments on commit 89119f0

Please sign in to comment.