Re: [PATCH 2/2] i2c: qcom-geni: Add I2C frequency table for firmware-based SEs

On 8/11/2026 6:12 PM, Mukesh Savaliya wrote:
>
>
> On 8/11/2026 2:53 PM, Kathiravan Thirumoorthy wrote:
>> On IPQ5424, RTL-based I2C can be supported by both minicore and firmware
>> based SEs. For firmware-based SEs, use different SCL settings as
>> recommended by the design team. Introduce a separate table for the same.
>>
>> Identify whether the SE is minicore or firmware-based and select the
> what is minicore ? Please Make it upstream specific, RTL hardened or 
> define minicore first.

Ack.

>> appropriate frequency table to achieve the desired bus frequencies.
>>
>> Move geni_i2c_clk_map_idx() from geni_i2c_resources_init() to
>> geni_i2c_init() to ensure SE_HW_PARAM_2 is accessed only after the
> Reasoning required - why only for 32 MHz ? OR better reasoning for 
> 32Mhz support related changes.

Ack.

>> required clocks have been enabled.
>>
>
> I suggest to add 32Mhz support as this is already having 19.2MHz 
> support. Subject shows generic frequency support sounding like no 
> support present.

Ack.

>
>> Signed-off-by: Kathiravan Thirumoorthy 
>> <[email protected]>
>> ---
>>   drivers/i2c/busses/i2c-qcom-geni.c | 23 +++++++++++++++++------
>>   include/linux/soc/qcom/geni-se.h   |  1 +
>>   2 files changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c 
>> b/drivers/i2c/busses/i2c-qcom-geni.c
>> index 00013b41a6f5..2f3026602c0c 100644
>> --- a/drivers/i2c/busses/i2c-qcom-geni.c
>> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
>> @@ -194,12 +194,21 @@ static const struct geni_i2c_clk_fld 
>> geni_i2c_clk_map_32mhz[] = {
> rename to _minicore ?

Ack.

>>       {}
>>   };
>>   +static const struct geni_i2c_clk_fld 
>> geni_i2c_clk_map_32mhz_fwcore[] = {
>> +    { I2C_MAX_STANDARD_MODE_FREQ, 12, 9, 10, 25 },
>> +    { I2C_MAX_FAST_MODE_FREQ, 4, 3, 9, 18 },
>> +    { I2C_MAX_FAST_MODE_PLUS_FREQ, 1, 6, 15, 29 },
>> +    {}
>> +};
>> +
>>   static int geni_i2c_clk_map_idx(struct geni_i2c_dev *gi2c)
>>   {
>>       const struct geni_i2c_clk_fld *itr;
>> +    bool is_minicore = FIELD_GET(GEN_USE_MINICORES,
>> +                     readl_relaxed(gi2c->se.base + SE_HW_PARAM_2));
>>         if (clk_get_rate(gi2c->se.clk) == 32 * HZ_PER_MHZ)
>> -        itr = geni_i2c_clk_map_32mhz;
>> +        itr = is_minicore ? geni_i2c_clk_map_32mhz : 
>> geni_i2c_clk_map_32mhz_fwcore;
> _minicore with above table renamed.

Ack.

>>       else
>>           itr = geni_i2c_clk_map_19p2mhz;
>>   @@ -1046,6 +1055,13 @@ static int geni_i2c_init(struct geni_i2c_dev 
>> *gi2c)
> [...]