Re: [PATCH v2 2/2] i2c: qcom-geni: add I2C frequency table for 32 MHz firmware-based SEs

[Kathiravan Thirumoorthy]

On 8/25/2026 10:50 PM, Praveen Talari wrote:
> Hi Kathiravan,
>
> On 12-08-2026 14:00, Kathiravan Thirumoorthy wrote:
> > On IPQ5424, the I2C controller can operate on both RTL-hardened SEs
> > and firmware-based SEs. As documented in commit 506bb2ab0075
> > ("i2c: qcom-geni: Support systems with 32MHz serial engine clock"),
> > in IPQ5424 SEs use a 32 MHz source clock.
> >
> > The existing frequency table does not provide the required SCL timing
> > parameters for firmware-based SEs. Add a dedicated frequency table
> > using the SCL counter values recommended by the hardware design team
> > and select it for firmware-based SEs to achieve the target 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
> > required clocks have been enabled.
> >
> > Signed-off-by: Kathiravan Thirumoorthy 
> > <kathiravan.thirumoorthy@xxxxxxxxxxxxxxxx>
> > ---
> >   drivers/i2c/busses/i2c-qcom-geni.c | 25 ++++++++++++++++++-------
> >   include/linux/soc/qcom/geni-se.h   |  1 +
> >   2 files changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-qcom-geni.c 
> > b/drivers/i2c/busses/i2c-qcom-geni.c
> > index 00013b41a6f5..15835f10b062 100644
> > --- a/drivers/i2c/busses/i2c-qcom-geni.c
> > +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> > @@ -187,19 +187,28 @@ static const struct geni_i2c_clk_fld 
> > geni_i2c_clk_map_19p2mhz[] = {
> >   };
> >     /* source_clock = 32 MHz */
> > -static const struct geni_i2c_clk_fld geni_i2c_clk_map_32mhz[] = {
> > +static const struct geni_i2c_clk_fld 
> > geni_i2c_clk_map_32mhz_minicore[] = {
> >       { I2C_MAX_STANDARD_MODE_FREQ, 12, 9, 10, 26 },
> >       { I2C_MAX_FAST_MODE_FREQ, 4, 3, 9, 19 },
> >       { I2C_MAX_FAST_MODE_PLUS_FREQ, 2, 3, 5, 15 },
> >       {}
> >   };
> >   +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_minicore : 
> > geni_i2c_clk_map_32mhz_fwcore;
> The source selection remains unchanged when using 32 MHz:
> writel_relaxed(0, gi2c->se.base + SE_GENI_CLK_SEL);

I have to check on this. But in downstream, we didn't change this 
configuration.

> However, what about the voltage requirements associated with the 
> higher frequency?

There are no such requirements in the IPQ SoC.

> Are they automatically managed by the underlying provider, or is 
> additional handling needed in the driver?
>
> Reference of konrad change for CCI I2C driver for 37.5MHz with voltage 
> requirement.
>
> https://lore.kernel.org/linux-arm-msm/20250904-topic-cci_updates-v1-4-d38559692703@xxxxxxxxxxxxxxxx/ 
>
>
>
> Thanks,
>
> Praveen Talari