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

[Mukesh Savaliya]

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");