The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Baineng Shou <shoubaineng@gmail.com>
To: Mika Westerberg <mika.westerberg@linux.intel.com>,
	Andi Shyti <andi.shyti@kernel.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org,
	Baineng Shou <shoubaineng@gmail.com>
Subject: [RFC PATCH] i2c: designware: add atomic transfer support for IRQ-off contexts
Date: Fri,  7 Aug 2026 19:33:33 +0800	[thread overview]
Message-ID: <20260807113333.1635449-1-shoubaineng@gmail.com> (raw)

The DesignWare I2C controller driver cannot perform transfers when
IRQs are disabled, e.g. during noirq system resume where an I2C
client (GPIO expander, PMIC) must be accessed before IRQs are
re-enabled: the interrupt-driven path calls wait_for_completion_timeout()
which deadlocks with IRQs off.

The i2c core already routes to master_xfer_atomic() when
i2c_in_atomic_xfer_mode() is true, but that gate requires
system_state > SYSTEM_RUNNING, which does not hold during resume_noirq
(system_state is already SYSTEM_RUNNING there).  So the framework
atomic path does not cover resume_noirq either, and designware does
not implement master_xfer_atomic at all.

Implement i2c_dw_xfer_atomic() and register it as ->xfer_atomic.
It reuses the existing i2c_dw_process_transfer() state machine (the
TX/RX/STOP/ABRT handling is identical to the interrupt path) but:

  - drives the clock directly via i2c_dw_prepare_clk() instead of
    pm_runtime (which may sleep),
  - sets ACCESS_POLLING for the duration of the transfer so register
    reads use IC_RAW_INTR_STAT and the hardware interrupt is masked,
  - polls with udelay() + a retry count instead of usleep_range() +
    jiffies, neither of which is safe with IRQs disabled (the tick is
    frozen so jiffies does not advance, and usleep_range() may sleep).

Also fall back to i2c_dw_xfer_atomic() from i2c_dw_xfer() when IRQs
are disabled, since i2c_in_atomic_xfer_mode() does not cover
resume_noirq.

This is an RFC: the resume_noirq coverage relies on the driver-side
irqs_disabled() fallback because the framework gate is closed there.
I'd like feedback on whether that fallback is acceptable or whether
the gate should be widened in the i2c core instead.

Signed-off-by: Baineng Shou <shoubaineng@gmail.com>
---
 drivers/i2c/busses/i2c-designware-common.c |   1 +
 drivers/i2c/busses/i2c-designware-core.h   |   1 +
 drivers/i2c/busses/i2c-designware-master.c | 109 +++++++++++++++++++++
 3 files changed, 111 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index e4dfa2ec58bb..561efcd39c7f 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -873,6 +873,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
 
 static const struct i2c_algorithm i2c_dw_algo = {
 	.xfer = i2c_dw_xfer,
+	.xfer_atomic = i2c_dw_xfer_atomic,
 	.functionality = i2c_dw_func,
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	.reg_slave = i2c_dw_reg_slave,
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index c71aa2dd368d..4bc22b53d470 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -398,6 +398,7 @@ extern void i2c_dw_configure_master(struct dw_i2c_dev *dev);
 extern int i2c_dw_probe_master(struct dw_i2c_dev *dev);
 
 int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num);
+int i2c_dw_xfer_atomic(struct i2c_adapter *adap, struct i2c_msg *msgs, int num);
 
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 extern void i2c_dw_configure_slave(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c
index 7a301c8b604e..9a94abf86233 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -918,6 +918,104 @@ i2c_dw_xfer_common(struct dw_i2c_dev *dev, struct i2c_msg msgs[], int num)
 	return num;
 }
 
