| Message ID | 1371669474-24473-1-git-send-email-nm@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 19, 2013 at 02:17:54PM -0500, Nishanth Menon wrote: > Account for step size accuracy when exact voltage requests are send for > step based regulators. If the consumer can tolerate a different voltage why not just request the range that can be tolerated? Your problem here is specifying an exact voltage. > The specific example I faced was using cpufreq-cpu0 driver with voltages > for OPPs for MPU rail and attempting the common definitions against voltages > that are non-exact multiples of stepsize of PMIC. > The alternative would be implement custom set_voltage (as againsta simpler > set_voltage_sel and using linear map/list functions) for the regulator which > will account for the same. > Yet another alternative might be to introduce yet another custom function similar > to regulator_set_voltage_tol which accounts for this. something like: > regulator_set_voltage_floor(regulator, voltage, tol) or something to that effect. Or as I keep telling you guys the consumer can just do that directly using the existing API; the whole point in specifying the voltage as a range is to allow the consumer to cope with arbatrary regulators by giving a range of voltages that it can accept. The API is deliberately very conservative in these matters since there is a danger of physical damage if things really go wrong in some applications, it makes sure that both the drivers and the system integration are involved.
On 23:38-20130619, Mark Brown wrote: > On Wed, Jun 19, 2013 at 02:17:54PM -0500, Nishanth Menon wrote: > > > Account for step size accuracy when exact voltage requests are send for > > step based regulators. > > If the consumer can tolerate a different voltage why not just request > the range that can be tolerated? Your problem here is specifying an > exact voltage. I think you mean using regulator_get_linear_step > > > The specific example I faced was using cpufreq-cpu0 driver with voltages > > for OPPs for MPU rail and attempting the common definitions against voltages > > that are non-exact multiples of stepsize of PMIC. > > > The alternative would be implement custom set_voltage (as againsta simpler > > set_voltage_sel and using linear map/list functions) for the regulator which > > will account for the same. > > > Yet another alternative might be to introduce yet another custom function similar > > to regulator_set_voltage_tol which accounts for this. something like: > > regulator_set_voltage_floor(regulator, voltage, tol) or something to that effect. > > Or as I keep telling you guys the consumer can just do that directly > using the existing API; the whole point in specifying the voltage as a > range is to allow the consumer to cope with arbatrary regulators by > giving a range of voltages that it can accept. > > The API is deliberately very conservative in these matters since there > is a danger of physical damage if things really go wrong in some > applications, it makes sure that both the drivers and the system > integration are involved. I agree. The intent of this series was to start a conversation to see if we can make it simpler. Searching for the users of regulator_get_linear_step in 3.10-rc6 shows none. For a generic driver which needs to handle platforms which have tolerance, they'd use regulator_set_voltage_tol. But the implementation would allow for uV - tol to uV + tol as range, which in the case I mentioned(min voltage =uV) wont work. If the consumer wants to be aware of linear step regulator, they'd have to do: step_uV = regulator_get_linear_step(...); regulator_set_voltage(uV, uV + step_uV); Then this wont handle tolerance. So the solution seems to be (for the consumer): step_uV = regulator_get_linear_step(...); .. if (tol) regulator_set_voltage_tol(uV, tol); else regulator_set_voltage(uV, uV + step_uV); (with the required error checks for regulator being a linear regulator etc..). At least to me, there is no sane manner to handle "tolerance" and linear step accuracy for a defined voltage (Should tolerance be rounded off to step_uV? what about the border cases etc.) Would you agree?
On Thu, Jun 20, 2013 at 07:45:42AM -0500, Nishanth Menon wrote: > On 23:38-20130619, Mark Brown wrote: > > If the consumer can tolerate a different voltage why not just request > > the range that can be tolerated? Your problem here is specifying an > > exact voltage. > I think you mean using regulator_get_linear_step No, I don't. Why would that make sense? It limits the regulators that can be used and the properties of the regulator have no impact on what the SoC can support. > > Or as I keep telling you guys the consumer can just do that directly > > using the existing API; the whole point in specifying the voltage as a > > range is to allow the consumer to cope with arbatrary regulators by > > giving a range of voltages that it can accept. > I agree. The intent of this series was to start a conversation to see if > we can make it simpler. It's already very simple. The consumer driver just needs to supply the list of voltages which it can accept, that's all. > For a generic driver which needs to handle platforms which > have tolerance, they'd use regulator_set_voltage_tol. But the > implementation would allow for uV - tol to uV + tol as range, which in > the case I mentioned(min voltage =uV) wont work. So just write it as an explicit voltage range then. For example many devices can tolerate any supply up to the maximum rated supply at any frequency so just specify that as the upper limit. > If the consumer wants to be aware of linear step regulator, they'd have to do: > step_uV = regulator_get_linear_step(...); > regulator_set_voltage(uV, uV + step_uV); No, the consumer really doesn't want to be aware of linear step regulators. Why would it care that there even are linear steps? If the consumer is doing this based on the properties of the regulator rather than on the properties of the consumer this indicates that the consumer has a problem If the consumer is doing this based on the properties of the regulator rather than on the properties of the consumer this indicates that the consumer has a problem
On 10:51-20130621, Mark Brown wrote: > On Thu, Jun 20, 2013 at 07:45:42AM -0500, Nishanth Menon wrote: > > On 23:38-20130619, Mark Brown wrote: > > > > If the consumer can tolerate a different voltage why not just request > > > the range that can be tolerated? Your problem here is specifying an > > > exact voltage. > > > I think you mean using regulator_get_linear_step > > No, I don't. Why would that make sense? It limits the regulators that > can be used and the properties of the regulator have no impact on what > the SoC can support. > > > > Or as I keep telling you guys the consumer can just do that directly > > > using the existing API; the whole point in specifying the voltage as a > > > range is to allow the consumer to cope with arbatrary regulators by > > > giving a range of voltages that it can accept. > > > I agree. The intent of this series was to start a conversation to see if > > we can make it simpler. > > It's already very simple. The consumer driver just needs to supply the > list of voltages which it can accept, that's all. > > > For a generic driver which needs to handle platforms which > > have tolerance, they'd use regulator_set_voltage_tol. But the > > implementation would allow for uV - tol to uV + tol as range, which in > > the case I mentioned(min voltage =uV) wont work. > > So just write it as an explicit voltage range then. For example many > devices can tolerate any supply up to the maximum rated supply at any > frequency so just specify that as the upper limit. > > > If the consumer wants to be aware of linear step regulator, they'd have to do: > > step_uV = regulator_get_linear_step(...); > > regulator_set_voltage(uV, uV + step_uV); > > No, the consumer really doesn't want to be aware of linear step > regulators. Why would it care that there even are linear steps? If > the consumer is doing this based on the properties of the regulator > rather than on the properties of the consumer this indicates that the > consumer has a problem If the consumer is doing this based on the > properties of the regulator rather than on the properties of the > consumer this indicates that the consumer has a problem The specific case that I am trying to tackle is as follows: cpufreq-cpu0 uses definitions of voltages that are SoC specific. For a given frequency, the optimal voltage is X, max voltage(Y) is already expected to be in constraints for device functionality. We however want to find the closest voltage for a regulator in range X to Y best achievable by regulator. I think the area where I am getting confused is this: I am thinking the job belongs to the consumer/regulator core to find the best match. However, looking at implementations in existing regulators and based on your explanation, it seems to be the job of the regulator driver rather than the consumer/ regulator core to provide the best match. Thanks for the discussion and explanation.
On Fri, Jun 21, 2013 at 07:43:55AM -0500, Nishanth Menon wrote: > On 10:51-20130621, Mark Brown wrote: > > No, the consumer really doesn't want to be aware of linear step > > regulators. Why would it care that there even are linear steps? If > > the consumer is doing this based on the properties of the regulator > > rather than on the properties of the consumer this indicates that the > > consumer has a problem If the consumer is doing this based on the > > properties of the regulator rather than on the properties of the > > consumer this indicates that the consumer has a problem > The specific case that I am trying to tackle is as follows: > cpufreq-cpu0 uses definitions of voltages that are SoC specific. For a > given frequency, the optimal voltage is X, max voltage(Y) is already > expected to be in constraints for device functionality. We however want > to find the closest voltage for a regulator in range X to Y best > achievable by regulator. I think the area where I am getting confused is > this: I am thinking the job belongs to the consumer/regulator core to > find the best match. However, looking at implementations in existing > regulators and based on your explanation, it seems to be the job of > the regulator driver rather than the consumer/ regulator core to provide > the best match. Right, though the consumer does have to provide a voltage range to the regulator to allow this to happen - if the consumer doesn't provide a range then there's only one option available. The consumer provides a range and then the driver satistifes that as best it can (after it's been filtered through the constraints).
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 288c75a..98c96b2 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -2407,6 +2407,9 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev, } } else if (rdev->desc->ops->set_voltage_sel) { + if (min_uV == max_uV && rdev->desc->uV_step) + max_uV += rdev->desc->uV_step; + if (rdev->desc->ops->map_voltage) { ret = rdev->desc->ops->map_voltage(rdev, min_uV, max_uV);
Regulator consumers are not aware of the characteristics of regulator used to supply. For example: consumerX requests for voltage min_uV = 500mV, max_uV = 500mV On a regulator which has a step size of 10mV, this can be exactly achieved. However, on a regulator which is non-exact divider step size (example 12.66mV step size), the closest achievable would be 506.4. regulator_set_voltage_tol does not work out either as <500mV is not an operational voltage. Account for step size accuracy when exact voltage requests are send for step based regulators. Signed-off-by: Nishanth Menon <nm@ti.com> --- The specific example I faced was using cpufreq-cpu0 driver with voltages for OPPs for MPU rail and attempting the common definitions against voltages that are non-exact multiples of stepsize of PMIC. The alternative would be implement custom set_voltage (as againsta simpler set_voltage_sel and using linear map/list functions) for the regulator which will account for the same. Yet another alternative might be to introduce yet another custom function similar to regulator_set_voltage_tol which accounts for this. something like: regulator_set_voltage_floor(regulator, voltage, tol) or something to that effect. drivers/regulator/core.c | 3 +++ 1 file changed, 3 insertions(+)