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

Thanks Loic !

On 7/27/2026 3:15 PM, Loic Poulain wrote:
> Hi Mukesh,
> 
> On Mon, Jul 27, 2026 at 11:33 AM Mukesh Savaliya
> <[email protected]> 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...).
> 
Sorry, understood now. hence protected with both the conditions. Makes 
sense.

> 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;
>>>
>>