diff mbox

[V4,1/3] OPP: Redefine bindings to overcome shortcomings

Message ID CAKohpo=09ZhXxh5Cm7Ww9Gy-AYRp9=j82tup6atVN+aOF8twdA@mail.gmail.com
State New
Headers show

Commit Message

Viresh Kumar May 5, 2015, 11:43 a.m. UTC
On 5 May 2015 at 16:27, Mark Brown <broonie@kernel.org> wrote:
> No, it doesn't - you're not answering the question about what this is
> for.

I don't know how this information will be used finally. Probably the
platform driver will do the configuration based on the volt-cur pair.

> To know if this makes sense I need to know what you beleive "setting the
> current" does.  If you literally mean setting the current it makes no
> sense at all.  If you mean something else that something else should
> probably be written into the binding.

Yeah, that was a wrong statement. We can't configure current separately.

Does this diff make it any better ?


   nanoseconds) for switching to this OPP from any other OPP.


(Restoring my laptop after a corrupted disk, and so sending it from gmail,
might be a bit corrupted)..
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Viresh Kumar May 6, 2015, 6:53 a.m. UTC | #1
On 5 May 2015 at 22:42, Mark Brown <broonie@kernel.org> wrote:
> This still doesn't say what the value is supposed to mean which means
> it's not possible for someone to write generic code which handles the
> binding - saying that people can reference the value doesn't tell them
> how to interpret it.  A limit?  A nominal constant draw value?  A value
> to regulate to?  Those are all different things expressed as a current.

@Stephen: Can you please answer these queries please, so that we
can get the bindings correctly ?
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Viresh Kumar May 7, 2015, 12:13 p.m. UTC | #2
On 7 May 2015 at 11:22, Stephen Boyd <sboyd@codeaurora.org> wrote:
> If you look at the cpufreq/clock/pmic code on our codeaurora.org
> tree you'll see that it's used to pass a value with uA units
> through the regulator_set_optimum_mode() API. The call to
> regulator_set_optimum_mode() is here[1], and the place where we
> parse the OPP table from DT is here[2]. My understanding is that
> with recent changes to the regulator framework, we would do a
> s/regulator_set_optimum_mode/regulator_set_load/ and things would
> otherwise be the same on the consumer side. On the regulator
> side, we would move the .get_optimum_mode op to .set_load instead
> of the hack where we write to the hardware in
> .get_optimum_mode[3].
>
> So does that mean it corresponds to a limit, or a nominal
> constant draw value, or a value to regulator to? From what I can
> tell it's used to figure out how many phases to enable[4]. I
> suspect that means it's being used to figure out what value it
> should regulate to. In newer PMICs this is all done
> automatically, but at least for some of the PMICs we need to do
> it manually.

By current phases, you probably meant [1], right ?

Also what I couldn't understand is how are you controlling the
current here?

- Is this a current only regulator ? Probably not as your code has
a uv value as well..

- Now if uV is fixed, how is the target current controlled separately ?

- Should we have a min/max/target value of this current ? Like what
we are doing for voltage. Or is this just the 'target' current ?

- And finally how exactly should we write this in bindings to make
it clear to all users ?

Thanks.

--
viresh

[1] http://en.wikipedia.org/wiki/Three-phase_electric_power
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Viresh Kumar May 8, 2015, 6:47 a.m. UTC | #3
On 8 May 2015 at 03:48, Mark Brown <broonie@kernel.org> wrote:

> That makes sense to me.  Perhaps "current drawn by the device" rather
> than "current load of the device" since current is sadly overloaded :/

Thanks Mark. I have reworded it as:

- opp-microamp: Current drawn by the device in micro Amperes. It is used to set
  the most efficient regulator operating mode.

  Should only be set if opp-microvolt is set for the OPP.

  Entries for multiple regulators must be present in the same order as
  regulators are specified in device's DT node. If this property isn't required
  for few regulators, then this should be marked as zero for them. If it isn't
  required for any regulator, then this property need not be present.


