linux-kernel - Re: [PATCH] regulator: core: allow non-exact matches in regulator_set_voltage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 1 Jun 2014 12:38:34 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Lucas Stach <l.stach@...gutronix.de>
Cc:	Liam Girdwood <lgirdwood@...il.com>, linux-kernel@...r.kernel.org,
	kernel@...gutronix.de
Subject: Re: [PATCH] regulator: core: allow non-exact matches in
 regulator_set_voltage_time()

On Wed, May 21, 2014 at 06:53:59PM +0200, Lucas Stach wrote:
> Currently this function only provides a valid output if both
> old_uV and new_uV are exact voltages that can be provided by the
> regulator.
> This is almost impossible to achive as the consumer has
> no way to know the exact voltages provided by the regulator.

Yes it does - this is what regulator_list_voltage() is there for.
Drivers can enumerate all the voltages supported by a regulator.

> This breaks the current cpufreq users of this function, as they
> stick in the raw voltages retrieved from their operating points,
> which may or may not match one of the regulator voltages.

At least the code in cpufreq-cpu0 looks a bit confused here.  The use of
min_uV and max_uV is a bit unclear but probably correct however for some
reason it appears that what it's doing is stepping through each single
step transition between two adjacent frequencies, getting the transition
latency for that and then summing those.  Given that it needs a single
number I'd expect it to instead be getting the minimum and maximum
voltages and then working out the highest latency for transitioning
between those, what it's doing at the minute will be overestimating any
fixed component of transition latency (from the time taken to issue
commands to the device for example).

Incidentally the clock API ought to have a similar thing - at the minute
the driver just has a fixed number stuffed into it from DT but it really
ought to be able to ask the clock API in the same way as it asks the
regulator API.

> To make this function behave as expected employ the same logic
> as used when calling set_voltage() and round the voltages to
> the closest matching voltage supported by the regulator.

That's not what the set_voltage() code does - what it does is find the
lowest voltage in the requested range.  

Your code won't actually do quite what cpufreq-cpu0 is doing since it
uses set_voltage_tol() which will ask for a range around the voltage
it's trying to set so the query in cpufreq-cpu0 will come out as
something different to what the driver actually ends up doing when it
does transitions.  We should probably add functions to query what the
actual voltage selected for a given set_voltage() and set_voltage_tol()
will be then let that be fed into requesting the transition times.

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)