Re: [RFC V7 2/2] OPP: Allow "opp-hz" and "opp-microvolt" to contain magic values

From: Rob Herring
Date: Tue Dec 26 2017 - 15:24:34 EST


On Thu, Nov 30, 2017 at 12:59 AM, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> On 29-11-17, 16:50, Stephen Boyd wrote:
>> Sorry it still makes zero sense to me. It seems that we're trying
>> to make the OPP table parsing generic just for the sake of code
>> brevity.
>
> Not just the code but bindings as well to make sure we don't add a new
> property (similar to earlier ones) for every platform that wants to
> use performance states.
>
>> Is this the goal? From a DT writer perspective it seems
>> confusing to say that opp-microvolt is sometimes a microvolt and
>> sometimes not a microvolt.
>
> Well it would still represent the voltage but not in microvolt units
> as the platform guys decided to hide those values from kernel and
> handle them directly in firmware.
>
>> Why can't the SoC specific genpd
>> driver parse something like "qcom,corner" instead out of the
>> node?
>
> Sure we can, but that means that a new property will be required for
> the next platform.
>
> I did it this way as Kevin (and Rob) suggested NOT to add another
> property but use the earlier ones as we aren't passing anything new
> here, just that the units of the property are different. For another
> SoC, we may want to hide both freq and voltage values from kernel and
> pass firmware dependent values. Should we add two new properties for
> that SoC then ?
>
>> BTW, I don't believe I have a use-case where I want to express
>> power domain OPP tables.
>
> I do remember that you once said [1] that you may want to pass the
> real voltage values as well via DT. And so I thought that you can pass
> performance-state (corner) in opp-hz and real voltage values in
> opp-microvolt.
>
>> I have many devices that all have
>> different frequencies that are all tied into the same power
>> domain. This binding makes it look like we can only have one
>> frequency per domain which won't work.
>
> No, that isn't the case. Looks like we have some confusion here. Let
> me try with a simple example:
>
> foo: foo-power-domain@09000000 {
> compatible = "foo,genpd";
> #power-domain-cells = <0>;
> operating-points-v2 = <&domain_opp_table>;
> };
>
> cpu0: cpu@0 {
> compatible = "arm,cortex-a53", "arm,armv8";
> ...
> operating-points-v2 = <&cpu_opp_table>;
> power-domains = <&foo>;
> };
>
>
> domain_opp_table: domain_opp_table {
> compatible = "operating-points-v2";
>
> domain_opp_1: opp00 {
> opp-hz = /bits/ 64 <1>; /* These are corners AKA perf states */
> };
> domain_opp_2: opp01 {
> opp-hz = /bits/ 64 <2>;
> };
> domain_opp_3: opp02 {
> opp-hz = /bits/ 64 <3>;
> };
> };
>
> cpu_opp_table: cpu_opp_table {
> compatible = "operating-points-v2";
> opp-shared;
>
> opp00 {
> opp-hz = /bits/ 64 <208000000>;
> clock-latency-ns = <500000>;
> power-domain-opp = <&domain_opp_1>;

What is this? opp00 here is not a device. One OPP should not point to
another. "power-domain-opp" is only supposed to appear in devices
alongside power-domains properties.

Rob