Re: [PATCH 3/3] i2c: qcom-cci: Fix CCI clock rate enforcement

Hi Konrad,

On Tue, Jul 21, 2026 at 5:07 PM Konrad Dybcio
<[email protected]> wrote:
>
> On 7/21/26 4:58 PM, Loic Poulain wrote:
> > The hw_params timing values (thigh, tlow, etc.) are in CCI clock ticks
> > and were calibrated for a specific clock rate per hardware variant. If
> > the clock is running at a different rate the I2C timings will be wrong,
> > potentially violating the I2C specification.
> >
> > Rather than just warning about a mismatch, actively set the clock to the
> > expected rate at probe time. Warn if the rate could not be applied so the
> > user is informed that timings may be incorrect.
> >
> > Signed-off-by: Loic Poulain <[email protected]>
> > ---
> >  drivers/i2c/busses/i2c-qcom-cci.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> >
> > diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
> > index 5f74edde8558382e63e2c3d0fbe19489f325e8ce..e001c0d565cebd386fd67b1d9d532360b29c67d0 100644
> > --- a/drivers/i2c/busses/i2c-qcom-cci.c
> > +++ b/drivers/i2c/busses/i2c-qcom-cci.c
> > @@ -519,6 +519,7 @@ static int cci_probe(struct platform_device *pdev)
> >       struct device *dev = &pdev->dev;
> >       struct device_node *child;
> >       struct resource *r;
> > +     struct clk *cci_clk;
> >       struct cci *cci;
> >       int ret, i;
> >       u32 val;
> > @@ -587,6 +588,15 @@ static int cci_probe(struct platform_device *pdev)
> >               return dev_err_probe(dev, -EINVAL, "not enough clocks in DT\n");
> >       cci->nclocks = ret;
> >
> > +     cci_clk = devm_clk_get(dev, "cci");
> > +     if (IS_ERR(cci_clk))
> > +             return dev_err_probe(dev, PTR_ERR(cci_clk), "failed to get CCI clock\n");
> > +
> > +     clk_set_rate(cci_clk, cci->data->cci_clk_rate);
> > +     if (clk_get_rate(cci_clk) != cci->data->cci_clk_rate)
> > +             dev_warn(dev, "CCI clock is not at expected %lu Hz\n",
> > +                      cci->data->cci_clk_rate);
>
> This needs to go through OPP.
>
> Feel free to take over and refresh:
>
> https://lore.kernel.org/linux-arm-msm/[email protected]/

Sorry for missing your series earlier. I still think we need this
series to restore the expected clock rate information in the driver
and enforce it at probe time, right now the timing tables are
calibrated for a specific frequency and the driver has no way to
verify or correct what it gets.

Then, I'm not against an OPP-based approach in principle, though I
think it should be built on top of proper foundations, we would need
to rework the driver so that the mode selection (standard/fast/fast+)
drives the clock rate selection, which in turn selects the appropriate
timing table.

Regards,
Loic