Re: [PATCH] i2c: qcom-geni: Fix hardcoded clock index in SE_GENI_CLK_SEL

[Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>]

On 9/21/2026 2:44 PM, Mukesh Savaliya wrote:
> 
> 
> On 8/28/2026 3:59 PM, Viken Dadhaniya wrote:
> 
>> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
>> index 658636c1ee0e..a83297b5fb0a 100644
>> --- a/drivers/i2c/busses/i2c-qcom-geni.c
>> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
>> @@ -82,6 +82,9 @@ enum geni_i2c_err_code {
>>   #define XFER_TIMEOUT        HZ
>>   #define RST_TIMEOUT        HZ
>>   +#define GENI_SE_CLK_32MHZ    (32 * HZ_PER_MHZ)
>> +#define GENI_SE_CLK_19P2MHZ    19200000UL
>> +
>>   struct geni_i2c_desc {
>>       bool no_dma_support;
>>       unsigned int tx_fifo_depth;
>> @@ -127,6 +130,7 @@ struct geni_i2c_dev {
>>       spinlock_t lock;
>>       u32 clk_freq_out;
>>       const struct geni_i2c_clk_fld *clk_fld;
>> +    u32 clk_idx;
>>       void *dma_buf;
>>       size_t xfer_len;
>>       dma_addr_t dma_addr;
>> @@ -197,19 +201,42 @@ static const struct geni_i2c_clk_fld geni_i2c_clk_map_32mhz[] = {
>>   static int geni_i2c_clk_map_idx(struct geni_i2c_dev *gi2c)
>>   {
>>       const struct geni_i2c_clk_fld *itr;
>> +    unsigned long res_freq;
>>   -    if (clk_get_rate(gi2c->se.clk) == 32 * HZ_PER_MHZ)
>> +    /*
>> +     * Counter tables are calibrated for a specific source clock frequency
> Minor: Frequency counter tables> +     * and are not valid for any multiple of it (e.g. 64 MHz, 128 MHz).

Updated in v2.

>> +     * Use exact=true and verify res_freq matches req_freq literally to
>> +     * reject harmonics: a 64 MHz clock that divides evenly to 32 MHz
>> +     * would pass exact matching but produce double the intended I2C
>> +     * frequency with these counter values.
>> +     */
>> +    if (!geni_se_clk_freq_match(&gi2c->se, GENI_SE_CLK_32MHZ,
>> +                    &gi2c->clk_idx, &res_freq, true) &&
>> +        res_freq == GENI_SE_CLK_32MHZ) {
>>           itr = geni_i2c_clk_map_32mhz;
>> -    else
>> +    } else if (!geni_se_clk_freq_match(&gi2c->se, GENI_SE_CLK_19P2MHZ,
>> +                       &gi2c->clk_idx, &res_freq, true) &&
>> +           res_freq == GENI_SE_CLK_19P2MHZ) {
>>           itr = geni_i2c_clk_map_19p2mhz;
>> +    } else {
>> +        dev_err(gi2c->se.dev,
>> +            "Unsupported SE source clock: must be exactly 32 MHz or 19.2 MHz\n");
>> +        return -EINVAL;
>> +    }
>>         while (itr->clk_freq_out != 0) {
>>           if (itr->clk_freq_out == gi2c->clk_freq_out) {
>>               gi2c->clk_fld = itr;
>> +            dev_dbg(gi2c->se.dev,
>> +                "I2C clk selected: freq: %u Hz, clk_idx: %u\n",
>> +                gi2c->clk_freq_out, gi2c->clk_idx);
>>               return 0;
>>           }
>>           itr++;
>>       }
>> +
>> +    dev_err(gi2c->se.dev, "Unsupported I2C output frequency %u Hz\n", gi2c->clk_freq_out);
> 
> add a line space before return>       return -EINVAL;

Added in v2.

>>   }
>>   
> 
> With above taken cared, rest all looks good.
>