| Message ID | 20260826-i2c_bus_recovery-v1-2-203f0ec76f84@oss.qualcomm.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: qcom-geni: Add I2C bus recovery support | expand |
On 8/26/2026 3:21 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(). > > 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 | 109 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 109 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c > index 658636c1ee0e..9fa1a8ac400c 100644 > --- a/drivers/i2c/busses/i2c-qcom-geni.c > +++ b/drivers/i2c/busses/i2c-qcom-geni.c > @@ -138,6 +138,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 { > @@ -956,6 +957,90 @@ 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, XFER_TIMEOUT); > + if (!time_left) { > + dev_err(gi2c->se.dev, "timeout waiting for bus cmd %u\n", cmd); > + gi2c->abort_done = false; > + geni_se_abort_m_cmd(&gi2c->se); > + time_left = ABORT_TIMEOUT; > + do { > + time_left = wait_for_completion_timeout(&gi2c->done, time_left); > + } while (!gi2c->abort_done && time_left); > + > + if (!time_left) > + dev_err(gi2c->se.dev, "abort timed out for bus cmd %u\n", cmd); > + > + 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 int geni_i2c_recover_bus(struct i2c_adapter *adap) > +{ > + struct geni_i2c_dev *gi2c = i2c_get_adapdata(adap); > + 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; > + } > + > + /* SDA is high means bus is free */ What about SCL line ?> + if (readl_relaxed(gi2c->se.base + SE_GENI_IOS) & RX_DATA_IN) { > + pm_runtime_put_autosuspend(gi2c->se.dev); > + return 0; > + } > + > + 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 the slave releases SDA, so the bus > + * state is the authority: RX_DATA_IN high means the bus is free, > + * while a clean opcode status with SDA still low is a failed > + * recovery. > + */ > + if (readl_relaxed(gi2c->se.base + SE_GENI_IOS) & RX_DATA_IN) > + ret = 0; > + else if (!ret) > + ret = -EBUSY; > + > + pm_runtime_put_autosuspend(gi2c->se.dev); > + > + return ret; > +} > + > static int geni_i2c_xfer(struct i2c_adapter *adap, > struct i2c_msg msgs[], > int num) > @@ -981,6 +1066,25 @@ static int geni_i2c_xfer(struct i2c_adapter *adap, > else > ret = geni_i2c_fifo_xfer(gi2c, msgs, num); > > + if (!gi2c->gpi_mode && > + (ret == -EPROTO || ret == -ETIMEDOUT || ret == -EAGAIN)) { > + /* > + * Only attempt recovery if SDA is stuck low. -EPROTO and > + * -ETIMEDOUT indicate bus errors where the target may be > + * holding SDA 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. > + */ > + if (!(readl_relaxed(gi2c->se.base + SE_GENI_IOS) & RX_DATA_IN)) { > + 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); > + } Can we move this to a helper function like "geni_i2c_need_bus_recovery" ?> + } > + > pm_runtime_put_autosuspend(gi2c->se.dev); > gi2c->cur = NULL; > gi2c->err = 0; > @@ -1188,6 +1292,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; > + } In geni_i2c_xfer(), geni_i2c_recover_bus() is invoked directly, then why do we need this ?> + > ret = i2c_add_adapter(&gi2c->adap); > if (ret) > return dev_err_probe(dev, ret, "Error adding i2c adapter\n"); >
On 9/3/2026 9:47 PM, Jyothi Kumar Seerapu wrote: > > > On 8/26/2026 3:21 PM, Aniket Randive wrote: [...] >> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/ >> i2c-qcom-geni.c >> index 658636c1ee0e..9fa1a8ac400c 100644 >> --- a/drivers/i2c/busses/i2c-qcom-geni.c >> +++ b/drivers/i2c/busses/i2c-qcom-geni.c >> @@ -138,6 +138,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; I could not find this structure being utilized in generic way for gpi mode. So is this specific to non gpi mode ?>> }; >> struct geni_i2c_err_log { >> @@ -956,6 +957,90 @@ static int geni_i2c_fifo_xfer(struct geni_i2c_dev >> *gi2c, >> return num; >> } [...] >> + >> static int geni_i2c_xfer(struct i2c_adapter *adap, >> struct i2c_msg msgs[], >> int num) >> @@ -981,6 +1066,25 @@ static int geni_i2c_xfer(struct i2c_adapter *adap, >> else >> ret = geni_i2c_fifo_xfer(gi2c, msgs, num); >> + if (!gi2c->gpi_mode && >> + (ret == -EPROTO || ret == -ETIMEDOUT || ret == -EAGAIN)) { Could not see similar call after gpi_xfer failure which invokes recovery mode. Thinking if design can be symmetrical here for both the modes.>> + /* >> + * Only attempt recovery if SDA is stuck low. -EPROTO and >> + * -ETIMEDOUT indicate bus errors where the target may be >> + * holding SDA 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. >> + */ [...] >> @@ -1188,6 +1292,11 @@ static int geni_i2c_probe(struct >> platform_device *pdev) >> if (ret < 0) >> return ret; one line Comment here how gpi mode handles ? I would suggest to keep geni_i2c_recover_bus() as common for both. Review how we can invoke this in generic way for any mode. else geni_i2c_recover_bus() should have _fifo. >> + if (!gi2c->gpi_mode) { >> + gi2c->rinfo.recover_bus = geni_i2c_recover_bus; >> + gi2c->adap.bus_recovery_info = &gi2c->rinfo; >> + } > In geni_i2c_xfer(), geni_i2c_recover_bus() is invoked directly, then why > do we need this ?> + >> ret = i2c_add_adapter(&gi2c->adap); >> if (ret) >> return dev_err_probe(dev, ret, "Error adding i2c adapter\n"); >> >
On 9/7/2026 11:35 AM, Mukesh Savaliya wrote: > > > On 9/3/2026 9:47 PM, Jyothi Kumar Seerapu wrote: >> >> >> On 8/26/2026 3:21 PM, Aniket Randive wrote: > > [...] > >>> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/ >>> i2c-qcom-geni.c >>> index 658636c1ee0e..9fa1a8ac400c 100644 >>> --- a/drivers/i2c/busses/i2c-qcom-geni.c >>> +++ b/drivers/i2c/busses/i2c-qcom-geni.c >>> @@ -138,6 +138,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; > I could not find this structure being utilized in generic way for gpi > mode. So is this specific to non gpi mode ?>> }; >>> struct geni_i2c_err_log { >>> @@ -956,6 +957,90 @@ static int geni_i2c_fifo_xfer(struct >>> geni_i2c_dev *gi2c, >>> return num; >>> } > > [...] > >>> + >>> static int geni_i2c_xfer(struct i2c_adapter *adap, >>> struct i2c_msg msgs[], >>> int num) >>> @@ -981,6 +1066,25 @@ static int geni_i2c_xfer(struct i2c_adapter *adap, >>> else >>> ret = geni_i2c_fifo_xfer(gi2c, msgs, num); >>> + if (!gi2c->gpi_mode && >>> + (ret == -EPROTO || ret == -ETIMEDOUT || ret == -EAGAIN)) { > Could not see similar call after gpi_xfer failure which invokes recovery > mode. Thinking if design can be symmetrical here for both the modes.>> > + /* >>> + * Only attempt recovery if SDA is stuck low. -EPROTO and >>> + * -ETIMEDOUT indicate bus errors where the target may be >>> + * holding SDA 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. >>> + */ > > [...] > >>> @@ -1188,6 +1292,11 @@ static int geni_i2c_probe(struct >>> platform_device *pdev) >>> if (ret < 0) >>> return ret; > > one line Comment here how gpi mode handles ? > I would suggest to keep geni_i2c_recover_bus() as common for both. > Review how we can invoke this in generic way for any mode. > else geni_i2c_recover_bus() should have _fifo. > Sorry, please ignore this comment, as i could see patch#3 is handling same way. >>> + if (!gi2c->gpi_mode) { >>> + gi2c->rinfo.recover_bus = geni_i2c_recover_bus; >>> + gi2c->adap.bus_recovery_info = &gi2c->rinfo; >>> + } > > >> In geni_i2c_xfer(), geni_i2c_recover_bus() is invoked directly, then >> why do we need this ?> + >>> ret = i2c_add_adapter(&gi2c->adap); >>> if (ret) >>> return dev_err_probe(dev, ret, "Error adding i2c adapter\n"); >>> >> >
On 9/3/2026 9:47 PM, Jyothi Kumar Seerapu wrote: > > > On 8/26/2026 3:21 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(). >> >> 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 | 109 +++++++++++++++++++++++++++ >> ++++++++++ >> 1 file changed, 109 insertions(+) >> >> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/ >> i2c-qcom-geni.c >> index 658636c1ee0e..9fa1a8ac400c 100644 >> --- a/drivers/i2c/busses/i2c-qcom-geni.c >> +++ b/drivers/i2c/busses/i2c-qcom-geni.c >> @@ -138,6 +138,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 { >> @@ -956,6 +957,90 @@ 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, XFER_TIMEOUT); >> + if (!time_left) { >> + dev_err(gi2c->se.dev, "timeout waiting for bus cmd %u\n", cmd); >> + gi2c->abort_done = false; >> + geni_se_abort_m_cmd(&gi2c->se); >> + time_left = ABORT_TIMEOUT; >> + do { >> + time_left = wait_for_completion_timeout(&gi2c->done, >> time_left); >> + } while (!gi2c->abort_done && time_left); >> + >> + if (!time_left) >> + dev_err(gi2c->se.dev, "abort timed out for bus cmd %u\n", >> cmd); >> + >> + 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 int geni_i2c_recover_bus(struct i2c_adapter *adap) >> +{ >> + struct geni_i2c_dev *gi2c = i2c_get_adapdata(adap); >> + 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; >> + } >> + >> + /* SDA is high means bus is free */ > What about SCL line ?> + if (readl_relaxed(gi2c->se.base + > SE_GENI_IOS) & RX_DATA_IN) { Yes. I will add a check for SCL Line as well. >> + pm_runtime_put_autosuspend(gi2c->se.dev); >> + return 0; >> + } >> + >> + 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 the slave releases SDA, so the bus >> + * state is the authority: RX_DATA_IN high means the bus is free, >> + * while a clean opcode status with SDA still low is a failed >> + * recovery. >> + */ >> + if (readl_relaxed(gi2c->se.base + SE_GENI_IOS) & RX_DATA_IN) >> + ret = 0; >> + else if (!ret) >> + ret = -EBUSY; >> + >> + pm_runtime_put_autosuspend(gi2c->se.dev); >> + >> + return ret; >> +} >> + >> static int geni_i2c_xfer(struct i2c_adapter *adap, >> struct i2c_msg msgs[], >> int num) >> @@ -981,6 +1066,25 @@ static int geni_i2c_xfer(struct i2c_adapter *adap, >> else >> ret = geni_i2c_fifo_xfer(gi2c, msgs, num); >> + if (!gi2c->gpi_mode && >> + (ret == -EPROTO || ret == -ETIMEDOUT || ret == -EAGAIN)) { >> + /* >> + * Only attempt recovery if SDA is stuck low. -EPROTO and >> + * -ETIMEDOUT indicate bus errors where the target may be >> + * holding SDA 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. >> + */ >> + if (!(readl_relaxed(gi2c->se.base + SE_GENI_IOS) & >> RX_DATA_IN)) { >> + 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); >> + } > Can we move this to a helper function like > "geni_i2c_need_bus_recovery" ?> + } Yes. I will add helper function here. >> + >> pm_runtime_put_autosuspend(gi2c->se.dev); >> gi2c->cur = NULL; >> gi2c->err = 0; >> @@ -1188,6 +1292,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; >> + } > In geni_i2c_xfer(), geni_i2c_recover_bus() is invoked directly, then why > do we need this ?> + It exposes recovery through the standard I²C core API for any future caller, rather than relying on a driver-private direct call. The registration tells the I²C core that “this adapter supports recovery. call geni_i2c_recover_bus() for it.” >> ret = i2c_add_adapter(&gi2c->adap); >> if (ret) >> return dev_err_probe(dev, ret, "Error adding i2c adapter\n"); >> >
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c index 658636c1ee0e..9fa1a8ac400c 100644 --- a/drivers/i2c/busses/i2c-qcom-geni.c +++ b/drivers/i2c/busses/i2c-qcom-geni.c @@ -138,6 +138,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 { @@ -956,6 +957,90 @@ 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, XFER_TIMEOUT); + if (!time_left) { + dev_err(gi2c->se.dev, "timeout waiting for bus cmd %u\n", cmd); + gi2c->abort_done = false; + geni_se_abort_m_cmd(&gi2c->se); + time_left = ABORT_TIMEOUT; + do { + time_left = wait_for_completion_timeout(&gi2c->done, time_left); + } while (!gi2c->abort_done && time_left); + + if (!time_left) + dev_err(gi2c->se.dev, "abort timed out for bus cmd %u\n", cmd); + + 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 int geni_i2c_recover_bus(struct i2c_adapter *adap) +{ + struct geni_i2c_dev *gi2c = i2c_get_adapdata(adap); + 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; + } + + /* SDA is high means bus is free */ + if (readl_relaxed(gi2c->se.base + SE_GENI_IOS) & RX_DATA_IN) { + pm_runtime_put_autosuspend(gi2c->se.dev); + return 0; + } + + 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 the slave releases SDA, so the bus + * state is the authority: RX_DATA_IN high means the bus is free, + * while a clean opcode status with SDA still low is a failed + * recovery. + */ + if (readl_relaxed(gi2c->se.base + SE_GENI_IOS) & RX_DATA_IN) + ret = 0; + else if (!ret) + ret = -EBUSY; + + pm_runtime_put_autosuspend(gi2c->se.dev); + + return ret; +} + static int geni_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) @@ -981,6 +1066,25 @@ static int geni_i2c_xfer(struct i2c_adapter *adap, else ret = geni_i2c_fifo_xfer(gi2c, msgs, num); + if (!gi2c->gpi_mode && + (ret == -EPROTO || ret == -ETIMEDOUT || ret == -EAGAIN)) { + /* + * Only attempt recovery if SDA is stuck low. -EPROTO and + * -ETIMEDOUT indicate bus errors where the target may be + * holding SDA 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. + */ + if (!(readl_relaxed(gi2c->se.base + SE_GENI_IOS) & RX_DATA_IN)) { + 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); + } + } + pm_runtime_put_autosuspend(gi2c->se.dev); gi2c->cur = NULL; gi2c->err = 0; @@ -1188,6 +1292,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) return dev_err_probe(dev, ret, "Error adding i2c adapter\n");
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(). 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 | 109 +++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+)