Skip to content

Commit 89119f0

Browse files
jhnikulaWolfram Sang
authored andcommitted
Revert "i2c: designware: do not disable adapter after transfer"
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>
1 parent 4d6d5f1 commit 89119f0

1 file changed

Lines changed: 18 additions & 37 deletions

File tree

drivers/i2c/busses/i2c-designware-core.c

Lines changed: 18 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,7 @@
9191
DW_IC_INTR_TX_ABRT | \
9292
DW_IC_INTR_STOP_DET)
9393

94-
#define DW_IC_STATUS_ACTIVITY 0x1
95-
#define DW_IC_STATUS_TFE BIT(2)
96-
#define DW_IC_STATUS_MST_ACTIVITY BIT(5)
94+
#define DW_IC_STATUS_ACTIVITY 0x1
9795

9896
#define DW_IC_SDA_HOLD_RX_SHIFT 16
9997
#define DW_IC_SDA_HOLD_RX_MASK GENMASK(23, DW_IC_SDA_HOLD_RX_SHIFT)
@@ -478,25 +476,9 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
478476
{
479477
struct i2c_msg *msgs = dev->msgs;
480478
u32 ic_tar = 0;
481-
bool enabled;
482479

483-
enabled = dw_readl(dev, DW_IC_ENABLE_STATUS) & 1;
484-
485-
if (enabled) {
486-
u32 ic_status;
487-
488-
/*
489-
* Only disable adapter if ic_tar and ic_con can't be
490-
* dynamically updated
491-
*/
492-
ic_status = dw_readl(dev, DW_IC_STATUS);
493-
if (!dev->dynamic_tar_update_enabled ||
494-
(ic_status & DW_IC_STATUS_MST_ACTIVITY) ||
495-
!(ic_status & DW_IC_STATUS_TFE)) {
496-
__i2c_dw_enable_and_wait(dev, false);
497-
enabled = false;
498-
}
499-
}
480+
/* Disable the adapter */
481+
__i2c_dw_enable_and_wait(dev, false);
500482

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

529-
if (!enabled)
530-
__i2c_dw_enable(dev, true);
511+
/* Enable the adapter */
512+
__i2c_dw_enable(dev, true);
531513

532514
/* Clear and enable interrupts */
533515
dw_readl(dev, DW_IC_CLR_INTR);
@@ -708,8 +690,7 @@ static int i2c_dw_handle_tx_abort(struct dw_i2c_dev *dev)
708690
}
709691

710692
/*
711-
* Prepare controller for a transaction and start transfer by calling
712-
* i2c_dw_xfer_init()
693+
* Prepare controller for a transaction and call i2c_dw_xfer_msg
713694
*/
714695
static int
715696
i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
@@ -752,6 +733,16 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
752733
goto done;
753734
}
754735

736+
/*
737+
* We must disable the adapter before returning and signaling the end
738+
* of the current transfer. Otherwise the hardware might continue
739+
* generating interrupts which in turn causes a race condition with
740+
* the following transfer. Needs some more investigation if the
741+
* additional interrupts are a hardware bug or this driver doesn't
742+
* handle them correctly yet.
743+
*/
744+
__i2c_dw_enable(dev, false);
745+
755746
if (dev->msg_err) {
756747
ret = dev->msg_err;
757748
goto done;
@@ -893,19 +884,9 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
893884
*/
894885

895886
tx_aborted:
896-
if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET))
897-
|| dev->msg_err) {
898-
/*
899-
* We must disable interruts before returning and signaling
900-
* the end of the current transfer. Otherwise the hardware
901-
* might continue generating interrupts for non-existent
902-
* transfers.
903-
*/
904-
i2c_dw_disable_int(dev);
905-
dw_readl(dev, DW_IC_CLR_INTR);
906-
887+
if ((stat & (DW_IC_INTR_TX_ABRT | DW_IC_INTR_STOP_DET)) || dev->msg_err)
907888
complete(&dev->cmd_complete);
908-
} else if (unlikely(dev->accessor_flags & ACCESS_INTR_MASK)) {
889+
else if (unlikely(dev->accessor_flags & ACCESS_INTR_MASK)) {
909890
/* workaround to trigger pending interrupt */
910891
stat = dw_readl(dev, DW_IC_INTR_MASK);
911892
i2c_dw_disable_int(dev);

0 commit comments

Comments
 (0)