Re: [PATCH V3] i2c: qcom-geni: Avoid extra TX DMA TRE for single read message in GPI mode

[Aniket RANDIVE <aniket.randive@oss.qualcomm.com>]

On 4/6/2026 10:08 AM, Mukesh Savaliya wrote:
> 
> 
> On 4/3/2026 2:11 PM, Aniket Randive wrote:
>> In GPI mode, the I2C GENI driver programs an extra TX DMA transfer
>> descriptor (TRE) on the TX channel when handling a single read message.
>> This results in an unintended write phase being issued on the I2C bus,
>> even though a read transaction does not require any TX data.
>>
>> For a single-byte read, the correct hardware sequence consists of the
>> CONFIG and GO commands followed by a single RX DMA TRE. Programming an
>> additional TX DMA TRE is redundant, causes unnecessary DMA buffer
>> mapping on the TX channel, and may lead to incorrect bus behavior.
>>
>> Update the transfer logic to avoid programming a TX DMA TRE for single
>> read messages in GPI mode.
>>
>> Co-developed-by: Maramaina Naresh <naresh.maramaina@oss.qualcomm.com>
>> Signed-off-by: Maramaina Naresh <naresh.maramaina@oss.qualcomm.com>
>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
>> ---
>> Changes in v3:
>>    - Added comment in the driver for better readability and changed the
>>      position of 'skip_dma' label to allow dma engine configuration.
>>
>> Changes in v2:
>>    - Updated the commit message.
>>
>>   drivers/i2c/busses/i2c-qcom-geni.c | 20 +++++++++++++++-----
>>   1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/ 
>> i2c-qcom-geni.c
>> index a4acb78fafb6..78b92db7c7fd 100644
>> --- a/drivers/i2c/busses/i2c-qcom-geni.c
>> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
>> @@ -625,8 +625,8 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, 
>> struct i2c_msg msgs[],
>>   {
>>       struct gpi_i2c_config *peripheral;
>>       unsigned int flags;
>> -    void *dma_buf;
>> -    dma_addr_t addr;
>> +    void *dma_buf = NULL;
>> +    dma_addr_t addr = 0;
>>       enum dma_data_direction map_dirn;
>>       enum dma_transfer_direction dma_dirn;
>>       struct dma_async_tx_descriptor *desc;
>> @@ -639,6 +639,12 @@ static int geni_i2c_gpi(struct geni_i2c_dev 
>> *gi2c, struct i2c_msg msgs[],
>>       gi2c_gpi_xfer = &gi2c->i2c_multi_desc_config;
>>       msg_idx = gi2c_gpi_xfer->msg_idx_cnt;
>> +    /* Skip TX DMA map for I2C_WRITE operation to avoid unintended 
>> write cycle */
> Seems you are missing writing important point - For read message ?
> Important is to clarity what's the condition we are handling, Skipping 
> something is anyway clear from goto skip_dma.

Sure, will add more description in comment for better understanding.
-Aniket

>> +    if (op == I2C_WRITE && msgs[msg_idx].flags & I2C_M_RD) {
>> +        peripheral->multi_msg = true;
>> +        goto skip_dma;
>> +    }
>> +
>>       dma_buf = i2c_get_dma_safe_msg_buf(&msgs[msg_idx], 1);
>>       if (!dma_buf) {
>>           ret = -ENOMEM;
>> @@ -658,6 +664,7 @@ static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, 
>> struct i2c_msg msgs[],
>>           goto out;
>>       }
>> +skip_dma:
> Better name - skip_tx_dma_map ?

Sure. I will change it.
-Aniket

>>       if (gi2c->is_tx_multi_desc_xfer) {
>>           flags = DMA_CTRL_ACK;
>> @@ -740,9 +747,12 @@ static int geni_i2c_gpi(struct geni_i2c_dev 
>> *gi2c, struct i2c_msg msgs[],
>>       return 0;
>>   err_config:
>> -    dma_unmap_single(gi2c->se.dev->parent, addr,
>> -             msgs[msg_idx].len, map_dirn);
>> -    i2c_put_dma_safe_msg_buf(dma_buf, &msgs[msg_idx], false);
>> +    /* Avoid DMA unmap as the write operation skipped DMA mapping */
>> +    if (dma_buf) {
>> +        dma_unmap_single(gi2c->se.dev->parent, addr,
>> +                 msgs[msg_idx].len, map_dirn);
>> +        i2c_put_dma_safe_msg_buf(dma_buf, &msgs[msg_idx], false);
>> +    }
>>   out:
>>       gi2c->err = ret;
>