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

[Praveen Talari <praveen.talari@oss.qualcomm.com>]

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