Re: [PATCH v1 3/3] i2c: qcom-geni: Add bus recovery support for GPI DMA mode

[Mukesh Savaliya]

On 9/3/2026 9:44 PM, Jyothi Kumar Seerapu wrote:
[...]

> > +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;
> > +
> > +    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;
> > +
> > +    if (dmaengine_slave_config(gi2c->tx_c, &config)) {
should capture the return value and print, as it returns specific error 
code.>> +        dev_err(gi2c->se.dev, "dma config error for bus cmd 
%u\n", cmd);
> > +        return -EIO;
> > +    }
> > +

[...]

> > @@ -1066,14 +1136,18 @@ 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)) {
> > +    if (ret == -EPROTO || ret == -ETIMEDOUT || ret == -EAGAIN ||
> > +        (gi2c->gpi_mode && ret == -EIO)) {
> >           /*
> >            * 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.
> > +         * In GPI DMA mode the completion callback reports NACK,
> > +         * BUS_PROTO and ARB_LOST alike as -EIO, so the SE_GENI_IOS
> > +         * RX_DATA_IN check below is what distinguishes a genuinely
> > +         * stuck bus from a benign NACK: a NACK leaves SDA released
> > +         * (high) after the STOP, so recovery is skipped.
after the STOP, indicating i2c bus is in idle state, so.....>>            */
> >           if (!(readl_relaxed(gi2c->se.base + SE_GENI_IOS) & 
> > RX_DATA_IN)) {
> >               int recovery_ret = i2c_recover_bus(adap);
> This does not appear to handle scenarios such as an I2C multi-message 
> transfer where the stretch bit is set during the first message, but an 
> error occurs while preparing the second message (for example, during 
> memory allocation or DMA descriptor preparation). In such cases, the SCL 
> line may remain asserted low and impact subsequent transfers.
>   > @@ -1292,10 +1366,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)