Please let me know if this looks fine now.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Viresh Kumar May 8, 2015, 6:49 a.m. UTC | #4
On 8 May 2015 at 03:00, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 05/07, Viresh Kumar wrote:

>> - Now if uV is fixed, how is the target current controlled separately ?
>
> uV is not fixed

I meant uV is fixed for this particular OPP, i.e. min/tar/max value.


What about the other changes? Does these fix the use cases you shared
earlier ? I will send a new updated version then..
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Viresh Kumar May 8, 2015, 11:01 a.m. UTC | #5
On 8 May 2015 at 16:28, Mark Brown <broonie@kernel.org> wrote:
> That should be fine, microamperes is usually one word.

Will fix that. Thanks a lot Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Viresh Kumar May 12, 2015, 5:20 a.m. UTC | #6
On 10-05-15, 20:07, Nishanth Menon wrote:
> just one minor concern being in the SoC end of the world :). In most
> times, the current consumption for a specific OPP varies depending on
> the specific location in the process node the die is -> this is even
> true among a single lot of wafers as well. some SoC vendors use hot,
> nominal and cold terminology to indicate the characteristics of the
> specific sample.
> 
> it might help state which sample end of the spectrum we are talking
> about here. reason being: if I put in values based on my board
> measurement, the results may not be similar to what someone else's
> sample be. Depending on technology, speed binning strategy used by the
> vendor, temperature and few other characteristics, these numbers could
> be widely divergent.

I don't have any clue about this.. :(

@Mark/stephen: Any inputs ?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Turquette May 12, 2015, 7:01 p.m. UTC | #7
Quoting Viresh Kumar (2015-05-11 22:20:33)
> On 10-05-15, 20:07, Nishanth Menon wrote:
> > just one minor concern being in the SoC end of the world :). In most
> > times, the current consumption for a specific OPP varies depending on
> > the specific location in the process node the die is -> this is even
> > true among a single lot of wafers as well. some SoC vendors use hot,
> > nominal and cold terminology to indicate the characteristics of the
> > specific sample.
> > 
> > it might help state which sample end of the spectrum we are talking
> > about here. reason being: if I put in values based on my board
> > measurement, the results may not be similar to what someone else's
> > sample be. Depending on technology, speed binning strategy used by the
> > vendor, temperature and few other characteristics, these numbers could
> > be widely divergent.
> 
> I don't have any clue about this.. :(
> 
> @Mark/stephen: Any inputs ?

Nishanth,

I do not think the idea of the mA property is to perfectly model current
consumption at a given opp. Instead it is a nominal value that may be
useful, e.g. for configuring a regulator in Stephen's case.

The TWLxxxx series of PMICs from TI have configurable SMPS which could
possibly benefit from this info as well. Most of the time those are left
in an "automatic mode", but there are manual programming steps to
achieve higher efficiencies and this property could potentially help you
do that.

Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/power/opp.txt
b/Documentation/devicetree/bindings/power/opp.txt
index c96dc77121b7..a57e88ab4554 100644
--- a/Documentation/devicetree/bindings/power/opp.txt
+++ b/Documentation/devicetree/bindings/power/opp.txt
@@ -59,16 +59,18 @@  properties.
   regulators are specified in device's DT node.

 - opp-microamp: current in micro Amperes. It can contain entries for multiple
-  regulators.
+  regulators. This can be referenced (along with voltage and freqency) while
+  programming the regulator.

   A single regulator's current is specified with an array of size one or three.
   Single entry is for target current and three entries are for <target min max>
   currents.

   Entries for multiple regulators must be present in the same order as
-  regulators are specified in device's DT node. If few regulators don't provide
-  capability to configure current, then values for then should be marked as
-  zero.
+  regulators are specified in device's DT node. If current value for few
+  regulators isn't required to be passed, then values for such
regulators should
+  be marked as zero. If it isn't required for any regulator, then this property
+  need not be present.

 - clock-latency-ns: Specifies the maximum possible transition latency (in