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

[PATCH] 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 voltage. For regulators with a ramp delay
the corresponding settle time is added to the ramp delay.

Change-Id: Ib3543a250e1dfc14964ec72bd2684237728a08a3
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 .../bindings/regulator/pwm-regulator.txt           | 10 +++++++++
 drivers/regulator/pwm-regulator.c                  | 24 ++++++++++++++++++----
 2 files changed, 30 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..cd7575b 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,19 @@ 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 +379,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] pwm-regulator: Add support for a fixed delay after duty cycle changes

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4076 bytes --]

Hi Matthias,

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.8-rc3 next-20160825]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Matthias-Kaehlcke/pwm-regulator-Add-support-for-a-fixed-delay-after-duty-cycle-changes/20160827-071710
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: i386-randconfig-x018-201634 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/linkage.h:4:0,
                    from include/linux/kernel.h:6,
                    from include/linux/delay.h:10,
                    from drivers/regulator/pwm-regulator.c:13:
   drivers/regulator/pwm-regulator.c: In function 'pwm_regulator_set_voltage':
   drivers/regulator/pwm-regulator.c:245:54: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
     if (!pwm_regulator_is_enabled(rdev) || (delay == 0) && (ramp_delay == 0))
                                            ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:149:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> drivers/regulator/pwm-regulator.c:245:2: note: in expansion of macro 'if'
     if (!pwm_regulator_is_enabled(rdev) || (delay == 0) && (ramp_delay == 0))
     ^~
   drivers/regulator/pwm-regulator.c:245:54: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
     if (!pwm_regulator_is_enabled(rdev) || (delay == 0) && (ramp_delay == 0))
                                            ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:149:42: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                             ^~~~
>> drivers/regulator/pwm-regulator.c:245:2: note: in expansion of macro 'if'
     if (!pwm_regulator_is_enabled(rdev) || (delay == 0) && (ramp_delay == 0))
     ^~
   drivers/regulator/pwm-regulator.c:245:54: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
     if (!pwm_regulator_is_enabled(rdev) || (delay == 0) && (ramp_delay == 0))
                                            ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:160:16: note: in definition of macro '__trace_if'
      ______r = !!(cond);     \
                   ^~~~
>> drivers/regulator/pwm-regulator.c:245:2: note: in expansion of macro 'if'
     if (!pwm_regulator_is_enabled(rdev) || (delay == 0) && (ramp_delay == 0))
     ^~

vim +/if +245 drivers/regulator/pwm-regulator.c

   229		else
   230			dutycycle = min_uV_duty + dutycycle;
   231	
   232		pwm_set_relative_duty_cycle(&pstate, dutycycle, duty_unit);
   233	
   234		ret = pwm_apply_state(drvdata->pwm, &pstate);
   235		if (ret) {
   236			dev_err(&rdev->dev, "Failed to configure PWM: %d\n", ret);
   237			return ret;
   238		}
   239	
   240		if (req_min_uV > old_uV)
   241	                delay = drvdata->settle_time_up_us;
   242	        else
   243	                delay = drvdata->settle_time_down_us;
   244	
 > 245		if (!pwm_regulator_is_enabled(rdev) || (delay == 0) && (ramp_delay == 0))
   246			return 0;
   247	
   248		if (ramp_delay != 0)
   249			/* Adjust ramp delay to uS and add to settle time. */
   250			delay += DIV_ROUND_UP(abs(req_min_uV - old_uV), ramp_delay);
   251	
   252		usleep_range(delay, delay + DIV_ROUND_UP(delay, 10));
   253	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 28069 bytes --]

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2326 bytes --]

Hi Matthias,

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.8-rc3 next-20160825]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Matthias-Kaehlcke/pwm-regulator-Add-support-for-a-fixed-delay-after-duty-cycle-changes/20160827-071710
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: i386-randconfig-x017-201634 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/regulator/pwm-regulator.c: In function 'pwm_regulator_set_voltage':
>> drivers/regulator/pwm-regulator.c:245:54: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
     if (!pwm_regulator_is_enabled(rdev) || (delay == 0) && (ramp_delay == 0))
                                            ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~

vim +245 drivers/regulator/pwm-regulator.c

   229		else
   230			dutycycle = min_uV_duty + dutycycle;
   231	
   232		pwm_set_relative_duty_cycle(&pstate, dutycycle, duty_unit);
   233	
   234		ret = pwm_apply_state(drvdata->pwm, &pstate);
   235		if (ret) {
   236			dev_err(&rdev->dev, "Failed to configure PWM: %d\n", ret);
   237			return ret;
   238		}
   239	
   240		if (req_min_uV > old_uV)
   241	                delay = drvdata->settle_time_up_us;
   242	        else
   243	                delay = drvdata->settle_time_down_us;
   244	
 > 245		if (!pwm_regulator_is_enabled(rdev) || (delay == 0) && (ramp_delay == 0))
   246			return 0;
   247	
   248		if (ramp_delay != 0)
   249			/* Adjust ramp delay to uS and add to settle time. */
   250			delay += DIV_ROUND_UP(abs(req_min_uV - old_uV), ramp_delay);
   251	
   252		usleep_range(delay, delay + DIV_ROUND_UP(delay, 10));
   253	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 21900 bytes --]

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

[Javier Martinez Canillas]

Hello Matthias,

On Fri, Aug 26, 2016 at 7:13 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 voltage. For regulators with a ramp delay
> the corresponding settle time is added to the ramp delay.
>
> Change-Id: Ib3543a250e1dfc14964ec72bd2684237728a08a3

You should remove this tag before posting since is not suitable for mainline.

> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---

Best regards,
Javier