On 02/12, Viresh Kumar wrote:

On 12 February 2015 at 08:52, Stephen Boyd [off-list ref] wrote:

quoted

Here's some feedback on how we can't use OPPs (and OPPs in DT) on
qcom platforms.

On these platforms the OPPs are not always frequency voltage
pairs. Sometimes they're a frequency voltage voltage triplet, or

So, making opp-microvolt an array of <target/min/max>, values should fix this?
Do we also need a opp-microvolt-names array as well? or can we reused
similar ones from the CPU node where regulator are defined.

I don't follow how target/min/max does anything for two different
voltages. I suppose something like opp-microvolt-names would work
though but I don't know how software would correlate that to the
regulator it uses because that information is elsewhere in the
device's node. Why not put the information about which clock and
regulator is used into the opp node?

quoted

Furthermore, we have a large number of OPP sets that apply to
different speed bins and silicon characteristics of the SoC. In
our systems we read some efuses (an eeprom of sorts) that tell us
to use a certain set of OPPs because the silicon is so fast or
has these certain characteristics. The bootloader is *not*
reading these fuses and populating OPPs in DT. So right now we
just put all these custom OPPish tables in DT and then pick the
right one based on a node name match constructed from the bits we
read in the efuses. How can we express this in DT with these
bindings?

What about keeping things as is in DT and disabling the OPPs which
you don't support at boot? And only keep enabled the set of OPPs
that you want to use based on these efuses ?

Let's look at an example:


	speed0 bin0 version0 = /* Hz      uV  uA */
                        <  300000000  815000  73 >,
                        <  345600000  825000  85 >,
                        <  422400000  835000 104 >,
			....

	speed0 bin1 version0 =
                        <  300000000  800000  73 >,
                        <  345600000  810000  85 >,
                        <  422400000  820000 104 >,
			...

Each set of fuses has the exact same frequency (as long as the
speed is the same) but the bin changes the voltage and sometimes
current. Maybe we could make this into:

 speed0_bin0_version0_opp: opp0 {
	 compatible = "qcom,speed0-bin0-version0-opp", "operating-points-v2";
	 entry0 {
		 opp-khz = <300000>;
		 opp-microvolt = <815000>;
		 opp-milliamp = <73>;
	 };
	 
	 entry1 {
		 opp-khz = <345600>;
		 opp-microvolt = <825000>;
		 opp-milliamp = <85>;
	 };
	 ...
 };
 
 speed0_bin1_version0_opp: opp0 {
	 compatible = "qcom,speed0-bin1-version0-opp", "operating-points-v2";
	 entry0 {
		 opp-khz = <300000>;
		 opp-microvolt = <800000>;
		 opp-milliamp = <73>;
	 };
	 
	 entry1 {
		 opp-khz = <345600>;
		 opp-microvolt = <810000>;
		 opp-milliamp = <85>;
	 };
	 ...
 };

And then we can construct a compatible string to search the cpus
node for. I wonder if we shouldn't put all this into an opps node
under the cpus node?

 cpus {
  opps { 
   opp0 {
     entry0 { }
     ...
   }
   opp1 {
    entry0 { }
     ...
   }
  }
 }

quoted

Or take msm8916 as another example. On this device the voltage
for a few frequencies comes from the efuses and then we
interpolate the rest of the frequency voltage pairs. The speed
bins are picked from another set of efuses so we can do the
interpolation. Unfortunately we don't encode the frequency in the
fuses, so we rely on a handful of tables being defined somewhere
so that we know speed bin 0 means this set of frequencies and
speed bin 1 means this set of frequencies. How do we encode this
in DT?  Should we have the frequencies as OPPs and leave the
voltage part out, filling it in at runtime based on what we read

Maybe yes.

quoted

out of the efuses? I assume it's desirable to have the frequency
tables in DT but we could also have them in the driver and if we
did that there wouldn't be any shared-opp property to set and
have the cpufreq-dt driver use to figure out clock sharing.

Probably better keep them in DT. But for platforms with dynamic OPPs
only, we will surely provide some API to make OPPs shared at runtime
too..

That would be good. Hopefully we can do that so that the
cpufreq-dt driver doesn't have to open code DT probing to figure
out that the OPPs are shared between CPUs.

quoted

Also sometimes we need to correlate OPPs between each other. For
example on msm8960/apq8064 if the CPU is running at a frequency
and voltage, the L2 needs to be running at another frequency,
voltage, and voltage (triplet). The L2 is in two power domains
but it only has one clock. Can/should this be expressed in DT? It

Not sure.

quoted

certainly seems that it's at least easier to add it on as a
feature because OPPs are nodes instead of an array. But we need

It wouldn't be a great idea to make these nodes too large just to support
some corner cases, and we better try to find the best way out. But if
we need it this way, we need it this way :)

quoted

to make sure we can support multiple regulators somehow, either
through correlated OPPs and multiple OPPs for a single device or
by being able to say opp-0-microvolt, opp-1-microvolt. I would

an array would be better.

Sure.

quoted

guess something similar could happen if there were two clocks and
one regulator although I've never seen such a scenario in
practice.

Isn't this common? A single regulator voltage supporting multiple clock
rates? Or did I misunderstood it :)

We can have separate OPP nodes in this case.

I was thinking the same device has two clocks that share the same
voltage and these two clocks can run at different rates. If two
opp nodes under the same device works then it sounds like it will
be ok. We probably still need some way to correlate the two so
that if one clock is at some freq, the other clock should be at
the corresponding freq in the OPP, assuming that the OPP is
really two clocks and a voltage (i.e. the clocks need to scale
together).

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project