On Thu, Nov 30, 2017 at 12:59 AM, Viresh Kumar <[email protected]> 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
On 26-12-17, 14:23, Rob Herring wrote:
> > 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.
There are two type of devices:
A.) With fixed performance state requirements and they will have the
new "required-opp" property in the device node itself as you said.
B.) Devices which can do DVFS (CPU, MMC, LCD, etc) and those may need
a different performance state of the domain for their individual OPPs
and so we can't have this property in the device all the time.
Does this make sense ?
--
viresh
On Tue, Dec 26, 2017 at 10:45 PM, Viresh Kumar <[email protected]> wrote:
> On 26-12-17, 14:23, Rob Herring wrote:
>> > 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.
>
> There are two type of devices:
>
> A.) With fixed performance state requirements and they will have the
> new "required-opp" property in the device node itself as you said.
>
> B.) Devices which can do DVFS (CPU, MMC, LCD, etc) and those may need
> a different performance state of the domain for their individual OPPs
> and so we can't have this property in the device all the time.
>
> Does this make sense ?
No. From the definition for power-domain-opp
"+- power-domain-opp: This contains phandle to one of the OPP nodes of
the master
+ power domain. This specifies the minimum required OPP of the master
domain for
+ the functioning of the device in this OPP (where this property is present).
+ This property can only be set for a device if the device node contains the
+ "power-domains" property. Also, either all or none of the OPP nodes in an OPP
+ table should have it set."
In the above example, you are violating the next to last sentence.
Though, I'm now confused by what the last sentence means.
Rob
On 27-12-17, 15:36, Rob Herring wrote:
> On Tue, Dec 26, 2017 at 10:45 PM, Viresh Kumar <[email protected]> wrote:
> > On 26-12-17, 14:23, Rob Herring wrote:
> >> > 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.
> >
> > There are two type of devices:
> >
> > A.) With fixed performance state requirements and they will have the
> > new "required-opp" property in the device node itself as you said.
> >
> > B.) Devices which can do DVFS (CPU, MMC, LCD, etc) and those may need
> > a different performance state of the domain for their individual OPPs
> > and so we can't have this property in the device all the time.
> >
> > Does this make sense ?
>
> No. From the definition for power-domain-opp
>
> "+- power-domain-opp: This contains phandle to one of the OPP nodes of
> the master
> + power domain. This specifies the minimum required OPP of the master
> domain for
> + the functioning of the device in this OPP (where this property is present).
The per-opp thing was mentioned here.
> + This property can only be set for a device if the device node contains the
> + "power-domains" property.
This was trying to say something else, though it wasn't clear and so your
concerns.
I wanted to say that the device node or its OPP nodes can have the
"power-domain-opp" property only if the device node has a "power-domains"
property. i.e. you need to have power domain first and then only the
power-domain-opp property.
> Also, either all or none of the OPP nodes in an OPP
> + table should have it set."
>
> In the above example, you are violating the next to last sentence.
>
> Though, I'm now confused by what the last sentence means.
Yeah, lets leave it as is as the V8 has changed this significantly and you
already Acked it :)
--
viresh