linux-kernel - [PATCH] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240904064224.2394-1-kimriver.liu@siengine.com>
Date: Wed, 4 Sep 2024 14:42:24 +0800
From: kimriver liu <kimriver.liu@...ngine.com>
To: <jarkko.nikula@...ux.intel.com>
CC: <andriy.shevchenko@...ux.intel.com>, <mika.westerberg@...ux.intel.com>,
        <jsd@...ihalf.com>, <andi.shyti@...nel.org>,
        <linux-i2c@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <kimriver.liu@...ngine.com>
Subject: [PATCH] i2c: designware: fix master is holding SCL low while ENABLE bit is disabled

From: "kimriver.liu" <kimriver.liu@...ngine.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@...ngine.com>
---
 drivers/i2c/busses/i2c-designware-common.c |  5 +++++
 drivers/i2c/busses/i2c-designware-master.c | 21 ++++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index e8a688d04aee..e1596b67e92f 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -453,6 +453,11 @@ 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;
+			usleep_range(25, 100);
+		}
 		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..f86d03b0472a 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -253,6 +253,23 @@ static void i2c_dw_xfer_init(struct dw_i2c_dev *dev)
 	__i2c_dw_write_intr_mask(dev, DW_IC_INTR_MASTER_MASK);
 }
 
+static int i2c_dw_check_mst_activity(struct dw_i2c_dev *dev)
+{
+	u32 status = 0;
+	int ret = 0;
+
+	regmap_read(dev->map, DW_IC_STATUS, &status);
+	if (status & DW_IC_STATUS_MASTER_ACTIVITY) {
+		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 mst activity not idle %d\n", ret);
+	}
+
+	return ret;
+}
+
 static int i2c_dw_check_stopbit(struct dw_i2c_dev *dev)
 {
 	u32 val;
@@ -796,7 +813,9 @@ 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);
+	ret = i2c_dw_check_mst_activity(dev);
+	if (!ret)
+		__i2c_dw_disable_nowait(dev);
 
 	if (dev->msg_err) {
 		ret = dev->msg_err;
-- 
2.17.1