[PATCH v2] pwm-regulator: Add support for a fixed delay after duty cycle changes

[PATCH v2] pwm-regulator: Add support for a fixed delay after duty cycle changes

[Matthias Kaehlcke]

A change of the duty cycle doesn't necessarily cause an inmediate switch
to the target voltage. The voltage change may be gradual and complete
with a certain delay. This change introduces the device tree properties
"settle-time-up-us" and "settle-time-down-us", which allow to specify a
fixed delay after a voltage increase or decrease. Often it is not
strictly necessary for a voltage decrease to complete, therefore the
delays may be asymmetric. For regulators with a ramp delay the
corresponding settle time is added to the ramp delay.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 .../bindings/regulator/pwm-regulator.txt           | 10 +++++++++
 drivers/regulator/pwm-regulator.c                  | 25 ++++++++++++++++++----
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
index 3aeba9f..42b6819 100644
--- a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
@@ -29,6 +29,16 @@ Required properties:
 
 - pwms:			PWM specification (See: ../pwm/pwm.txt)
 
+Optional properties:
+--------------------
+- settle-time-up-us:	Time to settle down after a voltage increase
+			(unit: us). For regulators with a ramp delay
+			the two values are added.
+
+- settle-time-down-us:	Time to settle down after a voltage decrease
+			(unit: us). For regulators with a ramp delay
+			the two values are added.
+
 Only required for Voltage Table Mode:
 - voltage-table: 	Voltage and Duty-Cycle table consisting of 2 cells
 			    First cell is voltage in microvolts (uV)
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index c245242..51fe4da 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -46,6 +46,9 @@ struct pwm_regulator_data {
 
 	int state;
 
+	u32 settle_time_up_us;
+	u32 settle_time_down_us;
+
 	/* Enable GPIO */
 	struct gpio_desc *enb_gpio;
 };
@@ -195,6 +198,7 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
 	unsigned int max_uV_duty = drvdata->continuous.max_uV_dutycycle;
 	unsigned int duty_unit = drvdata->continuous.dutycycle_unit;
 	unsigned int ramp_delay = rdev->constraints->ramp_delay;
+	unsigned int delay;
 	int min_uV = rdev->constraints->min_uV;
 	int max_uV = rdev->constraints->max_uV;
 	int diff_uV = max_uV - min_uV;
@@ -233,12 +237,20 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
 		return ret;
 	}
 
-	if ((ramp_delay == 0) || !pwm_regulator_is_enabled(rdev))
+	if (req_min_uV > old_uV)
+		delay = drvdata->settle_time_up_us;
+	else
+		delay = drvdata->settle_time_down_us;
+
+	if (!pwm_regulator_is_enabled(rdev) ||
+		((delay == 0) && (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));
+	if (ramp_delay != 0)
+		/* Adjust ramp delay to uS and add to settle time. */
+		delay += DIV_ROUND_UP(abs(req_min_uV - old_uV), ramp_delay);
+
+	usleep_range(delay, delay + DIV_ROUND_UP(delay, 10));
 
 	return 0;
 }
@@ -368,6 +380,11 @@ static int pwm_regulator_probe(struct platform_device *pdev)
 	if (!init_data)
 		return -ENOMEM;
 
+	of_property_read_u32(np, "settle-time-up-us",
+			&drvdata->settle_time_up_us);
+	of_property_read_u32(np, "settle-time-down-us",
+			&drvdata->settle_time_down_us);
+
 	config.of_node = np;
 	config.dev = &pdev->dev;
 	config.driver_data = drvdata;
-- 
2.6.6

Re: [PATCH v2] pwm-regulator: Add support for a fixed delay after duty cycle changes

[Doug Anderson]

Hi,

On Fri, Aug 26, 2016 at 5:20 PM, Matthias Kaehlcke <mka@chromium.org> wrote:
> A change of the duty cycle doesn't necessarily cause an inmediate switch
> to the target voltage. The voltage change may be gradual and complete
> with a certain delay. This change introduces the device tree properties
> "settle-time-up-us" and "settle-time-down-us", which allow to specify a
> fixed delay after a voltage increase or decrease. Often it is not
> strictly necessary for a voltage decrease to complete, therefore the
> delays may be asymmetric. For regulators with a ramp delay the
> corresponding settle time is added to the ramp delay.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  .../bindings/regulator/pwm-regulator.txt           | 10 +++++++++
>  drivers/regulator/pwm-regulator.c                  | 25 ++++++++++++++++++----
>  2 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
> index 3aeba9f..42b6819 100644
> --- a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
> +++ b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
> @@ -29,6 +29,16 @@ Required properties:
>
>  - pwms:                        PWM specification (See: ../pwm/pwm.txt)
>
> +Optional properties:
> +--------------------
> +- settle-time-up-us:   Time to settle down after a voltage increase
> +                       (unit: us). For regulators with a ramp delay
> +                       the two values are added.
> +
> +- settle-time-down-us: Time to settle down after a voltage decrease
> +                       (unit: us). For regulators with a ramp delay
> +                       the two values are added.

Based on investigations that we've been doing recently, it might make
sense to leave "settle-time-down-us" out of this patch for now.  From
how our EEs appear to have designed our hardware:

* There's no "ramp up" time that's in terms of uV / uS, just a
"settle-time-up-us" like you have here.

* For going down there's not really a settle time or a ramp down time.
The regulator will just fall at a rate that depends on the current
load.  ...and it's kinda slow / not worth it to block waiting for the
regulator to go down.

* We may want to introduce some other properties related to the
downward ramp to keep the regulator from falling too fast.  On our
board if you drop down too fast you can trigger the PWM regulator over
voltage protection, so on downward ramps we might have to break into
additional steps.


To say it another way:

- settle-time-up-us looks great.

- settle-time-down-us isn't useful on our board and it's probably
better to wait until there's a user before adding this property.

- future properties will come to help break the downward transition
into multiple steps.


Does that sound sane?

-Doug