* [PATCH v2 1/3] i2c: qcom-geni: use cancel command before abort on transfer timeout
2026-07-09 4:20 [PATCH v2 0/3] i2c: qcom-geni: improve transfer error recovery and synchronization Praveen Talari
@ 2026-07-09 4:20 ` Praveen Talari
2026-07-09 4:20 ` [PATCH v2 2/3] i2c: qcom-geni: use dedicated completions for abort and reset events Praveen Talari
2026-07-09 4:20 ` [PATCH v2 3/3] i2c: qcom-geni: distinguish address-phase and data-phase NACK Praveen Talari
2 siblings, 0 replies; 6+ messages in thread
From: Praveen Talari @ 2026-07-09 4:20 UTC (permalink / raw)
To: Mukesh Kumar Savaliya, Viken Dadhaniya, Andi Shyti
Cc: linux-arm-msm, linux-i2c, linux-kernel, Praveen Talari,
Naresh Maramaina
The GENI I2C driver currently invokes geni_se_abort_m_cmd() directly when
a transfer times out. However, the GENI hardware command cancellation
flow requires a cancel command to be issued first. An abort should only
be used as a fallback when the cancel operation itself fails to complete.
Introduce a dedicated cancel_done completion that is signaled when
M_CMD_CANCEL_EN is received. The timeout recovery path waits for cancel
completion and escalates to geni_i2c_abort_xfer() only if the cancel
command does not complete within the expected time.
Co-developed-by: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>
Signed-off-by: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>
Reviewed-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
drivers/i2c/busses/i2c-qcom-geni.c | 30 ++++++++++++++++++++++++++++--
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 96dbf04138be..15403edb355a 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -74,6 +74,7 @@ enum geni_i2c_err_code {
#define PACKING_BYTES_PW 4
#define ABORT_TIMEOUT HZ
+#define CANCEL_TIMEOUT HZ
#define XFER_TIMEOUT HZ
#define RST_TIMEOUT HZ
@@ -112,6 +113,7 @@ struct geni_i2c_dev {
int err;
struct i2c_adapter adap;
struct completion done;
+ struct completion cancel_done;
struct i2c_msg *cur;
int cur_wr;
int cur_rd;
@@ -361,6 +363,8 @@ static irqreturn_t geni_i2c_irq(int irq, void *dev)
dm_tx_st & TX_DMA_DONE || dm_tx_st & TX_RESET_DONE ||
dm_rx_st & RX_DMA_DONE || dm_rx_st & RX_RESET_DONE)
complete(&gi2c->done);
+ if (m_stat & M_CMD_CANCEL_EN)
+ complete(&gi2c->cancel_done);
spin_unlock(&gi2c->lock);
@@ -387,6 +391,27 @@ static void geni_i2c_abort_xfer(struct geni_i2c_dev *gi2c)
dev_err(gi2c->se.dev, "Timeout abort_m_cmd\n");
}
+static void geni_i2c_cancel_xfer(struct geni_i2c_dev *gi2c)
+{
+ unsigned long time_left = msecs_to_jiffies(CANCEL_TIMEOUT);
+ unsigned long flags;
+
+ reinit_completion(&gi2c->cancel_done);
+
+ spin_lock_irqsave(&gi2c->lock, flags);
+ if (!gi2c->err)
+ geni_i2c_err(gi2c, GENI_TIMEOUT);
+ gi2c->cur = NULL;
+ geni_se_cancel_m_cmd(&gi2c->se);
+ spin_unlock_irqrestore(&gi2c->lock, flags);
+
+ time_left = wait_for_completion_timeout(&gi2c->cancel_done, time_left);
+ if (!time_left) {
+ dev_err(gi2c->se.dev, "Timeout cancel_m_cmd\n");
+ geni_i2c_abort_xfer(gi2c);
+ }
+}
+
static void geni_i2c_rx_fsm_rst(struct geni_i2c_dev *gi2c)
{
u32 val;
@@ -473,7 +498,7 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
cur = gi2c->cur;
time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
if (!time_left)
- geni_i2c_abort_xfer(gi2c);
+ geni_i2c_cancel_xfer(gi2c);
geni_i2c_rx_msg_cleanup(gi2c, cur);
@@ -515,7 +540,7 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
cur = gi2c->cur;
time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
if (!time_left)
- geni_i2c_abort_xfer(gi2c);
+ geni_i2c_cancel_xfer(gi2c);
geni_i2c_tx_msg_cleanup(gi2c, cur);
@@ -1107,6 +1132,7 @@ static int geni_i2c_probe(struct platform_device *pdev)
gi2c->adap.algo = &geni_i2c_algo;
init_completion(&gi2c->done);
+ init_completion(&gi2c->cancel_done);
spin_lock_init(&gi2c->lock);
platform_set_drvdata(pdev, gi2c);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/3] i2c: qcom-geni: use dedicated completions for abort and reset events
2026-07-09 4:20 [PATCH v2 0/3] i2c: qcom-geni: improve transfer error recovery and synchronization Praveen Talari
2026-07-09 4:20 ` [PATCH v2 1/3] i2c: qcom-geni: use cancel command before abort on transfer timeout Praveen Talari
@ 2026-07-09 4:20 ` Praveen Talari
2026-07-10 6:28 ` Mukesh Savaliya
2026-07-09 4:20 ` [PATCH v2 3/3] i2c: qcom-geni: distinguish address-phase and data-phase NACK Praveen Talari
2 siblings, 1 reply; 6+ messages in thread
From: Praveen Talari @ 2026-07-09 4:20 UTC (permalink / raw)
To: Mukesh Kumar Savaliya, Viken Dadhaniya, Andi Shyti
Cc: linux-arm-msm, linux-i2c, linux-kernel, Praveen Talari,
Naresh Maramaina
The driver uses a common completion event for transfer, abort and DMA
reset operations. This allows unrelated completion events to
prematurely wake abort and reset waiters, leading to incorrect
synchronization.
Introduce dedicated completions for abort, TX reset, and RX reset
operations, and signal them only from their respective interrupt
events. This removes the dependency on shared completion state and
eliminates the abort_done flag-based synchronization.
Co-developed-by: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>
Signed-off-by: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
drivers/i2c/busses/i2c-qcom-geni.c | 54 +++++++++++++++++++-------------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 15403edb355a..9490aee4928c 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -113,7 +113,10 @@ struct geni_i2c_dev {
int err;
struct i2c_adapter adap;
struct completion done;
+ struct completion abort_done;
struct completion cancel_done;
+ struct completion tx_reset_done;
+ struct completion rx_reset_done;
struct i2c_msg *cur;
int cur_wr;
int cur_rd;
@@ -127,7 +130,6 @@ struct geni_i2c_dev {
struct dma_chan *rx_c;
bool no_dma;
bool gpi_mode;
- bool abort_done;
bool is_tx_multi_desc_xfer;
u32 num_msgs;
struct geni_i2c_gpi_multi_desc_xfer i2c_multi_desc_config;
@@ -256,8 +258,6 @@ static void geni_i2c_err(struct geni_i2c_dev *gi2c, int err)
switch (err) {
case GENI_ABORT_DONE:
- gi2c->abort_done = true;
- break;
case NACK:
case GENI_TIMEOUT:
dev_dbg(gi2c->se.dev, "%s\n", gi2c_log[err].msg);
@@ -359,12 +359,18 @@ static irqreturn_t geni_i2c_irq(int irq, void *dev)
writel_relaxed(dm_rx_st, base + SE_DMA_RX_IRQ_CLR);
/* if this is err with done-bit not set, handle that through timeout. */
- if (m_stat & M_CMD_DONE_EN || m_stat & M_CMD_ABORT_EN ||
- dm_tx_st & TX_DMA_DONE || dm_tx_st & TX_RESET_DONE ||
- dm_rx_st & RX_DMA_DONE || dm_rx_st & RX_RESET_DONE)
+ if (m_stat & M_CMD_DONE_EN ||
+ dm_tx_st & TX_DMA_DONE ||
+ dm_rx_st & RX_DMA_DONE)
complete(&gi2c->done);
if (m_stat & M_CMD_CANCEL_EN)
complete(&gi2c->cancel_done);
+ if (m_stat & M_CMD_ABORT_EN)
+ complete(&gi2c->abort_done);
+ if (dm_tx_st & TX_RESET_DONE)
+ complete(&gi2c->tx_reset_done);
+ if (dm_rx_st & RX_RESET_DONE)
+ complete(&gi2c->rx_reset_done);
spin_unlock(&gi2c->lock);
@@ -376,17 +382,13 @@ static void geni_i2c_abort_xfer(struct geni_i2c_dev *gi2c)
unsigned long time_left = ABORT_TIMEOUT;
unsigned long flags;
+ reinit_completion(&gi2c->abort_done);
+
spin_lock_irqsave(&gi2c->lock, flags);
- geni_i2c_err(gi2c, GENI_TIMEOUT);
- gi2c->cur = NULL;
- gi2c->abort_done = false;
geni_se_abort_m_cmd(&gi2c->se);
spin_unlock_irqrestore(&gi2c->lock, flags);
- do {
- time_left = wait_for_completion_timeout(&gi2c->done, time_left);
- } while (!gi2c->abort_done && time_left);
-
+ time_left = wait_for_completion_timeout(&gi2c->abort_done, time_left);
if (!time_left)
dev_err(gi2c->se.dev, "Timeout abort_m_cmd\n");
}
@@ -414,31 +416,25 @@ static void geni_i2c_cancel_xfer(struct geni_i2c_dev *gi2c)
static void geni_i2c_rx_fsm_rst(struct geni_i2c_dev *gi2c)
{
- u32 val;
unsigned long time_left = RST_TIMEOUT;
+ reinit_completion(&gi2c->rx_reset_done);
writel_relaxed(1, gi2c->se.base + SE_DMA_RX_FSM_RST);
- do {
- time_left = wait_for_completion_timeout(&gi2c->done, time_left);
- val = readl_relaxed(gi2c->se.base + SE_DMA_RX_IRQ_STAT);
- } while (!(val & RX_RESET_DONE) && time_left);
- if (!(val & RX_RESET_DONE))
+ time_left = wait_for_completion_timeout(&gi2c->rx_reset_done, time_left);
+ if (!time_left)
dev_err(gi2c->se.dev, "Timeout resetting RX_FSM\n");
}
static void geni_i2c_tx_fsm_rst(struct geni_i2c_dev *gi2c)
{
- u32 val;
unsigned long time_left = RST_TIMEOUT;
+ reinit_completion(&gi2c->tx_reset_done);
writel_relaxed(1, gi2c->se.base + SE_DMA_TX_FSM_RST);
- do {
- time_left = wait_for_completion_timeout(&gi2c->done, time_left);
- val = readl_relaxed(gi2c->se.base + SE_DMA_TX_IRQ_STAT);
- } while (!(val & TX_RESET_DONE) && time_left);
- if (!(val & TX_RESET_DONE))
+ time_left = wait_for_completion_timeout(&gi2c->tx_reset_done, time_left);
+ if (!time_left)
dev_err(gi2c->se.dev, "Timeout resetting TX_FSM\n");
}
@@ -851,6 +847,7 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], i
for (i = 0; i < num; i++) {
gi2c->cur = &msgs[i];
gi2c->err = 0;
+ reinit_completion(&gi2c->done);
dev_dbg(gi2c->se.dev, "msg[%d].len:%d\n", i, gi2c->cur->len);
peripheral.stretch = 0;
@@ -920,6 +917,8 @@ static int geni_i2c_fifo_xfer(struct geni_i2c_dev *gi2c,
m_param |= ((msgs[i].addr << SLV_ADDR_SHFT) & SLV_ADDR_MSK);
gi2c->cur = &msgs[i];
+ gi2c->err = 0;
+ reinit_completion(&gi2c->done);
if (msgs[i].flags & I2C_M_RD)
ret = geni_i2c_rx_one_msg(gi2c, &msgs[i], m_param);
else
@@ -939,8 +938,6 @@ static int geni_i2c_xfer(struct i2c_adapter *adap,
struct geni_i2c_dev *gi2c = i2c_get_adapdata(adap);
int ret;
- gi2c->err = 0;
- reinit_completion(&gi2c->done);
ret = pm_runtime_get_sync(gi2c->se.dev);
if (ret < 0) {
dev_err(gi2c->se.dev, "error turning SE resources:%d\n", ret);
@@ -1132,7 +1129,10 @@ static int geni_i2c_probe(struct platform_device *pdev)
gi2c->adap.algo = &geni_i2c_algo;
init_completion(&gi2c->done);
+ init_completion(&gi2c->abort_done);
init_completion(&gi2c->cancel_done);
+ init_completion(&gi2c->tx_reset_done);
+ init_completion(&gi2c->rx_reset_done);
spin_lock_init(&gi2c->lock);
platform_set_drvdata(pdev, gi2c);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/3] i2c: qcom-geni: use dedicated completions for abort and reset events
2026-07-09 4:20 ` [PATCH v2 2/3] i2c: qcom-geni: use dedicated completions for abort and reset events Praveen Talari
@ 2026-07-10 6:28 ` Mukesh Savaliya
0 siblings, 0 replies; 6+ messages in thread
From: Mukesh Savaliya @ 2026-07-10 6:28 UTC (permalink / raw)
To: Praveen Talari, Viken Dadhaniya, Andi Shyti
Cc: linux-arm-msm, linux-i2c, linux-kernel, Naresh Maramaina
On 7/9/2026 9:50 AM, Praveen Talari wrote:
> The driver uses a common completion event for transfer, abort and DMA
> reset operations. This allows unrelated completion events to
> prematurely wake abort and reset waiters, leading to incorrect
> synchronization.
>
> Introduce dedicated completions for abort, TX reset, and RX reset
> operations, and signal them only from their respective interrupt
> events. This removes the dependency on shared completion state and
> eliminates the abort_done flag-based synchronization.
>
> Co-developed-by: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>
> Signed-off-by: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---
> drivers/i2c/busses/i2c-qcom-geni.c | 54 +++++++++++++++++++-------------------
> 1 file changed, 27 insertions(+), 27 deletions(-)
>
Reviewed-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] i2c: qcom-geni: distinguish address-phase and data-phase NACK
2026-07-09 4:20 [PATCH v2 0/3] i2c: qcom-geni: improve transfer error recovery and synchronization Praveen Talari
2026-07-09 4:20 ` [PATCH v2 1/3] i2c: qcom-geni: use cancel command before abort on transfer timeout Praveen Talari
2026-07-09 4:20 ` [PATCH v2 2/3] i2c: qcom-geni: use dedicated completions for abort and reset events Praveen Talari
@ 2026-07-09 4:20 ` Praveen Talari
2026-07-10 11:36 ` Mukesh Savaliya
2 siblings, 1 reply; 6+ messages in thread
From: Praveen Talari @ 2026-07-09 4:20 UTC (permalink / raw)
To: Mukesh Kumar Savaliya, Viken Dadhaniya, Andi Shyti
Cc: linux-arm-msm, linux-i2c, linux-kernel, Praveen Talari,
Naresh Maramaina
The M_GP_IRQ_1 interrupt signals a NACK condition, but does not
distinguish whether it occurred during the address phase or the data
phase. The driver always attempted cancel and DMA FSM reset on any
NACK, which is incorrect for an address NACK since the DMA engine was
never armed and the hardware requires no recovery.
Add geni_i2c_check_addr_data_nack() to distinguish the two cases by
reading SE_GENI_M_GP_LENGTH after the NACK event. A zero value means
the address phase was NACKed (ADDR_NACK) and no DMA recovery is needed.
A non-zero value on a write transfer means a data byte was NACKed
(DATA_NACK) and the normal cancel and DMA FSM reset path must run.
Co-developed-by: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>
Signed-off-by: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
drivers/i2c/busses/i2c-qcom-geni.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 9490aee4928c..b1cd027b5ff2 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -56,7 +56,8 @@
enum geni_i2c_err_code {
GP_IRQ0,
- NACK,
+ ADDR_NACK,
+ DATA_NACK,
GP_IRQ2,
BUS_PROTO,
ARB_LOST,
@@ -67,7 +68,7 @@ enum geni_i2c_err_code {
GENI_TIMEOUT,
};
-#define DM_I2C_CB_ERR ((BIT(NACK) | BIT(BUS_PROTO) | BIT(ARB_LOST)) \
+#define DM_I2C_CB_ERR ((BIT(ADDR_NACK) | BIT(BUS_PROTO) | BIT(ARB_LOST)) \
<< 5)
#define I2C_AUTO_SUSPEND_DELAY 250
@@ -143,7 +144,8 @@ struct geni_i2c_err_log {
static const struct geni_i2c_err_log gi2c_log[] = {
[GP_IRQ0] = {-EIO, "Unknown I2C err GP_IRQ0"},
- [NACK] = {-ENXIO, "NACK: slv unresponsive, check its power/reset-ln"},
+ [ADDR_NACK] = {-ENXIO, "NACK: slv unresponsive, check its power/reset-ln"},
+ [DATA_NACK] = {-EIO, "Data NACK: device NACK before end of TX"},
[GP_IRQ2] = {-EIO, "Unknown I2C err GP IRQ2"},
[BUS_PROTO] = {-EPROTO, "Bus proto err, noisy/unexpected start/stop"},
[ARB_LOST] = {-EAGAIN, "Bus arbitration lost, clock line undriveable"},
@@ -258,7 +260,8 @@ static void geni_i2c_err(struct geni_i2c_dev *gi2c, int err)
switch (err) {
case GENI_ABORT_DONE:
- case NACK:
+ case ADDR_NACK:
+ case DATA_NACK:
case GENI_TIMEOUT:
dev_dbg(gi2c->se.dev, "%s\n", gi2c_log[err].msg);
break;
@@ -269,6 +272,14 @@ static void geni_i2c_err(struct geni_i2c_dev *gi2c, int err)
}
}
+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))
+ geni_i2c_err(gi2c, DATA_NACK);
+}
+
static irqreturn_t geni_i2c_irq(int irq, void *dev)
{
struct geni_i2c_dev *gi2c = dev;
@@ -294,7 +305,7 @@ static irqreturn_t geni_i2c_irq(int irq, void *dev)
m_stat & (M_CMD_FAILURE_EN | M_CMD_ABORT_EN) ||
dm_rx_st & (DM_I2C_CB_ERR)) {
if (m_stat & M_GP_IRQ_1_EN)
- geni_i2c_err(gi2c, NACK);
+ geni_i2c_check_addr_data_nack(gi2c);
if (m_stat & M_GP_IRQ_3_EN)
geni_i2c_err(gi2c, BUS_PROTO);
if (m_stat & M_GP_IRQ_4_EN)
@@ -443,7 +454,7 @@ static void geni_i2c_rx_msg_cleanup(struct geni_i2c_dev *gi2c,
{
gi2c->cur_rd = 0;
if (gi2c->dma_buf) {
- if (gi2c->err)
+ if (gi2c->err && gi2c->err != gi2c_log[ADDR_NACK].err)
geni_i2c_rx_fsm_rst(gi2c);
geni_se_rx_dma_unprep(&gi2c->se, gi2c->dma_addr, gi2c->xfer_len);
i2c_put_dma_safe_msg_buf(gi2c->dma_buf, cur, !gi2c->err);
@@ -455,7 +466,7 @@ static void geni_i2c_tx_msg_cleanup(struct geni_i2c_dev *gi2c,
{
gi2c->cur_wr = 0;
if (gi2c->dma_buf) {
- if (gi2c->err)
+ if (gi2c->err && gi2c->err != gi2c_log[ADDR_NACK].err)
geni_i2c_tx_fsm_rst(gi2c);
geni_se_tx_dma_unprep(&gi2c->se, gi2c->dma_addr, gi2c->xfer_len);
i2c_put_dma_safe_msg_buf(gi2c->dma_buf, cur, !gi2c->err);
@@ -493,7 +504,7 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
cur = gi2c->cur;
time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
- if (!time_left)
+ if (!time_left || (gi2c->err && gi2c->err != gi2c_log[ADDR_NACK].err))
geni_i2c_cancel_xfer(gi2c);
geni_i2c_rx_msg_cleanup(gi2c, cur);
@@ -535,7 +546,7 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
cur = gi2c->cur;
time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
- if (!time_left)
+ if (!time_left || (gi2c->err && gi2c->err != gi2c_log[ADDR_NACK].err))
geni_i2c_cancel_xfer(gi2c);
geni_i2c_tx_msg_cleanup(gi2c, cur);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 3/3] i2c: qcom-geni: distinguish address-phase and data-phase NACK
2026-07-09 4:20 ` [PATCH v2 3/3] i2c: qcom-geni: distinguish address-phase and data-phase NACK Praveen Talari
@ 2026-07-10 11:36 ` Mukesh Savaliya
0 siblings, 0 replies; 6+ messages in thread
From: Mukesh Savaliya @ 2026-07-10 11:36 UTC (permalink / raw)
To: Praveen Talari, Viken Dadhaniya, Andi Shyti
Cc: linux-arm-msm, linux-i2c, linux-kernel, Naresh Maramaina
On 7/9/2026 9:50 AM, Praveen Talari wrote:
> The M_GP_IRQ_1 interrupt signals a NACK condition, but does not
> distinguish whether it occurred during the address phase or the data
> phase. The driver always attempted cancel and DMA FSM reset on any
> NACK, which is incorrect for an address NACK since the DMA engine was
> never armed and the hardware requires no recovery.
>
> Add geni_i2c_check_addr_data_nack() to distinguish the two cases by
> reading SE_GENI_M_GP_LENGTH after the NACK event. A zero value means
> the address phase was NACKed (ADDR_NACK) and no DMA recovery is needed.
> A non-zero value on a write transfer means a data byte was NACKed
> (DATA_NACK) and the normal cancel and DMA FSM reset path must run.
>
> Co-developed-by: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>
> Signed-off-by: Naresh Maramaina <naresh.maramaina@oss.qualcomm.com>
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---
> drivers/i2c/busses/i2c-qcom-geni.c | 29 ++++++++++++++++++++---------
> 1 file changed, 20 insertions(+), 9 deletions(-)
[...]
> #define I2C_AUTO_SUSPEND_DELAY 250
> @@ -143,7 +144,8 @@ struct geni_i2c_err_log {
>
> static const struct geni_i2c_err_log gi2c_log[] = {
> [GP_IRQ0] = {-EIO, "Unknown I2C err GP_IRQ0"},
> - [NACK] = {-ENXIO, "NACK: slv unresponsive, check its power/reset-ln"},
> + [ADDR_NACK] = {-ENXIO, "NACK: slv unresponsive, check its power/reset-ln"},
Now slv could be target device, follow latest convention.
> + [DATA_NACK] = {-EIO, "Data NACK: device NACK before end of TX"},
Minor: Instead of before end of TX, could be simply TX transfer NACK ?
> [GP_IRQ2] = {-EIO, "Unknown I2C err GP IRQ2"},
> [BUS_PROTO] = {-EPROTO, "Bus proto err, noisy/unexpected start/stop"},
> [ARB_LOST] = {-EAGAIN, "Bus arbitration lost, clock line undriveable"},
> @@ -258,7 +260,8 @@ static void geni_i2c_err(struct geni_i2c_dev *gi2c, int err)
>
[...]
^ permalink raw reply [flat|nested] 6+ messages in thread