diff mbox series

i2c: designware: fix master is holding SCL low while ENABLE bit is disabled

Message ID 20240905055753.2276-1-kimriver.liu@siengine.com
State Superseded
Delegated to: Andi Shyti
Headers show
Series i2c: designware: fix master is holding SCL low while ENABLE bit is disabled | expand

Commit Message

Liu Kimriver/刘金河 Sept. 5, 2024, 5:57 a.m. UTC 
From: "kimriver.liu" <kimriver.liu@siengine.com>

Failure in normal Stop operational path

This failure happens rarely and is hard to reproduce. Debug trace
showed that IC_STATUS had value of 0x23 when STOP_DET occurred,
immediately disable ENABLE bit that can result in
IC_RAW_INTR_STAT.MASTER_ON_HOLD holding SCL low.

Failure in ENABLE bit is disabled path

It was observed that master is holding SCL low and the IC_ENABLE is
already disabled, Enable ABORT bit and ENABLE bit simultaneously
cannot take effect.

Check if the master is holding SCL low after ENABLE bit is already
disabled. If SCL is held low, The software can set this ABORT bit only
when ENABLE is already set，otherwise,
the controller ignores any write to ABORT bit. When the abort is done,
then proceed with disabling the controller.

These kernel logs show up whenever an I2C transaction is attempted
after this failure.
i2c_designware e95e0000.i2c: timeout in disabling adapter
i2c_designware e95e0000.i2c: timeout waiting for bus ready

The patch can be fix the controller cannot be disabled while SCL is
held low in ENABLE bit is already disabled.

Signed-off-by: kimriver.liu <kimriver.liu@siengine.com>
---
 drivers/i2c/busses/i2c-designware-common.c | 12 +++++++++++
 drivers/i2c/busses/i2c-designware-master.c | 23 +++++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

Comments

Andy Shevchenko Sept. 5, 2024, 11 a.m. UTC | #1 
On Thu, Sep 05, 2024 at 01:57:53PM +0800, kimriver liu wrote:
> From: "kimriver.liu" <kimriver.liu@siengine.com>

You forgot bumping patch version and adding a changelog.
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index e8a688d04aee..74cefbb62bcd 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -453,6 +453,18 @@  void __i2c_dw_disable(struct dw_i2c_dev *dev)
 
 	abort_needed = raw_intr_stats & DW_IC_INTR_MST_ON_HOLD;
 	if (abort_needed) {
+		if (!enable) {
+			regmap_write(dev->map, DW_IC_ENABLE, DW_IC_ENABLE_ENABLE);
+			enable |= DW_IC_ENABLE_ENABLE;
+
+			/*
+			 * Wait two ic_clk delay when enabling the i2c to ensure ENABLE bit
+			 * is already set by the driver (for 400KHz this is 25us)
+			 * as described in the DesignWare I2C databook.
+			 */
+			fsleep(25);
+		}
+
 		regmap_write(dev->map, DW_IC_ENABLE, enable | DW_IC_ENABLE_ABORT);
 		ret = regmap_read_poll_timeout(dev->map, DW_IC_ENABLE, enable,
 					       !(enable & DW_IC_ENABLE_ABORT), 10,
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index c7e56002809a..aba0b8fdfe9a 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -253,6 +253,26 @@  static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 	__i2c_dw_write_intr_mask(dev, DW_IC_INTR_MASTER_MASK);
 }
 
+static bool i2c_dw_is_master_idling(struct dw_i2c_dev *dev)
+{
+	u32 status;
+	int ret;
+
+	regmap_read(dev->map, DW_IC_STATUS, &status);
+	if (!(status & DW_IC_STATUS_MASTER_ACTIVITY))
+		return true;
+
+	ret = regmap_read_poll_timeout(dev->map, DW_IC_STATUS, status,
+			!(status & DW_IC_STATUS_MASTER_ACTIVITY),
+			1100, 20000);
+	if (ret) {
+		dev_err(dev->dev, "i2c master controller not idle %d\n", ret);
+		return false;
+	}
+
+	return true;
+}
+
 static int i2c_dw_check_stopbit(struct dw_i2c_dev *dev)
 {
 	u32 val;
@@ -796,7 +816,8 @@  i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
 	 * additional interrupts are a hardware bug or this driver doesn't
 	 * handle them correctly yet.
 	 */
-	__i2c_dw_disable_nowait(dev);
+	if (i2c_dw_is_master_idling(dev))
+		__i2c_dw_disable_nowait(dev);
 
 	if (dev->msg_err) {
 		ret = dev->msg_err;