Re: [PATCH 1/3] i2c: qcom-geni: use cancel command before abort on transfer timeout

[Praveen Talari]

Hi Mukesh,

Thank you for review.

On 08-07-2026 13:05, Mukesh Savaliya wrote:
> On 7/8/2026 11:45 AM, Praveen Talari wrote:
> > 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 signalled on
> Typo :signalled
>
> "signaled when M_CMD_CANCEL_EN is received" With this , rest looks good.

Sure. Will update in next patch.

Thanks,

Praveen Talari

> > M_CMD_CANCEL_EN. 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@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Naresh Maramaina <naresh.maramaina@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Praveen Talari <praveen.talari@xxxxxxxxxxxxxxxx>
> > ---
> Reviewed-by: Mukesh Kumar Savaliya <mukesh.savaliya@xxxxxxxxxxxxxxxx>
>
> >   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);