* [PATCH v4 1/3] dmaengine: qcom: gpi: Add I2C bus recovery opcode support
2026-11-03 8:56 [PATCH v4 0/3] i2c: qcom-geni: Add I2C bus recovery support Aniket Randive
@ 2026-11-03 8:56 ` Aniket Randive
2026-11-03 10:00 ` Mukesh Savaliya
2026-11-03 8:56 ` [PATCH v4 2/3] i2c: qcom-geni: Add bus recovery support for FIFO mode Aniket Randive
2026-11-03 8:56 ` [PATCH v4 3/3] i2c: qcom-geni: Add bus recovery support for GPI DMA mode Aniket Randive
2 siblings, 1 reply; 10+ messages in thread
From: Aniket Randive @ 2026-11-03 8:56 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Mukesh Kumar Savaliya, Viken Dadhaniya, Andi Shyti
Cc: naresh.maramaina, linux-arm-msm, dmaengine, linux-kernel,
linux-i2c, Aniket Randive
The I2C_BUS_CLEAR and I2C_STOP_ON_BUS GENI sequencer opcodes release a
stuck I2C bus, but the GPI DMA driver has no way to emit these
zero-payload control commands, so an I2C controller using GPI DMA
cannot recover the bus.
Add I2C_BUS_CLEAR and I2C_STOP_ON_BUS to enum i2c_op so I2C drivers can
request them through gpi_i2c_config.op. Handle them in
gpi_create_i2c_tre() by emitting a CONFIG TRE (when set_config is set)
followed by a GO TRE with IEOT set and no DMA TRE, and reserve the
matching number of ring entries in gpi_prep_slave_sg().
Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
---
drivers/dma/qcom/gpi.c | 30 ++++++++++++++++++++++++++++++
drivers/i2c/busses/i2c-qcom-geni.c | 2 --
include/linux/dma/qcom-gpi-dma.h | 2 ++
3 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index c41bfac65ddf..3dda58e6a99d 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -1620,6 +1620,17 @@ gpi_peripheral_config(struct dma_chan *chan, struct dma_slave_config *config)
return 0;
}
+static void gpi_create_i2c_go_recovery_tre(struct gpi_i2c_config *i2c,
+ struct gpi_tre *tre)
+{
+ tre->dword[0] = u32_encode_bits(i2c->op, TRE_I2C_GO_CMD);
+ tre->dword[1] = 0;
+ tre->dword[2] = 0;
+
+ tre->dword[3] = u32_encode_bits(TRE_TYPE_GO, TRE_FLAGS_TYPE);
+ tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_IEOT);
+}
+
static int gpi_create_i2c_tre(struct gchan *chan, struct gpi_desc *desc,
struct scatterlist *sgl, enum dma_transfer_direction direction,
unsigned long flags)
@@ -1650,6 +1661,12 @@ static int gpi_create_i2c_tre(struct gchan *chan, struct gpi_desc *desc,
tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_CHAIN);
}
+ if (i2c->op == I2C_BUS_CLEAR || i2c->op == I2C_STOP_ON_BUS) {
+ gpi_create_i2c_go_recovery_tre(i2c, &desc->tre[tre_idx]);
+ tre_idx++;
+ goto log_tre;
+ }
+
/* create the GO tre for Tx */
if (i2c->op == I2C_WRITE) {
tre = &desc->tre[tre_idx];
@@ -1692,6 +1709,7 @@ static int gpi_create_i2c_tre(struct gchan *chan, struct gpi_desc *desc,
tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_BEI);
}
+log_tre:
for (i = 0; i < tre_idx; i++)
dev_dbg(dev, "TRE:%d %x:%x:%x:%x\n", i, desc->tre[i].dword[0],
desc->tre[i].dword[1], desc->tre[i].dword[2], desc->tre[i].dword[3]);
@@ -1825,6 +1843,18 @@ gpi_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
nr_tre = 2;
if (direction == DMA_DEV_TO_MEM) /* rx */
nr_tre = 1;
+ /*
+ * Recovery opcodes do not require DMA data TREs, only CONFIG
+ * (for set_config) and GO TREs. Since gpi_prep_slave_sg() is
+ * common to all protocols, verify the channel is I2C before
+ * accessing the configuration data.
+ */
+ if (gchan->protocol == QCOM_GPI_I2C) {
+ struct gpi_i2c_config *i2c = gchan->config;
+
+ if (i2c->op == I2C_BUS_CLEAR || i2c->op == I2C_STOP_ON_BUS)
+ nr_tre = set_config ? 2 : 1;
+ }
/* calculate # of elements required & available */
nr = gpi_ring_num_elements_avail(ch_ring);
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 5cc47b670bd1..60b6273f538c 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -34,8 +34,6 @@
#define I2C_READ 0x2
#define I2C_WRITE_READ 0x3
#define I2C_ADDR_ONLY 0x4
-#define I2C_BUS_CLEAR 0x6
-#define I2C_STOP_ON_BUS 0x7
/* M_CMD params for I2C */
#define PRE_CMD_DELAY BIT(0)
#define TIMESTAMP_BEFORE BIT(1)
diff --git a/include/linux/dma/qcom-gpi-dma.h b/include/linux/dma/qcom-gpi-dma.h
index 332be28427e4..b9b75c302825 100644
--- a/include/linux/dma/qcom-gpi-dma.h
+++ b/include/linux/dma/qcom-gpi-dma.h
@@ -52,6 +52,8 @@ struct gpi_spi_config {
enum i2c_op {
I2C_WRITE = 1,
I2C_READ,
+ I2C_BUS_CLEAR = 6,
+ I2C_STOP_ON_BUS,
};
/**
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v4 1/3] dmaengine: qcom: gpi: Add I2C bus recovery opcode support
2026-11-03 8:56 ` [PATCH v4 1/3] dmaengine: qcom: gpi: Add I2C bus recovery opcode support Aniket Randive
@ 2026-11-03 10:00 ` Mukesh Savaliya
0 siblings, 0 replies; 10+ messages in thread
From: Mukesh Savaliya @ 2026-11-03 10:00 UTC (permalink / raw)
To: Aniket Randive, Vinod Koul, Frank Li, Viken Dadhaniya, Andi Shyti
Cc: naresh.maramaina, linux-arm-msm, dmaengine, linux-kernel, linux-i2c
On 11/3/2026 2:26 PM, Aniket Randive wrote:
> The I2C_BUS_CLEAR and I2C_STOP_ON_BUS GENI sequencer opcodes release a
> stuck I2C bus, but the GPI DMA driver has no way to emit these
> zero-payload control commands, so an I2C controller using GPI DMA
> cannot recover the bus.
>
> Add I2C_BUS_CLEAR and I2C_STOP_ON_BUS to enum i2c_op so I2C drivers can
> request them through gpi_i2c_config.op. Handle them in
> gpi_create_i2c_tre() by emitting a CONFIG TRE (when set_config is set)
> followed by a GO TRE with IEOT set and no DMA TRE, and reserve the
> matching number of ring entries in gpi_prep_slave_sg().
>
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> ---
Reviewed-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 2/3] i2c: qcom-geni: Add bus recovery support for FIFO mode
2026-11-03 8:56 [PATCH v4 0/3] i2c: qcom-geni: Add I2C bus recovery support Aniket Randive
2026-11-03 8:56 ` [PATCH v4 1/3] dmaengine: qcom: gpi: Add I2C bus recovery opcode support Aniket Randive
@ 2026-11-03 8:56 ` Aniket Randive
2026-11-03 11:35 ` Mukesh Savaliya
2026-11-03 8:56 ` [PATCH v4 3/3] i2c: qcom-geni: Add bus recovery support for GPI DMA mode Aniket Randive
2 siblings, 1 reply; 10+ messages in thread
From: Aniket Randive @ 2026-11-03 8:56 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Mukesh Kumar Savaliya, Viken Dadhaniya, Andi Shyti
Cc: naresh.maramaina, linux-arm-msm, dmaengine, linux-kernel,
linux-i2c, Aniket Randive
I2C transfers in FIFO mode can fail with -EPROTO, -ETIMEDOUT or
-EAGAIN when a target holds SDA low, leaving the bus stuck and
preventing subsequent transactions.
Add bus recovery support using the I2C_BUS_CLEAR and I2C_STOP_ON_BUS
hardware opcodes to restore the bus to an idle state. Check the SDA
line state through SE_GENI_IOS.RX_DATA_IN and skip recovery when the
bus is already free. Trigger recovery automatically from
geni_i2c_xfer() on -EPROTO, -ETIMEDOUT and -EAGAIN (arbitration-lost)
errors, and register the recovery callback through i2c_bus_recovery_info
to allow recovery via i2c_recover_bus().
Use the adapter timeout for recovery commands. Configure the serial clock
and bus timing after runtime resume so direct calls to i2c_recover_bus()
do not depend on an earlier transfer having configured them.
This adds bus recovery support for FIFO mode only.
Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
---
drivers/i2c/busses/i2c-qcom-geni.c | 124 ++++++++++++++++++++++++++++++++++++-
1 file changed, 123 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 60b6273f538c..b08353616adc 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -34,6 +34,7 @@
#define I2C_READ 0x2
#define I2C_WRITE_READ 0x3
#define I2C_ADDR_ONLY 0x4
+#define I2C_BUS_FREE (RX_DATA_IN | IO2_DATA_IN)
/* M_CMD params for I2C */
#define PRE_CMD_DELAY BIT(0)
#define TIMESTAMP_BEFORE BIT(1)
@@ -145,6 +146,7 @@ struct geni_i2c_dev {
u32 num_msgs;
struct geni_i2c_gpi_multi_desc_xfer i2c_multi_desc_config;
const struct geni_i2c_desc *dev_data;
+ struct i2c_bus_recovery_info rinfo;
};
struct geni_i2c_err_log {
@@ -316,7 +318,7 @@ static void geni_i2c_check_addr_data_nack(struct geni_i2c_dev *gi2c)
{
if (!readl_relaxed(gi2c->se.base + SE_GENI_M_GP_LENGTH))
geni_i2c_err(gi2c, ADDR_NACK);
- else if (!(gi2c->cur->flags & I2C_M_RD))
+ else if (gi2c->cur && !(gi2c->cur->flags & I2C_M_RD))
geni_i2c_err(gi2c, DATA_NACK);
}
@@ -1013,6 +1015,112 @@ static int geni_i2c_fifo_xfer(struct geni_i2c_dev *gi2c,
return num;
}
+static int geni_i2c_fifo_bus_cmd(struct geni_i2c_dev *gi2c, u32 cmd)
+{
+ unsigned long time_left;
+ unsigned long flags;
+
+ /*
+ * Clear cur so the IRQ handler does not attempt FIFO watermark
+ * filling or draining while the recovery opcode is in flight.
+ * cur and err are shared with geni_i2c_irq(), which reads cur and
+ * writes err under gi2c->lock, so take the lock around this reset.
+ */
+ spin_lock_irqsave(&gi2c->lock, flags);
+ gi2c->cur = NULL;
+ gi2c->err = 0;
+ spin_unlock_irqrestore(&gi2c->lock, flags);
+ geni_se_select_mode(&gi2c->se, GENI_SE_FIFO);
+ reinit_completion(&gi2c->done);
+
+ geni_se_setup_m_cmd(&gi2c->se, cmd, 0);
+ time_left = wait_for_completion_timeout(&gi2c->done, gi2c->adap.timeout);
+ if (!time_left) {
+ dev_err(gi2c->se.dev, "timeout waiting for bus cmd %u\n", cmd);
+ geni_i2c_abort_xfer(gi2c);
+
+ return -ETIMEDOUT;
+ }
+
+ /*
+ * ARB_LOST and BUS_PROTO interrupts may be reported while the bus
+ * transitions from stuck to idle during the recovery sequence.
+ * The opcode completed successfully so treat these as success.
+ */
+ if (gi2c->err == -EAGAIN || gi2c->err == -EPROTO)
+ return 0;
+
+ return gi2c->err;
+}
+
+static bool geni_i2c_bus_is_free(struct geni_i2c_dev *gi2c)
+{
+ return (readl_relaxed(gi2c->se.base + SE_GENI_IOS) & I2C_BUS_FREE) ==
+ I2C_BUS_FREE;
+}
+
+static bool geni_i2c_needs_bus_recovery(struct geni_i2c_dev *gi2c, int ret)
+{
+ /*
+ * -EPROTO and -ETIMEDOUT indicate bus errors where the target may be
+ * holding a bus line low. ARB_LOST (-EAGAIN) on a single-controller
+ * bus indicates a stuck target, not a real arbitration loss.
+ * GPI DMA mode extends this trigger separately.
+ */
+ return !gi2c->gpi_mode &&
+ (ret == -EPROTO || ret == -ETIMEDOUT || ret == -EAGAIN) &&
+ !geni_i2c_bus_is_free(gi2c);
+}
+
+static int geni_i2c_recover_bus(struct i2c_adapter *adap)
+{
+ struct geni_i2c_dev *gi2c = i2c_get_adapdata(adap);
+ u32 ios;
+ int ret;
+
+ ret = pm_runtime_get_sync(gi2c->se.dev);
+ if (ret < 0) {
+ dev_err(gi2c->se.dev, "bus recovery failed, error turning SE resources:%d\n", ret);
+ pm_runtime_put_noidle(gi2c->se.dev);
+ return ret;
+ }
+
+ ret = gi2c->dev_data->set_rate(&gi2c->se, gi2c->clk_freq_out);
+ if (ret) {
+ dev_err(gi2c->se.dev, "bus recovery failed, error setting clock rate: %d\n",
+ ret);
+ goto out_pm;
+ }
+
+ if (geni_i2c_bus_is_free(gi2c)) {
+ ret = 0;
+ goto out_pm;
+ }
+
+ /* Bus clear cannot recover a target holding SCL low. */
+ ios = readl_relaxed(gi2c->se.base + SE_GENI_IOS);
+ if (!(ios & IO2_DATA_IN)) {
+ ret = -EBUSY;
+ goto out_pm;
+ }
+
+ ret = geni_i2c_fifo_bus_cmd(gi2c, I2C_BUS_CLEAR);
+ if (!ret)
+ ret = geni_i2c_fifo_bus_cmd(gi2c, I2C_STOP_ON_BUS);
+
+ /*
+ * Recovery succeeds only once both bus lines are released. The bus
+ * state is the authority, while a clean opcode status with either
+ * line low is a failed recovery.
+ */
+ ret = geni_i2c_bus_is_free(gi2c) ? 0 : (ret ?: -EBUSY);
+
+out_pm:
+ pm_runtime_put_autosuspend(gi2c->se.dev);
+
+ return ret;
+}
+
static int geni_i2c_xfer(struct i2c_adapter *adap,
struct i2c_msg msgs[],
int num)
@@ -1037,6 +1145,15 @@ static int geni_i2c_xfer(struct i2c_adapter *adap,
else
ret = geni_i2c_fifo_xfer(gi2c, msgs, num);
+ if (geni_i2c_needs_bus_recovery(gi2c, ret)) {
+ int recovery_ret = i2c_recover_bus(adap);
+
+ if (recovery_ret)
+ dev_err(gi2c->se.dev,
+ "bus recovery failed: %d (xfer error: %d)\n",
+ recovery_ret, ret);
+ }
+
out_pm:
pm_runtime_put_autosuspend(gi2c->se.dev);
gi2c->cur = NULL;
@@ -1239,6 +1356,11 @@ static int geni_i2c_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
+ if (!gi2c->gpi_mode) {
+ gi2c->rinfo.recover_bus = geni_i2c_recover_bus;
+ gi2c->adap.bus_recovery_info = &gi2c->rinfo;
+ }
+
ret = i2c_add_adapter(&gi2c->adap);
if (ret) {
release_gpi_dma(gi2c);
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v4 2/3] i2c: qcom-geni: Add bus recovery support for FIFO mode
2026-11-03 8:56 ` [PATCH v4 2/3] i2c: qcom-geni: Add bus recovery support for FIFO mode Aniket Randive
@ 2026-11-03 11:35 ` Mukesh Savaliya
0 siblings, 0 replies; 10+ messages in thread
From: Mukesh Savaliya @ 2026-11-03 11:35 UTC (permalink / raw)
To: Aniket Randive, Vinod Koul, Frank Li, Viken Dadhaniya, Andi Shyti
Cc: naresh.maramaina, linux-arm-msm, dmaengine, linux-kernel, linux-i2c
On 11/3/2026 2:26 PM, Aniket Randive wrote:
> I2C transfers in FIFO mode can fail with -EPROTO, -ETIMEDOUT or
> -EAGAIN when a target holds SDA low, leaving the bus stuck and
> preventing subsequent transactions.
>
> Add bus recovery support using the I2C_BUS_CLEAR and I2C_STOP_ON_BUS
> hardware opcodes to restore the bus to an idle state. Check the SDA
> line state through SE_GENI_IOS.RX_DATA_IN and skip recovery when the
> bus is already free. Trigger recovery automatically from
> geni_i2c_xfer() on -EPROTO, -ETIMEDOUT and -EAGAIN (arbitration-lost)
> errors, and register the recovery callback through i2c_bus_recovery_info
> to allow recovery via i2c_recover_bus().
>
> Use the adapter timeout for recovery commands. Configure the serial clock
> and bus timing after runtime resume so direct calls to i2c_recover_bus()
> do not depend on an earlier transfer having configured them.
>
> This adds bus recovery support for FIFO mode only.
>
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> ---
Reviewed-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 3/3] i2c: qcom-geni: Add bus recovery support for GPI DMA mode
2026-11-03 8:56 [PATCH v4 0/3] i2c: qcom-geni: Add I2C bus recovery support Aniket Randive
2026-11-03 8:56 ` [PATCH v4 1/3] dmaengine: qcom: gpi: Add I2C bus recovery opcode support Aniket Randive
2026-11-03 8:56 ` [PATCH v4 2/3] i2c: qcom-geni: Add bus recovery support for FIFO mode Aniket Randive
@ 2026-11-03 8:56 ` Aniket Randive
2026-11-03 10:44 ` Mukesh Savaliya
2026-11-03 11:47 ` Mukesh Savaliya
2 siblings, 2 replies; 10+ messages in thread
From: Aniket Randive @ 2026-11-03 8:56 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Mukesh Kumar Savaliya, Viken Dadhaniya, Andi Shyti
Cc: naresh.maramaina, linux-arm-msm, dmaengine, linux-kernel,
linux-i2c, Aniket Randive
I2C transfers in GPI DMA mode can fail with -EIO when a target holds
SDA low, leaving the bus stuck and preventing subsequent transactions.
The GPI completion callback reports a NACK and a genuine bus error
alike as -EIO, and the existing recovery path switches the SE to FIFO
mode which is not valid in GPI DMA mode.
Add bus recovery support using the I2C_BUS_CLEAR and I2C_STOP_ON_BUS
opcodes issued via the GPI TX DMA channel. Guard the -EIO recovery
trigger with a SE_GENI_IOS I2C_BUS_FREE check to skip recovery when both
SDA and SCL are already released. Extend the recovery callback to dispatch
through the GPI DMA path when gi2c->gpi_mode is set.
Use the adapter timeout for GPI recovery commands, and preserve the DMA
configuration error code when GPI setup fails.
Stop recovery on a GPI command timeout and report DMA termination
failures. Do not issue another command or override the timeout with a
bus-idle result, since DMA cleanup may not have completed successfully.
Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
---
drivers/i2c/busses/i2c-qcom-geni.c | 107 ++++++++++++++++++++++++++++++++-----
1 file changed, 94 insertions(+), 13 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index b08353616adc..d5d5d90544e8 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -1065,13 +1065,82 @@ static bool geni_i2c_needs_bus_recovery(struct geni_i2c_dev *gi2c, int ret)
* -EPROTO and -ETIMEDOUT indicate bus errors where the target may be
* holding a bus line low. ARB_LOST (-EAGAIN) on a single-controller
* bus indicates a stuck target, not a real arbitration loss.
- * GPI DMA mode extends this trigger separately.
+ * In GPI DMA mode the completion callback reports NACK, BUS_PROTO and
+ * ARB_LOST alike as -EIO, so the SE_GENI_IOS I2C_BUS_FREE check
+ * distinguishes a genuinely stuck bus from a benign NACK. Recovery is
+ * skipped when both SDA and SCL are released (high) after the STOP.
*/
- return !gi2c->gpi_mode &&
- (ret == -EPROTO || ret == -ETIMEDOUT || ret == -EAGAIN) &&
+ return (ret == -EPROTO || ret == -ETIMEDOUT || ret == -EAGAIN ||
+ (gi2c->gpi_mode && ret == -EIO)) &&
!geni_i2c_bus_is_free(gi2c);
}
+static int geni_i2c_gpi_bus_cmd(struct geni_i2c_dev *gi2c, enum i2c_op cmd)
+{
+ const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
+ struct dma_async_tx_descriptor *desc;
+ struct gpi_i2c_config peripheral = {};
+ struct dma_slave_config config = {};
+ unsigned long time_left;
+ unsigned long flags;
+ dma_cookie_t cookie;
+ int ret;
+
+ config.peripheral_config = &peripheral;
+ config.peripheral_size = sizeof(peripheral);
+
+ peripheral.set_config = 1;
+ peripheral.pack_enable = I2C_PACK_TX | I2C_PACK_RX;
+ peripheral.cycle_count = itr->t_cycle_cnt;
+ peripheral.high_count = itr->t_high_cnt;
+ peripheral.low_count = itr->t_low_cnt;
+ peripheral.clk_div = itr->clk_div;
+ peripheral.op = cmd;
+
+ ret = dmaengine_slave_config(gi2c->tx_c, &config);
+ if (ret) {
+ dev_err(gi2c->se.dev, "dma config error %d for bus cmd %u\n", ret, cmd);
+ return ret;
+ }
+
+ desc = dmaengine_prep_slave_single(gi2c->tx_c, 0, 0, DMA_MEM_TO_DEV,
+ DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ if (!desc)
+ return -EIO;
+
+ desc->callback_result = i2c_gpi_cb_result;
+ desc->callback_param = gi2c;
+
+ reinit_completion(&gi2c->done);
+ cookie = dmaengine_submit(desc);
+ ret = dma_submit_error(cookie);
+ if (ret)
+ return ret;
+
+ /*
+ * cur and err are shared with geni_i2c_irq() and the GPI callback
+ * i2c_gpi_cb_result(); both write err (the IRQ handler under
+ * gi2c->lock). Reset them under the lock before issuing the transfer.
+ */
+ spin_lock_irqsave(&gi2c->lock, flags);
+ gi2c->cur = NULL;
+ gi2c->err = 0;
+ spin_unlock_irqrestore(&gi2c->lock, flags);
+ dma_async_issue_pending(gi2c->tx_c);
+
+ time_left = wait_for_completion_timeout(&gi2c->done, gi2c->adap.timeout);
+ if (!time_left) {
+ dev_err(gi2c->se.dev, "timeout waiting for GPI bus cmd %u\n", cmd);
+ ret = dmaengine_terminate_sync(gi2c->tx_c);
+ if (ret)
+ dev_err(gi2c->se.dev, "DMA termination failed for bus cmd %u: %d\n",
+ cmd, ret);
+ return -ETIMEDOUT;
+ }
+
+ return gi2c->err;
+}
+
static int geni_i2c_recover_bus(struct i2c_adapter *adap)
{
struct geni_i2c_dev *gi2c = i2c_get_adapdata(adap);
@@ -1104,14 +1173,28 @@ static int geni_i2c_recover_bus(struct i2c_adapter *adap)
goto out_pm;
}
- ret = geni_i2c_fifo_bus_cmd(gi2c, I2C_BUS_CLEAR);
- if (!ret)
- ret = geni_i2c_fifo_bus_cmd(gi2c, I2C_STOP_ON_BUS);
+ if (gi2c->gpi_mode) {
+ ret = geni_i2c_gpi_bus_cmd(gi2c, I2C_BUS_CLEAR);
+ /* Do not issue more commands after a timeout: DMA cleanup may have failed. */
+ if (ret == -ETIMEDOUT)
+ goto out_pm;
+ if (ret)
+ dev_dbg(gi2c->se.dev, "GPI bus clear returned %d, issuing stop anyway\n",
+ ret);
+
+ ret = geni_i2c_gpi_bus_cmd(gi2c, I2C_STOP_ON_BUS);
+ if (ret == -ETIMEDOUT)
+ goto out_pm;
+ } else {
+ ret = geni_i2c_fifo_bus_cmd(gi2c, I2C_BUS_CLEAR);
+ if (!ret)
+ ret = geni_i2c_fifo_bus_cmd(gi2c, I2C_STOP_ON_BUS);
+ }
/*
- * Recovery succeeds only once both bus lines are released. The bus
- * state is the authority, while a clean opcode status with either
- * line low is a failed recovery.
+ * Recovery succeeds when both bus lines are released. Trust the bus
+ * state over the opcode return path, which cannot flag a benign
+ * recovery event in GPI mode.
*/
ret = geni_i2c_bus_is_free(gi2c) ? 0 : (ret ?: -EBUSY);
@@ -1356,10 +1439,8 @@ static int geni_i2c_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
- if (!gi2c->gpi_mode) {
- gi2c->rinfo.recover_bus = geni_i2c_recover_bus;
- gi2c->adap.bus_recovery_info = &gi2c->rinfo;
- }
+ gi2c->rinfo.recover_bus = geni_i2c_recover_bus;
+ gi2c->adap.bus_recovery_info = &gi2c->rinfo;
ret = i2c_add_adapter(&gi2c->adap);
if (ret) {
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v4 3/3] i2c: qcom-geni: Add bus recovery support for GPI DMA mode
2026-11-03 8:56 ` [PATCH v4 3/3] i2c: qcom-geni: Add bus recovery support for GPI DMA mode Aniket Randive
@ 2026-11-03 10:44 ` Mukesh Savaliya
2026-11-03 11:16 ` Aniket RANDIVE
2026-11-03 11:47 ` Mukesh Savaliya
1 sibling, 1 reply; 10+ messages in thread
From: Mukesh Savaliya @ 2026-11-03 10:44 UTC (permalink / raw)
To: Aniket Randive, Vinod Koul, Frank Li, Viken Dadhaniya, Andi Shyti
Cc: naresh.maramaina, linux-arm-msm, dmaengine, linux-kernel, linux-i2c
On 11/3/2026 2:26 PM, Aniket Randive wrote:
> I2C transfers in GPI DMA mode can fail with -EIO when a target holds
> SDA low, leaving the bus stuck and preventing subsequent transactions.
> The GPI completion callback reports a NACK and a genuine bus error
> alike as -EIO, and the existing recovery path switches the SE to FIFO
> mode which is not valid in GPI DMA mode.
>
> Add bus recovery support using the I2C_BUS_CLEAR and I2C_STOP_ON_BUS
> opcodes issued via the GPI TX DMA channel. Guard the -EIO recovery
> trigger with a SE_GENI_IOS I2C_BUS_FREE check to skip recovery when both
> SDA and SCL are already released. Extend the recovery callback to dispatch
> through the GPI DMA path when gi2c->gpi_mode is set.
>
> Use the adapter timeout for GPI recovery commands, and preserve the DMA
> configuration error code when GPI setup fails.
>
> Stop recovery on a GPI command timeout and report DMA termination
> failures. Do not issue another command or override the timeout with a
> bus-idle result, since DMA cleanup may not have completed successfully.
>
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> ---
> drivers/i2c/busses/i2c-qcom-geni.c | 107 ++++++++++++++++++++++++++++++++-----
> 1 file changed, 94 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
[...]
> @@ -1356,10 +1439,8 @@ static int geni_i2c_probe(struct platform_device *pdev)
> if (ret < 0)
> return ret;
>
> - if (!gi2c->gpi_mode) {
> - gi2c->rinfo.recover_bus = geni_i2c_recover_bus;
> - gi2c->adap.bus_recovery_info = &gi2c->rinfo;
> - }
> + gi2c->rinfo.recover_bus = geni_i2c_recover_bus;
> + gi2c->adap.bus_recovery_info = &gi2c->rinfo;
I guess one more time this. Why patch 2 is adding and patch 3 is
removing the change ? keep it clean way.
>
> ret = i2c_add_adapter(&gi2c->adap);
> if (ret) {
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v4 3/3] i2c: qcom-geni: Add bus recovery support for GPI DMA mode
2026-11-03 10:44 ` Mukesh Savaliya
@ 2026-11-03 11:16 ` Aniket RANDIVE
2026-11-03 11:24 ` Mukesh Savaliya
0 siblings, 1 reply; 10+ messages in thread
From: Aniket RANDIVE @ 2026-11-03 11:16 UTC (permalink / raw)
To: Mukesh Savaliya, Vinod Koul, Frank Li, Viken Dadhaniya, Andi Shyti
Cc: naresh.maramaina, linux-arm-msm, dmaengine, linux-kernel, linux-i2c
On 11/3/2026 4:14 PM, Mukesh Savaliya wrote:
>
>
> On 11/3/2026 2:26 PM, Aniket Randive wrote:
>> I2C transfers in GPI DMA mode can fail with -EIO when a target holds
>> SDA low, leaving the bus stuck and preventing subsequent transactions.
>> The GPI completion callback reports a NACK and a genuine bus error
>> alike as -EIO, and the existing recovery path switches the SE to FIFO
>> mode which is not valid in GPI DMA mode.
>>
>> Add bus recovery support using the I2C_BUS_CLEAR and I2C_STOP_ON_BUS
>> opcodes issued via the GPI TX DMA channel. Guard the -EIO recovery
>> trigger with a SE_GENI_IOS I2C_BUS_FREE check to skip recovery when both
>> SDA and SCL are already released. Extend the recovery callback to
>> dispatch
>> through the GPI DMA path when gi2c->gpi_mode is set.
>>
>> Use the adapter timeout for GPI recovery commands, and preserve the DMA
>> configuration error code when GPI setup fails.
>>
>> Stop recovery on a GPI command timeout and report DMA termination
>> failures. Do not issue another command or override the timeout with a
>> bus-idle result, since DMA cleanup may not have completed successfully.
>>
>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
>> ---
>> drivers/i2c/busses/i2c-qcom-geni.c | 107 +++++++++++++++++++++++++++
>> +++++-----
>> 1 file changed, 94 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/
>> i2c-qcom-geni.c
>
> [...]
>
>> @@ -1356,10 +1439,8 @@ static int geni_i2c_probe(struct
>> platform_device *pdev)
>> if (ret < 0)
>> return ret;
>> - if (!gi2c->gpi_mode) {
>> - gi2c->rinfo.recover_bus = geni_i2c_recover_bus;
>> - gi2c->adap.bus_recovery_info = &gi2c->rinfo;
>> - }
>> + gi2c->rinfo.recover_bus = geni_i2c_recover_bus;
>> + gi2c->adap.bus_recovery_info = &gi2c->rinfo;
>
> I guess one more time this. Why patch 2 is adding and patch 3 is
> removing the change ? keep it clean way.
This is intentional. In patch 2, bus recovery is supported only in FIFO
mode, so we guard the registration with !gi2c->gpi_mode. Patch 3 adds
the corresponding bus recovery support for GPI mode, after which the
mode check is no longer needed and is removed.
>> ret = i2c_add_adapter(&gi2c->adap);
>> if (ret) {
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v4 3/3] i2c: qcom-geni: Add bus recovery support for GPI DMA mode
2026-11-03 11:16 ` Aniket RANDIVE
@ 2026-11-03 11:24 ` Mukesh Savaliya
0 siblings, 0 replies; 10+ messages in thread
From: Mukesh Savaliya @ 2026-11-03 11:24 UTC (permalink / raw)
To: Aniket RANDIVE, Vinod Koul, Frank Li, Viken Dadhaniya, Andi Shyti
Cc: naresh.maramaina, linux-arm-msm, dmaengine, linux-kernel, linux-i2c
On 11/3/2026 4:46 PM, Aniket RANDIVE wrote:
>
>
> On 11/3/2026 4:14 PM, Mukesh Savaliya wrote:
>>
>>
>> On 11/3/2026 2:26 PM, Aniket Randive wrote:
>>> I2C transfers in GPI DMA mode can fail with -EIO when a target holds
>>> SDA low, leaving the bus stuck and preventing subsequent transactions.
>>> The GPI completion callback reports a NACK and a genuine bus error
>>> alike as -EIO, and the existing recovery path switches the SE to FIFO
>>> mode which is not valid in GPI DMA mode.
>>>
>>> Add bus recovery support using the I2C_BUS_CLEAR and I2C_STOP_ON_BUS
>>> opcodes issued via the GPI TX DMA channel. Guard the -EIO recovery
>>> trigger with a SE_GENI_IOS I2C_BUS_FREE check to skip recovery when both
>>> SDA and SCL are already released. Extend the recovery callback to
>>> dispatch
>>> through the GPI DMA path when gi2c->gpi_mode is set.
>>>
>>> Use the adapter timeout for GPI recovery commands, and preserve the DMA
>>> configuration error code when GPI setup fails.
>>>
>>> Stop recovery on a GPI command timeout and report DMA termination
>>> failures. Do not issue another command or override the timeout with a
>>> bus-idle result, since DMA cleanup may not have completed successfully.
>>>
>>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
>>> ---
>>> drivers/i2c/busses/i2c-qcom-geni.c | 107 ++++++++++++++++++++++++++
>>> + +++++-----
>>> 1 file changed, 94 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/
>>> i2c-qcom-geni.c
>>
>> [...]
>>
>>> @@ -1356,10 +1439,8 @@ static int geni_i2c_probe(struct
>>> platform_device *pdev)
>>> if (ret < 0)
>>> return ret;
>>> - if (!gi2c->gpi_mode) {
>>> - gi2c->rinfo.recover_bus = geni_i2c_recover_bus;
>>> - gi2c->adap.bus_recovery_info = &gi2c->rinfo;
>>> - }
>>> + gi2c->rinfo.recover_bus = geni_i2c_recover_bus;
>>> + gi2c->adap.bus_recovery_info = &gi2c->rinfo;
>>
>> I guess one more time this. Why patch 2 is adding and patch 3 is
>> removing the change ? keep it clean way.
>
> This is intentional. In patch 2, bus recovery is supported only in FIFO
> mode, so we guard the registration with !gi2c->gpi_mode. Patch 3 adds
> the corresponding bus recovery support for GPI mode, after which the
> mode check is no longer needed and is removed.
>
You are right, i saw that as complete logical block removal.
>>> ret = i2c_add_adapter(&gi2c->adap);
>>> if (ret) {
>>>
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] i2c: qcom-geni: Add bus recovery support for GPI DMA mode
2026-11-03 8:56 ` [PATCH v4 3/3] i2c: qcom-geni: Add bus recovery support for GPI DMA mode Aniket Randive
2026-11-03 10:44 ` Mukesh Savaliya
@ 2026-11-03 11:47 ` Mukesh Savaliya
1 sibling, 0 replies; 10+ messages in thread
From: Mukesh Savaliya @ 2026-11-03 11:47 UTC (permalink / raw)
To: Aniket Randive, Vinod Koul, Frank Li, Viken Dadhaniya, Andi Shyti
Cc: naresh.maramaina, linux-arm-msm, dmaengine, linux-kernel, linux-i2c
On 11/3/2026 2:26 PM, Aniket Randive wrote:
> I2C transfers in GPI DMA mode can fail with -EIO when a target holds
> SDA low, leaving the bus stuck and preventing subsequent transactions.
> The GPI completion callback reports a NACK and a genuine bus error
> alike as -EIO, and the existing recovery path switches the SE to FIFO
> mode which is not valid in GPI DMA mode.
>
> Add bus recovery support using the I2C_BUS_CLEAR and I2C_STOP_ON_BUS
> opcodes issued via the GPI TX DMA channel. Guard the -EIO recovery
> trigger with a SE_GENI_IOS I2C_BUS_FREE check to skip recovery when both
> SDA and SCL are already released. Extend the recovery callback to dispatch
> through the GPI DMA path when gi2c->gpi_mode is set.
>
> Use the adapter timeout for GPI recovery commands, and preserve the DMA
> configuration error code when GPI setup fails.
>
> Stop recovery on a GPI command timeout and report DMA termination
> failures. Do not issue another command or override the timeout with a
> bus-idle result, since DMA cleanup may not have completed successfully.
>
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> ---
Reviewed-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread