Hi Kevin,

On Wed, Feb 22, 2012 at 05:36:12, Hilman, Kevin wrote:
> In this case, volt comes from the OPP table, and was requested using a
> rounding call into the OPP table, so the resolution problem is handled
> there.  If 'volt' cannot be set by the regulator, then the OPP tables
> are also broken.
> 
> Also, in your patch, you only add some offset.  If you want to be
> approximate, shouldn't you have plus and minus?
> 
> IMO, we should let the OPP table handle that, and not the CPUfreq driver.

I have a different opinion.

Consider following case,

Voltage to set for OPPX is 1262.5mV and regulator has steps of 10mV,
and suppose regulator steps are .., 1260mV, 1270mV etc. Regulator
framework will not be able to set voltage for OPPX (certainly if
set_voltage_sel is used) if no range specified.

But instead if range of 1262.5 - (1262.5 + 10 - 1) is provided,
regulator can certainly set voltage for 1270mV (a nearest value)

Resolution in my patch was not meant to be resolution per se, it was meant
so that a voltage step can always be found for the regulator (assuming
worst resolution is 12.5mV), and for regulator to get a step, resolution
had to be used.

Ideal solution may be to use a variable to have a default resolution
of worst regulator and update it to that of regulator used (perhaps
making use of something like module parameters)

If regulator for a particular SoC is changed, exact OPP voltage may
not be settable, but a near value should be sufficient, this can't be
achieved if range is not specified. That happens exactly for AM335X,
voltage for one of OPP is 1.26V, but as regulator cannot set the
value, it is being set at 1.2625V and if no range is specified,
it will not work.

And in my patch plus - minus was not used as regulator framework will
try to set voltage for the least voltage which sometimes corresponds
to exact OPP required value.

For cpufreq omap driver to work with various regulators, it would be
better to specify ranges.

> I agree, my version is not very robust in the face of errors from the
> regulator framework.
> 
> Hoever, I'm not crazy about the extra notifications in your proposed
> patch.  I think it's cleaner to always pre and post notify.  If there's
> a failure, the post notify will have the same freq as the pre-notify,
> but that's not a big problem.


Yes, agree that error handling path is bulky. 

A problem that can happen is that drivers who has registered cpufreq
notifier will think that frequency has changed, that may cause problem
in addition to delay time getting altered.

But whether these are worth handling with a penalty of bulky error
handling, probably you know better.

Regards
Afzal
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to