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

[Loic Poulain]

Hi Mukesh,

On Mon, Jul 27, 2026 at 11:33 AM Mukesh Savaliya
<mukesh.savaliya@xxxxxxxxxxxxxxxx> wrote:
>
>
>
> On 7/27/2026 2:51 PM, Loic Poulain wrote:
> [...]
>
> > @@ -587,6 +618,24 @@ static int cci_probe(struct platform_device *pdev)
> >               return dev_err_probe(dev, -EINVAL, "not enough clocks in DT\n");
> >       cci->nclocks = ret;
> >
> > +     cci->cci_clk = devm_clk_get(dev, "cci");
> > +     if (IS_ERR(cci->cci_clk))
> > +             return dev_err_probe(dev, PTR_ERR(cci->cci_clk),
> > +                                  "failed to get CCI clock\n");
> > +
> > +     ret = devm_pm_opp_set_clkname(dev, "cci");
> > +     if (ret)
> > +             return dev_err_probe(dev, ret, "failed to set CCI OPP clk\n");
> > +
> > +     /* OPP table is optional */
> > +     ret = devm_pm_opp_of_add_table(dev);
> > +     if (ret && ret != -ENODEV)
> > +             return dev_err_probe(dev, ret, "failed to add OPP table\n");
> > +
>
> if this is an optional why to return with an error ? IMO, further
> operations should continue which are not opp dependent ?

The OPP table is optional only in the sense that it may be absent,
that's the -ENODEV case, which we deliberately don't treat as an
error. If a table is present but malformed, or can't be added yet
because a required resource isn't ready (-EPROBE_DEFER), we do want to
report/propagate the error rather than silently continue. This is the
established idiom (spi-qcom-qspi, qcom_geni_serial...).

Regards,
Loic


>
> > +     ret = cci_set_core_rate(cci, cci->data->cci_clk_rate);
> > +     if (ret)
> > +             return dev_err_probe(dev, ret, "failed to set CCI clock rate\n");
> > +
> >       ret = cci_enable_clocks(cci);
> >       if (ret < 0)
> >               return ret;
> >
>