Re: [PATCH v7 2/2] i2c: qcom-geni: Add dynamic transfer timeout based on transfer length and frequency

[Aniket RANDIVE]

Thanks Mukesh for the review.

On 8/24/2026 12:41 PM, Mukesh Savaliya wrote:
> On 8/13/2026 11:15 AM, Aniket Randive wrote:
>
> [...]
>
> >   static int geni_i2c_gpi_multi_xfer_timeout_handler(struct device *dev,
> >                              struct geni_i2c_gpi_multi_desc_xfer 
> > *multi_xfer,
> > -                           u32 transfer_timeout_msecs,
> > +                           unsigned long timeout_jiffies,
> >                              struct completion *transfer_comp)
> >   {
> >       int i;
> > -    u32 time_left;
> > +    unsigned long time_left;
> >       for (i = 0; i < multi_xfer->msg_idx_cnt - 1; i++) {
> >           reinit_completion(transfer_comp);
> >           if (multi_xfer->msg_idx_cnt != multi_xfer->irq_cnt) {
> >               time_left = wait_for_completion_timeout(transfer_comp,
> > -                                transfer_timeout_msecs);
> > +                                timeout_jiffies);
> >               if (!time_left) {
> >                   dev_err(dev, "%s: Transfer timeout\n", __func__);
> >                   return -ETIMEDOUT;
> > @@ -778,8 +787,22 @@ static int geni_i2c_gpi(struct geni_i2c_dev 
> > *gi2c, struct i2c_msg msgs[],
> >           dma_async_issue_pending(gi2c->tx_c);
> >           if ((msg_idx == (gi2c->num_msgs - 1)) || flags & 
> > DMA_PREP_INTERRUPT) {
> > +            size_t max_len = 0;
> > +            int j;
> > +
> > +            /*
> > +             * Use the longest message as the timeout base: one 
> > completion
> > +             * covers the whole BEI batch, so the budget must fit the 
> > worst
> > +             * case single-message wire time.
> > +             */
>
> Using only the longest message length may under-estimate the required 
> Timeout.

Yes. We can take sum of all messages in GPI multi desc path. I will do 
this change in next patch.

> > +            for (j = 0; j < gi2c->num_msgs; j++)
> > +                max_len = max_t(size_t, max_len, msgs[j].len);
> can we use sum(msgs[i].len) instead of max() ?> + 
> i2c_update_timeout(&gi2c->adap, gi2c->clk_freq_out, max_len,
> > +                       I2C_TIMEOUT_SAFETY_COEFFICIENT,
> > +                       I2C_TIMEOUT_MIN_USEC);
> >               ret = geni_i2c_gpi_multi_xfer_timeout_handler(gi2c- 
> > >se.dev, gi2c_gpi_xfer,
> > -                                      XFER_TIMEOUT, &gi2c->done);
> > +                                      gi2c->adap.timeout,
> > +                                      &gi2c->done);
> >               if (ret) {
> >                   dev_err(gi2c->se.dev,
> >                       "I2C multi write msg transfer timeout: %d\n",
> > @@ -899,7 +922,10 @@ static int geni_i2c_gpi_xfer(struct geni_i2c_dev 
> > *gi2c, struct i2c_msg msgs[], i
> >           if (!gi2c->is_tx_multi_desc_xfer) {
> >               dma_async_issue_pending(gi2c->tx_c);
> > -            time_left = wait_for_completion_timeout(&gi2c->done, 
> > XFER_TIMEOUT);
> > +            i2c_update_timeout(&gi2c->adap, gi2c->clk_freq_out, 
> > msgs[i].len,
> > +                       I2C_TIMEOUT_SAFETY_COEFFICIENT,
> > +                       I2C_TIMEOUT_MIN_USEC);
> > +            time_left = wait_for_completion_timeout(&gi2c->done, 
> > gi2c->adap.timeout);
> >               if (!time_left) {
> >                   dev_err(gi2c->se.dev, "%s:I2C timeout\n", __func__);
> >                   gi2c->err = -ETIMEDOUT;