On Sun, 10 Feb 2019 at 10:49, Simon Glass sjg@chromium.org wrote:
Hi Krzysztof,
On Sat, 9 Feb 2019 at 16:54, Krzysztof Kozlowski krzk@kernel.org wrote:
Changing voltage and enabling regulator might require delays so the regulator stabilizes at expected level.
Add support for "regulator-ramp-delay" binding which can introduce required time to both enabling the regulator and to changing the voltage.
Is this binding used in Linux? Can you please add binding documentation for this?
Yes, these are bindings from the kernel. I will add them.
Signed-off-by: Krzysztof Kozlowski krzk@kernel.org
drivers/power/regulator/regulator-uclass.c | 45 +++++++++++++++++++++- include/power/regulator.h | 2 + 2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 39e46279d533..4119f244c74b 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -35,10 +35,22 @@ int regulator_get_value(struct udevice *dev) return ops->get_value(dev); }
+static void regulator_set_value_delay(struct udevice *dev, int old_uV,
int new_uV, unsigned int ramp_delay)+{
int delay = DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);So the ramp delay is microseconds per (microvolt delta)?
The ramp delay is microvolt per microseconds. int delay = uv / (uV/uS) = uS
debug("regulator %s: delay %u us (%d uV -> %d uV)\n", dev->name,delay, old_uV, new_uV);udelay(delay);+}
int regulator_set_value(struct udevice *dev, int uV) { const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); struct dm_regulator_uclass_platdata *uc_pdata;
int ret, old_uV = uV; uc_pdata = dev_get_uclass_platdata(dev); if (uc_pdata->min_uV != -ENODATA && uV < uc_pdata->min_uV)@@ -49,7 +61,18 @@ int regulator_set_value(struct udevice *dev, int uV) if (!ops || !ops->set_value) return -ENOSYS;
return ops->set_value(dev, uV);
if (uc_pdata->ramp_delay)old_uV = regulator_get_value(dev);ret = ops->set_value(dev, uV);if (!ret) {if (uc_pdata->ramp_delay && old_uV > 0)regulator_set_value_delay(dev, old_uV, uV,uc_pdata->ramp_delay);}return ret;}
/* @@ -107,6 +130,7 @@ int regulator_set_enable(struct udevice *dev, bool enable) { const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); struct dm_regulator_uclass_platdata *uc_pdata;
int ret, old_enable = 0; if (!ops || !ops->set_enable) return -ENOSYS;@@ -115,7 +139,22 @@ int regulator_set_enable(struct udevice *dev, bool enable) if (!enable && uc_pdata->always_on) return 0;
return ops->set_enable(dev, enable);
if (uc_pdata->ramp_delay)old_enable = regulator_get_enable(dev);ret = ops->set_enable(dev, enable);if (!ret) {if (uc_pdata->ramp_delay && !old_enable) {int uV = regulator_get_value(dev);if (uV > 0) {regulator_set_value_delay(dev, 0, uV,uc_pdata->ramp_delay);}How come there is a delay when enabling it as well as when setting the voltage?
Enabling a regulator also requires the time, which might be sum of: 1. Initial enable delay (not included here), 2. Rising of voltage delay (ramp delay) from 0 to expected.
In Linux kernel this is separate property regulator-enable-ramp-delay. Here I reused the ramp delay to make it simpler.
However indeed there is no point to wait with ramp delay if the regulator is disabled.
}}return ret;}
int regulator_get_mode(struct udevice *dev) @@ -324,6 +363,8 @@ static int regulator_pre_probe(struct udevice *dev) -ENODATA); uc_pdata->always_on = dev_read_bool(dev, "regulator-always-on"); uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay",0); /* Those values are optional (-ENODATA if unset) */ if ((uc_pdata->min_uV != -ENODATA) &&diff --git a/include/power/regulator.h b/include/power/regulator.h index 5318ab3acece..c13fa1f336e5 100644 --- a/include/power/regulator.h +++ b/include/power/regulator.h @@ -149,6 +149,7 @@ enum regulator_flag {
- @max_uA* - maximum amperage (micro Amps)
- @always_on* - bool type, true or false
- @boot_on* - bool type, true or false
- @ramp_delay - Time to settle down after voltage change (unit: uV/us)
us/uV isn't it?
No, opposite.
- TODO(sjg@chromium.org): Consider putting the above two into @flags
This comment needs updating or moving up.
Yes, thanks!
Best regards, Krzysztof