| Message ID | 1352800177-16139-1-git-send-email-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 13, 2012 at 10:49:37AM +0100, Marek Szyprowski wrote: > + if (rdev->constraints->valid_ops_mask & REGULATOR_CHANGE_VOLTAGE) { > + if (rdev->desc->n_voltages) > + return rdev->desc->n_voltages; > + else > + return -EINVAL; > + } else { > + return 1; > + } Hrm, now I can read the logic I'm not convinced this is a good idea. This will report that we have an available voltage for devices which don't know their voltage (things like battery supplies often do this as the voltage is unregulated) and it will mean that we are doing something different for the case where there's only one voltage (reporting the restricted count instead of the physically supported count). I think we want a regulator_can_change_voltage() or possibly a count function (though I can't see any use cases except this) which answers the question directly instead of layering on top of this function.
Hello, On 11/14/2012 3:01 AM, Mark Brown wrote: > On Tue, Nov 13, 2012 at 10:49:37AM +0100, Marek Szyprowski wrote: > > > + if (rdev->constraints->valid_ops_mask & REGULATOR_CHANGE_VOLTAGE) { > > + if (rdev->desc->n_voltages) > > + return rdev->desc->n_voltages; > > + else > > + return -EINVAL; > > + } else { > > + return 1; > > + } > > Hrm, now I can read the logic I'm not convinced this is a good idea. > This will report that we have an available voltage for devices which > don't know their voltage (things like battery supplies often do this as > the voltage is unregulated) and it will mean that we are doing something > different for the case where there's only one voltage (reporting the > restricted count instead of the physically supported count). > > I think we want a regulator_can_change_voltage() or possibly a count > function (though I can't see any use cases except this) which answers > the question directly instead of layering on top of this function. Right, regulator_can_change_voltage() sounds much better than my hacky approach. The first client would be probably sdhci/mmc driver, as 'can_change_voltage' check sounds much more appropriate than counting available voltage values. I will prepare patches soon. Best regards
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 042c1ff..d07c240 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -1872,7 +1872,14 @@ int regulator_count_voltages(struct regulator *regulator) { struct regulator_dev *rdev = regulator->rdev; - return rdev->desc->n_voltages ? : -EINVAL; + if (rdev->constraints->valid_ops_mask & REGULATOR_CHANGE_VOLTAGE) { + if (rdev->desc->n_voltages) + return rdev->desc->n_voltages; + else + return -EINVAL; + } else { + return 1; + } } EXPORT_SYMBOL_GPL(regulator_count_voltages);
Some drivers has additional logic for fixed regulators. Let regulator core to treat regulators which cannot change their voltage due to applied constraints as fixed. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/regulator/core.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)