+/* Poll up to ~1s in 10us steps; bounded fallback for the IRQ-off path. */
+#define I2C_DESIGNWARE_ATOMIC_POLL_RETRIES	100000
+
+/*
+ * Atomic-context variant of i2c_dw_wait_transfer().  The normal polling
+ * path (ACCESS_POLLING branch in i2c_dw_wait_transfer()) uses usleep_range()
+ * and a jiffies deadline, neither of which is safe when IRQs are disabled
+ * (noirq system resume, shutdown): the tick is frozen so jiffies does not
+ * advance, and usleep_range() may sleep.  Poll IC_RAW_INTR_STAT with
+ * udelay() and a retry count instead, while reusing the shared
+ * i2c_dw_process_transfer() state machine so TX/RX/STOP/ABRT handling is
+ * identical to the interrupt path.  Caller must have set ACCESS_POLLING.
+ */
+static int i2c_dw_wait_transfer_atomic(struct dw_i2c_dev *dev)
+{
+	unsigned int stat;
+	int retries = I2C_DESIGNWARE_ATOMIC_POLL_RETRIES;
+
+	do {
+		if (try_wait_for_completion(&dev->cmd_complete))
+			return 0;
+
+		stat = i2c_dw_read_clear_intrbits(dev);
+		if (stat)
+			i2c_dw_process_transfer(dev, stat);
+		else
+			udelay(10);
+	} while (--retries > 0);
+
+	return -ETIMEDOUT;
+}
+
+/*
+ * i2c_dw_xfer_atomic - transfer messages in atomic context.
+ *
+ * Used when IRQs are disabled, e.g. during noirq system resume where an
+ * I2C client (GPIO expander, PMIC) must be accessed before IRQs are
+ * re-enabled.  pm_runtime and mutexes may sleep, so drive the clock
+ * directly via i2c_dw_prepare_clk(); ACCESS_POLLING makes register reads
+ * use IC_RAW_INTR_STAT and routes the wait through
+ * i2c_dw_wait_transfer_atomic().
+ */
+int
+i2c_dw_xfer_atomic(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
+	unsigned int flags = dev->flags;
+	int ret;
+
+	dev->flags |= ACCESS_POLLING;
+
+	ret = i2c_dw_prepare_clk(dev, true);
+	if (ret)
+		goto out_flags;
+
+	ret = i2c_dw_acquire_lock(dev);
+	if (ret)
+		goto out_clk;
+
+	reinit_completion(&dev->cmd_complete);
+	dev->msgs = msgs;
+	dev->msgs_num = num;
+	dev->cmd_err = 0;
+	dev->msg_write_idx = 0;
+	dev->msg_read_idx = 0;
+	dev->msg_err = 0;
+	dev->status = 0;
+	dev->abort_source = 0;
+	dev->rx_outstanding = 0;
+
+	i2c_dw_xfer_init(dev);
+
+	ret = i2c_dw_wait_transfer_atomic(dev);
+
+	if (i2c_dw_is_controller_active(dev)) {
+		i2c_recover_bus(&dev->adapter);
+		i2c_dw_init(dev);
+	} else {
+		__i2c_dw_disable_nowait(dev);
+	}
+
+	if (!ret) {
+		if (likely(!dev->cmd_err && !dev->status))
+			ret = 0;
+		else if (dev->cmd_err == DW_IC_ERR_TX_ABRT)
+			ret = i2c_dw_handle_tx_abort(dev);
+		else
+			ret = -EIO;
+	}
+
+	i2c_dw_release_lock(dev);
+out_clk:
+	i2c_dw_prepare_clk(dev, false);
+out_flags:
+	dev->flags = flags;
+	return ret < 0 ? ret : num;
+}
+
 int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 {
 	struct dw_i2c_dev *dev = i2c_get_adapdata(adap);
@@ -925,6 +1023,17 @@ int i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU)
 		return amd_i2c_dw_xfer_quirk(dev, msgs, num);
 
+	/*
+	 * Fall back to the atomic path when IRQs are disabled, e.g. during
+	 * noirq system resume where an I2C client (GPIO expander, PMIC)
+	 * must be accessed before IRQs are re-enabled.  The i2c core's
+	 * i2c_in_atomic_xfer_mode() gate does not cover resume_noirq
+	 * (system_state is already SYSTEM_RUNNING there), so the driver has
+	 * to route the transfer itself.
+	 */
+	if (IS_ENABLED(CONFIG_PREEMPT_COUNT) ? !preemptible() : irqs_disabled())
+		return i2c_dw_xfer_atomic(adap, msgs, num);
+
 	return i2c_dw_xfer_common(dev, msgs, num);
 }
 
-- 
2.34.1


             reply	other threads:[~2026-08-07 11:33 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-07 11:33 Baineng Shou [this message]
2026-08-08 20:46 ` [RFC PATCH] i2c: designware: add atomic transfer support for IRQ-off contexts Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260807113333.1635449-1-shoubaineng@gmail.com \
    --to=shoubaineng@gmail.com \
    --cc=andi.shyti@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox