Re: [PATCH 5/8] OPP: Allow multiple clocks for a device

Krzysztof Kozlowski
2022-07-05T08:18:58Z

On 05/07/2022 08:59, Viresh Kumar wrote:
> On 30-06-22, 14:39, Krzysztof Kozlowski wrote:
>> On 30/06/2022 14:32, Krzysztof Kozlowski wrote:
>>> On 10/06/2022 10:20, Viresh Kumar wrote:
>>>> + ret = _read_rate(new_opp, opp_table, np);
>>>> + if (ret)
>>>> + return ret;
>>>> + else if (opp_table->clk_count == 1)
>>>
>>> Shouldn't this be >=1? I got several clocks and this one fails.
>>
>> Actually this might be correct, but you need to update the bindings. Now
>> you require opp-level for case with multiple clocks.
>
> I have thought about this again and adding such "fake" property in DT
> doesn't look right, specially in binding document. It maybe fine to
> have a "level" property in your case of UFS, where we want something
> to represent gears. But others may not want it.

I would say it is not different than existing opp-level property. To me
it sounded fine, so at least one DT bindings maintainer would accept it. :)

>
> So, in the new version I am sending now, we still consider opp-hz
> property as the property that uniquely identifies an OPP. Just that we
> compare all the rates now, and not just the first one. I have updated
> _opp_compare_keys() for this as well.
>
> The drivers, for multiple clock case, are expected to call
> dev_pm_opp_set_opp() to set the specific OPP. Though how they find the
> target OPP is left for the users to handle. For some, we may have
> another unique OPP property, like level, which can be used to find the
> OPP. While in case of others, we may want to implement freq-based OPP
> finder APIs for multiple clock rates. I have decided not to implement
> them in advance, and add them only someone wants to use them.

Thanks! Let me take a look at v2.

Best regards,
Krzysztof