Re: [PATCH] regulator: pwm: Fix regulator ramp delay for continuous mode

From: Caesar Wang
Date: Tue Jun 28 2016 - 06:32:37 EST



On 2016å06æ28æ 12:53, Douglas Anderson wrote:
The original commit adding support for continuous voltage mode didn't
handle the regulator ramp delay properly. It treated the delay as a
fixed delay in uS despite the property being defined as uV / uS. Let's
adjust it. Luckily there appear to be no users of this ramp delay for
PWM regulators (as per grepping through device trees in linuxnext).

Note also that the upper bound of usleep_range probably shouldn't be a
full 1 ms longer than the lower bound since I've seen plenty of hardware
with a ramp rate of ~5000 uS / uV and for small jumps the total delays
are in the tens of uS. 1000 is way too much. We'll try to be dynamic
and use 10%

I'm agree with the dynamic and use 10%.


Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>

Tested-by: Caesar Wang <wxt@xxxxxxxxxxxxxx>

Tested for my rk3399 board. That's still happy work for my board.
..
[ 2891.541958] pwm_regulator_set_voltage: delay=38, min-v=800000, old-v=1024000
[ 2898.188785] pwm_regulator_set_voltage: delay=13, min-v=875000, old-v=800000
[ 2898.211873] pwm_regulator_set_voltage: delay=8, min-v=925000, old-v=877000
[ 2898.312026] pwm_regulator_set_voltage: delay=21, min-v=-800000, old-v=926000
..



---
Note that this patch is atop Boris's recent PWM regulator fixes. If
desired it wouldn't be too hard to write it atop the old code, though
quite honestly anyone using a PWM regulator should probably be using his
new code.

drivers/regulator/pwm-regulator.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index fa1c74c77bb0..de94d19f6e1f 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -188,6 +188,7 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
struct pwm_state pstate;
unsigned int diff_duty;
unsigned int dutycycle;
+ int old_uV = pwm_regulator_get_voltage(rdev);
int ret;
pwm_init_state(drvdata->pwm, &pstate);
@@ -219,8 +220,12 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
return ret;
}
- /* Delay required by PWM regulator to settle to the new voltage */
- usleep_range(ramp_delay, ramp_delay + 1000);
+ if (ramp_delay == 0)
+ return 0;
+
+ /* Ramp delay is in uV/uS. Adjust to uS and delay */
+ ramp_delay = DIV_ROUND_UP(abs(req_min_uV - old_uV), ramp_delay);
+ usleep_range(ramp_delay, ramp_delay + DIV_ROUND_UP(ramp_delay, 10));
return 0;
}