Re: [PATCH 1/2] dmaengine: qcom-gpi: Add I2C High-Speed mode configuration support

[Jyothi Kumar Seerapu <jyothi.seerapu@oss.qualcomm.com>]

On 8/28/2026 12:56 AM, Mukesh Savaliya wrote:
> 
> 
> On 8/24/2026 4:55 PM, Jyothi Kumar Seerapu wrote:
>> Add support in the Qualcomm GPI (Generic Packet Interface) DMA engine
>> to configure I2C High-Speed (HS) mode transfers.
>>
>> Introduce support for CONFIG1 Transfer Ring Element (TRE) to convey
>> HS-specific timing parameters to the hardware. Define a new
>> gpi_i2c_config1 structure containing tcycle_cnt and tlow_cnt fields,
>> with default values of 28 and 38 respectively, as required for 3.4 MHz
>> HS mode operation.
>>
> Little more enhanced -
> 
> dmaengine: qcom: gpi: Add I2C High-Speed mode frequency support
> 
> Introduce CONFIG1 Transfer Ring Element (TRE) support to program
> I2C High-Speed (HS) mode timing parameters through the Qualcomm
> GPI DMA engine.
> 
> Add a new gpi_i2c_config1 structure containing tcycle_cnt and
> tlow_cnt fields. The fields are initialized to the hardware
> recommended values of 28 and 38, enabling 3.4 MHz HS-mode I2C
> transfers.
> 
Sure will review and update in V2.>
>> Signed-off-by: Jyothi Kumar Seerapu <jyothi.seerapu@oss.qualcomm.com>
>> ---
>>   drivers/dma/qcom/gpi.c           | 54 ++++++++++++++++++++++++++++++ 
>> +++++-----
>>   include/linux/dma/qcom-gpi-dma.h | 28 ++++++++++++++++++++-
>>   2 files changed, 75 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
>> index a5055a6273af..52a7800ed4f5 100644
>> --- a/drivers/dma/qcom/gpi.c
>> +++ b/drivers/dma/qcom/gpi.c
>> @@ -21,6 +21,7 @@
>>   #define TRE_TYPE_IMMEDIATE_DMA    0x11
>>   #define TRE_TYPE_GO        0x20
>>   #define TRE_TYPE_CONFIG0    0x22
>> +#define TRE_TYPE_CONFIG1    0x23
>>   /* TRE flags */
>>   #define TRE_FLAGS_CHAIN        BIT(0)
>> @@ -58,9 +59,14 @@
>>   #define TRE_I2C_C0_TX_PACK    BIT(24)
>>   #define TRE_I2C_C0_RX_PACK      BIT(25)
>> +/* I2C Config1 WD0 */
>> +#define TRE_I2C_C1_TLOW        GENMASK(9, 0)
>> +#define TRE_I2C_C1_TCYCLE    GENMASK(19, 10)
>> +
> 
> Not sure why above two not looking aligned by tab
> 
>>   /* I2C GO WD0 */
>>   #define TRE_I2C_GO_CMD          GENMASK(4, 0)
> 
> [...]
> 
>> -    /* create the GO tre for Tx */
>> -    if (i2c->op == I2C_WRITE) {
>> +    /* Create CONFIG1 TRE if requested */
>> +    if (i2c->set_config1) {
> can we move this set_config1 into else if part of set_config, above ?> 
>          tre = &desc->tre[tre_idx];
>>           tre_idx++;
>> +        /* CONFIG1 TRE with timing parameters */
>> +        tre->dword[0] = u32_encode_bits(i2c->config1.tlow_cnt, 
>> TRE_I2C_C1_TLOW);
>> +        tre->dword[0] |= u32_encode_bits(i2c->config1.tcycle_cnt, 
>> TRE_I2C_C1_TCYCLE);
>> +        tre->dword[1] = 0;
>> +        tre->dword[2] = 0;
>> +        tre->dword[3] = u32_encode_bits(TRE_TYPE_CONFIG1, 
>> TRE_FLAGS_TYPE);
>> +        tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_CHAIN);
>> +    }
>> +
>> +    /* create the GO tre for Tx */
>> +    if (i2c->op == I2C_WRITE || i2c->op == I2C_HS_WRITE) {
>> +        u8 master_code = 0; /* Default master code for Linux EE */
> why for Linux EE ? what else could be ?> +        u32 cmd_opcode;
>> +        bool is_hs_mode = false;
>> +
>> +        tre = &desc->tre[tre_idx];
>> +        tre_idx++;
>> +
>> +        is_hs_mode = (i2c->op == I2C_HS_WRITE || i2c->op == 
>> I2C_HS_READ);
>> +
>> +        /* Determine the command opcode based on HS mode and 
>> multi_msg flag */
> Comment is mostly restates the code, would write as -
> /* Select HS-mode or standard I2C opcode. */
>>           if (i2c->multi_msg)
>> -            tre->dword[0] = u32_encode_bits(I2C_READ, TRE_I2C_GO_CMD);
>> +            cmd_opcode = is_hs_mode ? I2C_HS_READ : I2C_READ;
> can you add explanation of cmd opcode into commit log too ? That would 
> make story describing changes.>           else
>> -            tre->dword[0] = u32_encode_bits(i2c->op, TRE_I2C_GO_CMD);
>> +            /* I2C HS write vs Regular I2C write */
>> +            cmd_opcode = is_hs_mode ? I2C_HS_WRITE : i2c->op;
>> +
> what if i write as below, will be simplified ?
> i2c->op = cmd_opcode ; //OR can store directly into i2c->op.
> tre->dword[0] = u32_encode_bits(i2c->op, TRE_I2C_GO_CMD);
> 
>> +        tre->dword[0] = u32_encode_bits(cmd_opcode, TRE_I2C_GO_CMD);
>> +
>> +        if (is_hs_mode)
>> +            tre->dword[0] |= u32_encode_bits(master_code, 
>> TRE_I2C_GO_MASTER_CODE);
>>           tre->dword[0] |= u32_encode_bits(i2c->addr, TRE_I2C_GO_ADDR);
>>           tre->dword[0] |= u32_encode_bits(i2c->stretch, 
>> TRE_I2C_GO_STRETCH);
>> @@ -1674,7 +1708,7 @@ static int gpi_create_i2c_tre(struct gchan 
>> *chan, struct gpi_desc *desc,
>>               tre->dword[3] |= u32_encode_bits(1, TRE_FLAGS_CHAIN);
>>       }
>> -    if (i2c->op == I2C_READ || i2c->multi_msg == false) {
>> +    if (i2c->op == I2C_READ || i2c->op == I2C_HS_READ || i2c- 
>> >multi_msg == false) {
>>           /* create the DMA TRE */
>>           tre = &desc->tre[tre_idx];
>>           tre_idx++;
>> @@ -1826,6 +1860,14 @@ gpi_prep_slave_sg(struct dma_chan *chan, struct 
>> scatterlist *sgl,
>>       if (direction == DMA_DEV_TO_MEM) /* rx */
>>           nr_tre = 1;
>> +    /* I2C High-Speed mode sends an extra CONFIG1 TRE ahead of the GO 
>> TRE */
>> +    if (gchan->protocol == QCOM_GPI_I2C) {
>> +        struct gpi_i2c_config *i2c = gchan->config;
>> +
>> +        if (i2c->set_config1)
>> +            nr_tre++;
>> +    }
>> +
>>       /* calculate # of elements required & available */
>>       nr = gpi_ring_num_elements_avail(ch_ring);
>>       if (nr < nr_tre) {
>> diff --git a/include/linux/dma/qcom-gpi-dma.h b/include/linux/dma/ 
>> qcom-gpi-dma.h
>> index 332be28427e4..6e9f09fc109a 100644
>> --- a/include/linux/dma/qcom-gpi-dma.h
>> +++ b/include/linux/dma/qcom-gpi-dma.h
> 
> [...]
> 
>>   /**
>> @@ -62,15 +82,19 @@ enum i2c_op {
>>    * @high_count: high period of clock
>>    * @low_count: low period of clock
>>    * @clk_div: source clock divider
>> + * @clk_src: serial clock
> source clock right ?
yes, its source clock.>    * @addr: i2c bus address
>>    * @stretch: stretch the clock at eot
>> - * @set_config: set peripheral config
>> + * @set_config: set peripheral config (CONFIG0)
>> + * @set_config1: set peripheral config1 (CONFIG1)
>> + * @config1: I2C HS mode timing configuration (CONFIG1 TRE parameters)
>>    * @rx_len: receive length for buffer
>>    * @op: i2c cmd
>>    * @multi_msg: is part of multi i2c r-w msgs
>>    */
>>   struct gpi_i2c_config {
>>       u8 set_config;
>> +    u8 set_config1;
>>       u8 pack_enable;
>>       u8 cycle_count;
>>       u8 high_count;
>> @@ -78,6 +102,8 @@ struct gpi_i2c_config {
>>       u8 addr;
>>       u8 stretch;
>>       u16 clk_div;
>> +    u32 clk_src;
>> +    struct gpi_i2c_config1 config1;
>>       u32 rx_len;
>>       enum i2c_op op;
>>       bool multi_msg;
>>
>