All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 1/6] regulator: core: Use local ops variable in _regulator_do_set_voltage()
@ 2016-09-14 16:52 Matthias Kaehlcke
  2016-09-14 16:52 ` [PATCH v5 2/6] regulator: core: Simplify error flow " Matthias Kaehlcke
                   ` (5 more replies)
  0 siblings, 6 replies; 20+ messages in thread
From: Matthias Kaehlcke @ 2016-09-14 16:52 UTC (permalink / raw)
  To: Mark Brown, lgirdwood
  Cc: Douglas Anderson, briannorris, javier, robh+dt, mark.rutland,
	linux-kernel, devicetree, Matthias Kaehlcke

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v5:
- This patch is new for v5.

 drivers/regulator/core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index db320e8..b059e83 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2751,6 +2751,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	int best_val = 0;
 	unsigned int selector;
 	int old_selector = -1;
+	const struct regulator_ops *ops = rdev->desc->ops;
 
 	trace_regulator_set_voltage(rdev_get_name(rdev), min_uV, max_uV);
 
@@ -2762,29 +2763,28 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	 * info to call set_voltage_time_sel().
 	 */
 	if (_regulator_is_enabled(rdev) &&
-	    rdev->desc->ops->set_voltage_time_sel &&
-	    rdev->desc->ops->get_voltage_sel) {
-		old_selector = rdev->desc->ops->get_voltage_sel(rdev);
+	    ops->set_voltage_time_sel && ops->get_voltage_sel) {
+		old_selector = ops->get_voltage_sel(rdev);
 		if (old_selector < 0)
 			return old_selector;
 	}
 
-	if (rdev->desc->ops->set_voltage) {
+	if (ops->set_voltage) {
 		ret = _regulator_call_set_voltage(rdev, min_uV, max_uV,
 						  &selector);
 
 		if (ret >= 0) {
-			if (rdev->desc->ops->list_voltage)
-				best_val = rdev->desc->ops->list_voltage(rdev,
-									 selector);
+			if (ops->list_voltage)
+				best_val = ops->list_voltage(rdev,
+							     selector);
 			else
 				best_val = _regulator_get_voltage(rdev);
 		}
 
-	} else if (rdev->desc->ops->set_voltage_sel) {
+	} else if (ops->set_voltage_sel) {
 		ret = regulator_map_voltage(rdev, min_uV, max_uV);
 		if (ret >= 0) {
-			best_val = rdev->desc->ops->list_voltage(rdev, ret);
+			best_val = ops->list_voltage(rdev, ret);
 			if (min_uV <= best_val && max_uV >= best_val) {
 				selector = ret;
 				if (old_selector == selector)
@@ -2804,7 +2804,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	if (ret == 0 && !rdev->constraints->ramp_disable && old_selector >= 0
 		&& old_selector != selector) {
 
-		delay = rdev->desc->ops->set_voltage_time_sel(rdev,
+		delay = ops->set_voltage_time_sel(rdev,
 						old_selector, selector);
 		if (delay < 0) {
 			rdev_warn(rdev, "set_voltage_time_sel() failed: %d\n",
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 2/6] regulator: core: Simplify error flow in _regulator_do_set_voltage()
  2016-09-14 16:52 [PATCH v5 1/6] regulator: core: Use local ops variable in _regulator_do_set_voltage() Matthias Kaehlcke
@ 2016-09-14 16:52 ` Matthias Kaehlcke
  2016-09-16 17:40     ` Mark Brown
  2016-09-14 16:52 ` [PATCH v5 3/6] regulator: core: Don't skip set_voltage_time when ramp delay disabled Matthias Kaehlcke
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Matthias Kaehlcke @ 2016-09-14 16:52 UTC (permalink / raw)
  To: Mark Brown, lgirdwood
  Cc: Douglas Anderson, briannorris, javier, robh+dt, mark.rutland,
	linux-kernel, devicetree, Matthias Kaehlcke

If the voltage can not be set jump to the end of the function. This
avoids having to check for an error multiple times and eliminates one
level of nesting in a follow-up change.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v5:
- This patch is new for v5.

 drivers/regulator/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b059e83..b0076cc 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2800,8 +2800,11 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 		ret = -EINVAL;
 	}
 
+	if (ret)
+		goto out;
+
 	/* Call set_voltage_time_sel if successfully obtained old_selector */
-	if (ret == 0 && !rdev->constraints->ramp_disable && old_selector >= 0
+	if (!rdev->constraints->ramp_disable && old_selector >= 0
 		&& old_selector != selector) {
 
 		delay = ops->set_voltage_time_sel(rdev,
@@ -2821,13 +2824,14 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 		}
 	}
 
-	if (ret == 0 && best_val >= 0) {
+	if (best_val >= 0) {
 		unsigned long data = best_val;
 
 		_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE,
 				     (void *)data);
 	}
 
+out:
 	trace_regulator_set_voltage_complete(rdev_get_name(rdev), best_val);
 
 	return ret;
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 3/6] regulator: core: Don't skip set_voltage_time when ramp delay disabled
  2016-09-14 16:52 [PATCH v5 1/6] regulator: core: Use local ops variable in _regulator_do_set_voltage() Matthias Kaehlcke
  2016-09-14 16:52 ` [PATCH v5 2/6] regulator: core: Simplify error flow " Matthias Kaehlcke
@ 2016-09-14 16:52 ` Matthias Kaehlcke
  2016-09-16 17:40     ` Mark Brown
  2016-09-14 16:52 ` [PATCH v5 4/6] regulator: core: Add set_voltage_time op Matthias Kaehlcke
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Matthias Kaehlcke @ 2016-09-14 16:52 UTC (permalink / raw)
  To: Mark Brown, lgirdwood
  Cc: Douglas Anderson, briannorris, javier, robh+dt, mark.rutland,
	linux-kernel, devicetree, Matthias Kaehlcke

The current code assumes that only the ramp_delay is used to determine
the time needed for the voltage to stabilize. This may be true for the
calculation done by regulator_set_voltage_time_sel(), however regulators
can implement their own set_voltage_time_sel() op which would be skipped
if no ramp delay is specified. Remove the check in
_regulator_do_set_voltage(), the functions calculating the ramp delay
return 0 anyway when the ramp delay is not configured.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v5:
- This patch is new for v5.

 drivers/regulator/core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b0076cc..df3028b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2804,9 +2804,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 		goto out;
 
 	/* Call set_voltage_time_sel if successfully obtained old_selector */
-	if (!rdev->constraints->ramp_disable && old_selector >= 0
-		&& old_selector != selector) {
-
+	if (!old_selector >= 0 && old_selector != selector) {
 		delay = ops->set_voltage_time_sel(rdev,
 						old_selector, selector);
 		if (delay < 0) {
@@ -3051,10 +3049,8 @@ int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 	else if (rdev->desc->ramp_delay)
 		ramp_delay = rdev->desc->ramp_delay;
 
-	if (ramp_delay == 0) {
-		rdev_warn(rdev, "ramp_delay not set\n");
+	if (ramp_delay == 0)
 		return 0;
-	}
 
 	/* sanity check */
 	if (!rdev->desc->ops->list_voltage)
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 4/6] regulator: core: Add set_voltage_time op
  2016-09-14 16:52 [PATCH v5 1/6] regulator: core: Use local ops variable in _regulator_do_set_voltage() Matthias Kaehlcke
  2016-09-14 16:52 ` [PATCH v5 2/6] regulator: core: Simplify error flow " Matthias Kaehlcke
  2016-09-14 16:52 ` [PATCH v5 3/6] regulator: core: Don't skip set_voltage_time when ramp delay disabled Matthias Kaehlcke
@ 2016-09-14 16:52 ` Matthias Kaehlcke
  2016-09-16 17:40     ` Mark Brown
  2016-09-14 16:52   ` Matthias Kaehlcke
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 20+ messages in thread
From: Matthias Kaehlcke @ 2016-09-14 16:52 UTC (permalink / raw)
  To: Mark Brown, lgirdwood
  Cc: Douglas Anderson, briannorris, javier, robh+dt, mark.rutland,
	linux-kernel, devicetree, Matthias Kaehlcke

The new op is analogous to set_voltage_time_sel. It can be used by
regulators which don't have a table of discrete voltages. The function
returns the time for the regulator output voltage to stabilize after
being set to a new value, in microseconds. If the op is not set a
default implementation is used to calculate the delay.

This change also removes the ramp_delay calculation in the PWM
regulator, since the driver now uses the core code for the calculation
of the delay.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v5:
- Use _regulator_set_voltage_time() as default if set_voltage_time op is not set
- Split out some changes in separate patche to improve readability of the diff
- Use conditional logic in _regulator_do_set_voltage() instead of gotos
- Reworked regulator_set_voltage_time() to make diff more readable
- Removed ramp delay from PWM regulator
- Updated comments in include/linux/regulator/driver.h
- Updated commit message

 drivers/regulator/core.c          | 86 +++++++++++++++++++++++++++------------
 drivers/regulator/pwm-regulator.c | 10 -----
 include/linux/regulator/driver.h  | 10 ++++-
 3 files changed, 67 insertions(+), 39 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index df3028b..c52fc0c 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2743,6 +2743,24 @@ static int _regulator_call_set_voltage_sel(struct regulator_dev *rdev,
 	return ret;
 }
 
+static int _regulator_set_voltage_time(struct regulator_dev *rdev,
+				       int old_uV, int new_uV)
+{
+	unsigned int ramp_delay = 0;
+
+	if (rdev->constraints->ramp_delay)
+		ramp_delay = rdev->constraints->ramp_delay;
+	else if (rdev->desc->ramp_delay)
+		ramp_delay = rdev->desc->ramp_delay;
+
+	if (ramp_delay == 0) {
+		rdev_warn(rdev, "ramp_delay not set\n");
+		return 0;
+	}
+
+	return DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
+}
+
 static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 				     int min_uV, int max_uV)
 {
@@ -2752,6 +2770,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	unsigned int selector;
 	int old_selector = -1;
 	const struct regulator_ops *ops = rdev->desc->ops;
+	int old_uV = _regulator_get_voltage(rdev);
 
 	trace_regulator_set_voltage(rdev_get_name(rdev), min_uV, max_uV);
 
@@ -2803,23 +2822,37 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	if (ret)
 		goto out;
 
-	/* Call set_voltage_time_sel if successfully obtained old_selector */
-	if (!old_selector >= 0 && old_selector != selector) {
-		delay = ops->set_voltage_time_sel(rdev,
-						old_selector, selector);
-		if (delay < 0) {
-			rdev_warn(rdev, "set_voltage_time_sel() failed: %d\n",
-				  delay);
-			delay = 0;
+	if (ops->set_voltage_time_sel) {
+		/*
+		 * Call set_voltage_time_sel if successfully obtained
+		 * old_selector
+		 */
+		if (old_selector >= 0 && old_selector != selector)
+			delay = ops->set_voltage_time_sel(rdev, old_selector,
+							  selector);
+	} else {
+		if (old_uV != best_val) {
+			if (ops->set_voltage_time)
+				delay = ops->set_voltage_time(rdev, old_uV,
+							      best_val);
+			else
+				delay = _regulator_set_voltage_time(rdev,
+								    old_uV,
+								    best_val);
 		}
+	}
 
-		/* Insert any necessary delays */
-		if (delay >= 1000) {
-			mdelay(delay / 1000);
-			udelay(delay % 1000);
-		} else if (delay) {
-			udelay(delay);
-		}
+	if (delay < 0) {
+		rdev_warn(rdev, "failed to get delay: %d\n", delay);
+		delay = 0;
+	}
+
+	/* Insert any necessary delays */
+	if (delay >= 1000) {
+		mdelay(delay / 1000);
+		udelay(delay % 1000);
+	} else if (delay) {
+		udelay(delay);
 	}
 
 	if (best_val >= 0) {
@@ -3000,9 +3033,13 @@ int regulator_set_voltage_time(struct regulator *regulator,
 	int voltage;
 	int i;
 
+	if (ops->set_voltage_time)
+		return ops->set_voltage_time(rdev, old_uV, new_uV);
+	else if (!ops->set_voltage_time_sel)
+		return _regulator_set_voltage_time(rdev, old_uV, new_uV);
+
 	/* Currently requires operations to do this */
-	if (!ops->list_voltage || !ops->set_voltage_time_sel
-	    || !rdev->desc->n_voltages)
+	if (!ops->list_voltage || !rdev->desc->n_voltages)
 		return -EINVAL;
 
 	for (i = 0; i < rdev->desc->n_voltages; i++) {
@@ -3041,17 +3078,8 @@ int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 				   unsigned int old_selector,
 				   unsigned int new_selector)
 {
-	unsigned int ramp_delay = 0;
 	int old_volt, new_volt;
 
-	if (rdev->constraints->ramp_delay)
-		ramp_delay = rdev->constraints->ramp_delay;
-	else if (rdev->desc->ramp_delay)
-		ramp_delay = rdev->desc->ramp_delay;
-
-	if (ramp_delay == 0)
-		return 0;
-
 	/* sanity check */
 	if (!rdev->desc->ops->list_voltage)
 		return -EINVAL;
@@ -3059,7 +3087,11 @@ int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 	old_volt = rdev->desc->ops->list_voltage(rdev, old_selector);
 	new_volt = rdev->desc->ops->list_voltage(rdev, new_selector);
 
-	return DIV_ROUND_UP(abs(new_volt - old_volt), ramp_delay);
+	if (rdev->desc->ops->set_voltage_time)
+		return rdev->desc->ops->set_voltage_time(rdev, old_volt,
+							 new_volt);
+	else
+		return _regulator_set_voltage_time(rdev, old_volt, new_volt);
 }
 EXPORT_SYMBOL_GPL(regulator_set_voltage_time_sel);
 
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index c245242..1b88e0e1 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -10,7 +10,6 @@
  * published by the Free Software Foundation.
  */
 
-#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/err.h>
@@ -194,12 +193,10 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
 	unsigned int min_uV_duty = drvdata->continuous.min_uV_dutycycle;
 	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;
 	int min_uV = rdev->constraints->min_uV;
 	int max_uV = rdev->constraints->max_uV;
 	int diff_uV = max_uV - min_uV;
 	struct pwm_state pstate;
-	int old_uV = pwm_regulator_get_voltage(rdev);
 	unsigned int diff_duty;
 	unsigned int dutycycle;
 	int ret;
@@ -233,13 +230,6 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
 		return ret;
 	}
 
-	if ((ramp_delay == 0) || !pwm_regulator_is_enabled(rdev))
-		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;
 }
 
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index fcfa40a..37b5324 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -113,10 +113,14 @@ struct regulator_linear_range {
  *               stabilise after being enabled, in microseconds.
  * @set_ramp_delay: Set the ramp delay for the regulator. The driver should
  *		select ramp delay equal to or less than(closest) ramp_delay.
+ * @set_voltage_time: Time taken for the regulator voltage output voltage
+ *               to stabilise after being set to a new value, in microseconds.
+ *               The function receives the from and to voltage as input, it
+ *               should return the worst case.
  * @set_voltage_time_sel: Time taken for the regulator voltage output voltage
  *               to stabilise after being set to a new value, in microseconds.
- *               The function provides the from and to voltage selector, the
- *               function should return the worst case.
+ *               The function receives the from and to voltage selector as
+ *               input, it should return the worst case.
  * @set_soft_start: Enable soft start for the regulator.
  *
  * @set_suspend_voltage: Set the voltage for the regulator when the system
@@ -168,6 +172,8 @@ struct regulator_ops {
 	/* Time taken to enable or set voltage on the regulator */
 	int (*enable_time) (struct regulator_dev *);
 	int (*set_ramp_delay) (struct regulator_dev *, int ramp_delay);
+	int (*set_voltage_time) (struct regulator_dev *, int old_uV,
+				 int new_uV);
 	int (*set_voltage_time_sel) (struct regulator_dev *,
 				     unsigned int old_selector,
 				     unsigned int new_selector);
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 5/6] regulator: core: Add support for a fixed delay after voltage changes
@ 2016-09-14 16:52   ` Matthias Kaehlcke
  0 siblings, 0 replies; 20+ messages in thread
From: Matthias Kaehlcke @ 2016-09-14 16:52 UTC (permalink / raw)
  To: Mark Brown, lgirdwood
  Cc: Douglas Anderson, briannorris, javier, robh+dt, mark.rutland,
	linux-kernel, devicetree, Matthias Kaehlcke

The target voltage is not necessarily reached inmediately after
requesting a regulator to change the voltage. In some cases the
ramp_delay can be used to calculate the stabilisation time, in others
there is no direct relationship between the delta in the voltage and
the stabilisation time. This change introduces the device tree properties
"regulator-settle-time-up-us"/"regulator-settle-time-down-us" which
allow to specify a fixed delay after a voltage increase or decrease.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v5:
- Added support for delay on voltage decreases
- Don't skip set_voltage_time op if no settle time and ramp delay

 .../devicetree/bindings/regulator/regulator.txt        |  4 ++++
 drivers/regulator/core.c                               | 18 +++++++++++-------
 drivers/regulator/of_regulator.c                       |  8 ++++++++
 include/linux/regulator/machine.h                      |  4 ++++
 4 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index ecfc593..4f792d1 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -21,6 +21,10 @@ Optional properties:
   design requires. This property describes the total system ramp time
   required due to the combination of internal ramping of the regulator itself,
   and board design issues such as trace capacitance and load on the supply.
+- regulator-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.
+- regulator-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.
 - regulator-soft-start: Enable soft start so that voltage ramps slowly
 - regulator-state-mem sub-root node for Suspend-to-RAM mode
   : suspend to memory, the device goes to sleep, but all data stored in memory,
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index c52fc0c..dbb238f 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2746,19 +2746,23 @@ static int _regulator_call_set_voltage_sel(struct regulator_dev *rdev,
 static int _regulator_set_voltage_time(struct regulator_dev *rdev,
 				       int old_uV, int new_uV)
 {
+	unsigned int settle_time = 0;
 	unsigned int ramp_delay = 0;
 
+	if (new_uV > old_uV)
+		settle_time = rdev->constraints->settle_time_up;
+	else if (new_uV < old_uV)
+		settle_time = rdev->constraints->settle_time_down;
+
 	if (rdev->constraints->ramp_delay)
 		ramp_delay = rdev->constraints->ramp_delay;
 	else if (rdev->desc->ramp_delay)
 		ramp_delay = rdev->desc->ramp_delay;
 
-	if (ramp_delay == 0) {
-		rdev_warn(rdev, "ramp_delay not set\n");
-		return 0;
-	}
+	if (ramp_delay == 0)
+		return settle_time;
 
-	return DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
+	return settle_time + DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
 }
 
 static int _regulator_do_set_voltage(struct regulator_dev *rdev,
@@ -3071,8 +3075,8 @@ EXPORT_SYMBOL_GPL(regulator_set_voltage_time);
  * Provided with the starting and target voltage selectors, this function
  * returns time in microseconds required to rise or fall to this new voltage
  *
- * Drivers providing ramp_delay in regulation_constraints can use this as their
- * set_voltage_time_sel() operation.
+ * Drivers providing ramp_delay or settle_time in regulation_constraints can
+ * use this as their set_voltage_time_sel() operation.
  */
 int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 				   unsigned int old_selector,
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 4f613ec..d3b20ae 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -90,6 +90,14 @@ static void of_get_regulation_constraints(struct device_node *np,
 	if (!ret)
 		constraints->enable_time = pval;
 
+	ret = of_property_read_u32(np, "regulator-settle-time-up-us", &pval);
+	if (!ret)
+		constraints->settle_time_up = pval;
+
+	ret = of_property_read_u32(np, "regulator-settle-time-down-us", &pval);
+	if (!ret)
+		constraints->settle_time_down = pval;
+
 	constraints->soft_start = of_property_read_bool(np,
 					"regulator-soft-start");
 	ret = of_property_read_u32(np, "regulator-active-discharge", &pval);
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index ad3e515..11ac36c 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -108,6 +108,8 @@ struct regulator_state {
  * @initial_state: Suspend state to set by default.
  * @initial_mode: Mode to set at startup.
  * @ramp_delay: Time to settle down after voltage change (unit: uV/us)
+ * @settle_time_up: Time to settle down after voltage increase (unit: uV/us)
+ * @settle_time_down: Time to settle down after voltage decrease (unit: uV/us)
  * @active_discharge: Enable/disable active discharge. The enum
  *		      regulator_active_discharge values are used for
  *		      initialisation.
@@ -150,6 +152,8 @@ struct regulation_constraints {
 
 	unsigned int ramp_delay;
 	unsigned int enable_time;
+	unsigned int settle_time_up;
+	unsigned int settle_time_down;
 
 	unsigned int active_discharge;
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 5/6] regulator: core: Add support for a fixed delay after voltage changes
@ 2016-09-14 16:52   ` Matthias Kaehlcke
  0 siblings, 0 replies; 20+ messages in thread
From: Matthias Kaehlcke @ 2016-09-14 16:52 UTC (permalink / raw)
  To: Mark Brown, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w
  Cc: Douglas Anderson, briannorris-F7+t8E8rja9g9hUCZPvPmw,
	javier-0uQlZySMnqxg9hUCZPvPmw, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Matthias Kaehlcke

The target voltage is not necessarily reached inmediately after
requesting a regulator to change the voltage. In some cases the
ramp_delay can be used to calculate the stabilisation time, in others
there is no direct relationship between the delta in the voltage and
the stabilisation time. This change introduces the device tree properties
"regulator-settle-time-up-us"/"regulator-settle-time-down-us" which
allow to specify a fixed delay after a voltage increase or decrease.

Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v5:
- Added support for delay on voltage decreases
- Don't skip set_voltage_time op if no settle time and ramp delay

 .../devicetree/bindings/regulator/regulator.txt        |  4 ++++
 drivers/regulator/core.c                               | 18 +++++++++++-------
 drivers/regulator/of_regulator.c                       |  8 ++++++++
 include/linux/regulator/machine.h                      |  4 ++++
 4 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index ecfc593..4f792d1 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -21,6 +21,10 @@ Optional properties:
   design requires. This property describes the total system ramp time
   required due to the combination of internal ramping of the regulator itself,
   and board design issues such as trace capacitance and load on the supply.
+- regulator-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.
+- regulator-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.
 - regulator-soft-start: Enable soft start so that voltage ramps slowly
 - regulator-state-mem sub-root node for Suspend-to-RAM mode
   : suspend to memory, the device goes to sleep, but all data stored in memory,
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index c52fc0c..dbb238f 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2746,19 +2746,23 @@ static int _regulator_call_set_voltage_sel(struct regulator_dev *rdev,
 static int _regulator_set_voltage_time(struct regulator_dev *rdev,
 				       int old_uV, int new_uV)
 {
+	unsigned int settle_time = 0;
 	unsigned int ramp_delay = 0;
 
+	if (new_uV > old_uV)
+		settle_time = rdev->constraints->settle_time_up;
+	else if (new_uV < old_uV)
+		settle_time = rdev->constraints->settle_time_down;
+
 	if (rdev->constraints->ramp_delay)
 		ramp_delay = rdev->constraints->ramp_delay;
 	else if (rdev->desc->ramp_delay)
 		ramp_delay = rdev->desc->ramp_delay;
 
-	if (ramp_delay == 0) {
-		rdev_warn(rdev, "ramp_delay not set\n");
-		return 0;
-	}
+	if (ramp_delay == 0)
+		return settle_time;
 
-	return DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
+	return settle_time + DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
 }
 
 static int _regulator_do_set_voltage(struct regulator_dev *rdev,
@@ -3071,8 +3075,8 @@ EXPORT_SYMBOL_GPL(regulator_set_voltage_time);
  * Provided with the starting and target voltage selectors, this function
  * returns time in microseconds required to rise or fall to this new voltage
  *
- * Drivers providing ramp_delay in regulation_constraints can use this as their
- * set_voltage_time_sel() operation.
+ * Drivers providing ramp_delay or settle_time in regulation_constraints can
+ * use this as their set_voltage_time_sel() operation.
  */
 int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 				   unsigned int old_selector,
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 4f613ec..d3b20ae 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -90,6 +90,14 @@ static void of_get_regulation_constraints(struct device_node *np,
 	if (!ret)
 		constraints->enable_time = pval;
 
+	ret = of_property_read_u32(np, "regulator-settle-time-up-us", &pval);
+	if (!ret)
+		constraints->settle_time_up = pval;
+
+	ret = of_property_read_u32(np, "regulator-settle-time-down-us", &pval);
+	if (!ret)
+		constraints->settle_time_down = pval;
+
 	constraints->soft_start = of_property_read_bool(np,
 					"regulator-soft-start");
 	ret = of_property_read_u32(np, "regulator-active-discharge", &pval);
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index ad3e515..11ac36c 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -108,6 +108,8 @@ struct regulator_state {
  * @initial_state: Suspend state to set by default.
  * @initial_mode: Mode to set at startup.
  * @ramp_delay: Time to settle down after voltage change (unit: uV/us)
+ * @settle_time_up: Time to settle down after voltage increase (unit: uV/us)
+ * @settle_time_down: Time to settle down after voltage decrease (unit: uV/us)
  * @active_discharge: Enable/disable active discharge. The enum
  *		      regulator_active_discharge values are used for
  *		      initialisation.
@@ -150,6 +152,8 @@ struct regulation_constraints {
 
 	unsigned int ramp_delay;
 	unsigned int enable_time;
+	unsigned int settle_time_up;
+	unsigned int settle_time_down;
 
 	unsigned int active_discharge;
 
-- 
2.8.0.rc3.226.g39d4020

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 6/6] regulator: core: Prevent falling too fast
@ 2016-09-14 16:52   ` Matthias Kaehlcke
  0 siblings, 0 replies; 20+ messages in thread
From: Matthias Kaehlcke @ 2016-09-14 16:52 UTC (permalink / raw)
  To: Mark Brown, lgirdwood
  Cc: Douglas Anderson, briannorris, javier, robh+dt, mark.rutland,
	linux-kernel, devicetree, Matthias Kaehlcke

From: Douglas Anderson <dianders@chromium.org>

On some boards it is possible that transitioning the regulator downwards
too fast will trigger the over voltage protection (OVP) on the
regulator. This is because until the voltage actually falls there is
time when the requested voltage is much lower than the actual voltage.

We'll fix this OVP problem by allowing users to specify the maximum
voltage that we can safely fall. The maximum safe voltage decrease
is specified as a percentage of the current voltage. The driver will
then break things into separate steps with a delay in between.

In order to figure out what the delay should be we need to figure out
how slowly the voltage rail might fall in the worst (slowest) case.
We'll assume this worst case is present and delay so we know for sure
that we've finished each step.

In this patch we actually block returning from the set_voltage() call
until we've finished delaying. A future patch atop this one might
choose to return more immediately and let the voltages fall in the
background. That would possibly allow us to cancel a slow downward
decay if there was a request to go back up.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
Changes in v5:
- Leave set_voltage tracepoints where they were
- Fixed error handling in code dealing with the device tree, return an error if configuration is invalid
- Fixed coding style and formatting issues
- Updated commit message

 .../devicetree/bindings/regulator/regulator.txt    |  7 ++++
 drivers/regulator/core.c                           | 49 +++++++++++++++++++---
 drivers/regulator/of_regulator.c                   | 42 ++++++++++++++++++-
 include/linux/regulator/machine.h                  |  2 +
 4 files changed, 93 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 4f792d1..485f14c 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -25,6 +25,13 @@ Optional properties:
   (unit: us). For regulators with a ramp delay the two values are added.
 - regulator-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.
+- regulator-safe-fall-percent:  If specified, it's not safe to transition the
+  regulator down faster than this amount and bigger jumps need to be broken into
+  more than one step.
+- regulator-slowest-decay-rate: Describes how slowly the regulator voltage will
+  decay down in the worst case (lightest expected load). Specified in uV / us
+  (like main regulator ramp rate). This is required when safe-fall-percent is
+  specified.
 - regulator-soft-start: Enable soft start so that voltage ramps slowly
 - regulator-state-mem sub-root node for Suspend-to-RAM mode
   : suspend to memory, the device goes to sleep, but all data stored in memory,
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index dbb238f..36abfdf 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -105,8 +105,8 @@ static int _regulator_get_current_limit(struct regulator_dev *rdev);
 static unsigned int _regulator_get_mode(struct regulator_dev *rdev);
 static int _notifier_call_chain(struct regulator_dev *rdev,
 				  unsigned long event, void *data);
-static int _regulator_do_set_voltage(struct regulator_dev *rdev,
-				     int min_uV, int max_uV);
+static int _regulator_set_voltage(struct regulator_dev *rdev,
+				  int min_uV, int max_uV);
 static struct regulator *create_regulator(struct regulator_dev *rdev,
 					  struct device *dev,
 					  const char *supply_name);
@@ -910,7 +910,7 @@ static int machine_constraints_voltage(struct regulator_dev *rdev,
 		if (target_min != current_uV || target_max != current_uV) {
 			rdev_info(rdev, "Bringing %duV into %d-%duV\n",
 				  current_uV, target_min, target_max);
-			ret = _regulator_do_set_voltage(
+			ret = _regulator_set_voltage(
 				rdev, target_min, target_max);
 			if (ret < 0) {
 				rdev_err(rdev,
@@ -2872,6 +2872,45 @@ out:
 	return ret;
 }
 
+static int _regulator_set_voltage(struct regulator_dev *rdev,
+				  int min_uV, int max_uV)
+{
+	int safe_fall_percent = rdev->constraints->safe_fall_percent;
+	int slowest_decay_rate = rdev->constraints->slowest_decay_rate;
+	int orig_uV = _regulator_get_voltage(rdev);
+	int uV = orig_uV;
+	int ret;
+
+	/* If we're rising or we're falling but don't need to slow; easy */
+	if (min_uV >= uV || !safe_fall_percent)
+		return _regulator_do_set_voltage(rdev, min_uV, max_uV);
+
+	while (uV > min_uV) {
+		int max_drop_uV = (uV * safe_fall_percent) / 100;
+		int next_uV;
+		int delay;
+
+		/* Make sure no infinite loop even in crazy cases */
+		if (max_drop_uV == 0)
+			max_drop_uV = 1;
+
+		next_uV = max_t(int, min_uV, uV - max_drop_uV);
+		delay = DIV_ROUND_UP(uV - next_uV, slowest_decay_rate);
+
+		ret = _regulator_do_set_voltage(rdev, uV, next_uV);
+		if (ret) {
+			/* Try to go back to original */
+			_regulator_do_set_voltage(rdev, uV, orig_uV);
+			return ret;
+		}
+
+		usleep_range(delay, delay + DIV_ROUND_UP(delay, 10));
+		uV = next_uV;
+	}
+
+	return 0;
+}
+
 static int regulator_set_voltage_unlocked(struct regulator *regulator,
 					  int min_uV, int max_uV)
 {
@@ -2962,7 +3001,7 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 		}
 	}
 
-	ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
+	ret = _regulator_set_voltage(rdev, min_uV, max_uV);
 	if (ret < 0)
 		goto out2;
 
@@ -3138,7 +3177,7 @@ int regulator_sync_voltage(struct regulator *regulator)
 	if (ret < 0)
 		goto out;
 
-	ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
+	ret = _regulator_set_voltage(rdev, min_uV, max_uV);
 
 out:
 	mutex_unlock(&rdev->mutex);
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index d3b20ae..d7b74b2 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -24,7 +24,7 @@ static const char *const regulator_states[PM_SUSPEND_MAX + 1] = {
 	[PM_SUSPEND_MAX]	= "regulator-state-disk",
 };
 
-static void of_get_regulation_constraints(struct device_node *np,
+static int of_get_regulation_constraints(struct device_node *np,
 					struct regulator_init_data **init_data,
 					const struct regulator_desc *desc)
 {
@@ -98,6 +98,40 @@ static void of_get_regulation_constraints(struct device_node *np,
 	if (!ret)
 		constraints->settle_time_down = pval;
 
+	ret = of_property_read_u32(np, "regulator-safe-fall-percent", &pval);
+	if (!ret) {
+		constraints->safe_fall_percent = pval;
+
+		if (constraints->safe_fall_percent > 100) {
+			pr_err("%s: regulator-safe-fall-percent (%u) > 100\n",
+			       np->name, constraints->safe_fall_percent);
+			return -EINVAL;
+		}
+	}
+
+	ret = of_property_read_u32(np, "regulator-slowest-decay-rate", &pval);
+	if (!ret) {
+		constraints->slowest_decay_rate = pval;
+
+		/* We use the value as int and as divider; sanity check */
+		if (constraints->slowest_decay_rate == 0) {
+			pr_err("%s: regulator-slowest-decay-rate must not be 0\n",
+			       np->name);
+			return -EINVAL;
+		} else if (constraints->slowest_decay_rate > INT_MAX) {
+			pr_err("%s: regulator-slowest-decay-rate (%u) too big\n",
+			       np->name, constraints->slowest_decay_rate);
+			return -EINVAL;
+		}
+	}
+
+	if (constraints->safe_fall_percent &&
+	    !constraints->slowest_decay_rate) {
+		pr_err("%s: regulator-safe-fall-percent requires regulator-slowest-decay-rate\n",
+		       np->name);
+		return -EINVAL;
+	}
+
 	constraints->soft_start = of_property_read_bool(np,
 					"regulator-soft-start");
 	ret = of_property_read_u32(np, "regulator-active-discharge", &pval);
@@ -178,6 +212,8 @@ static void of_get_regulation_constraints(struct device_node *np,
 		suspend_state = NULL;
 		suspend_np = NULL;
 	}
+
+	return 0;
 }
 
 /**
@@ -203,7 +239,9 @@ struct regulator_init_data *of_get_regulator_init_data(struct device *dev,
 	if (!init_data)
 		return NULL; /* Out of memory? */
 
-	of_get_regulation_constraints(node, &init_data, desc);
+	if (of_get_regulation_constraints(node, &init_data, desc))
+		return NULL;
+
 	return init_data;
 }
 EXPORT_SYMBOL_GPL(of_get_regulator_init_data);
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index 11ac36c..2d797dd 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -154,6 +154,8 @@ struct regulation_constraints {
 	unsigned int enable_time;
 	unsigned int settle_time_up;
 	unsigned int settle_time_down;
+	unsigned int slowest_decay_rate;
+	unsigned int safe_fall_percent;
 
 	unsigned int active_discharge;
 
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 6/6] regulator: core: Prevent falling too fast
@ 2016-09-14 16:52   ` Matthias Kaehlcke
  0 siblings, 0 replies; 20+ messages in thread
From: Matthias Kaehlcke @ 2016-09-14 16:52 UTC (permalink / raw)
  To: Mark Brown, lgirdwood-Re5JQEeQqe8AvxtiuMwx3w
  Cc: Douglas Anderson, briannorris-F7+t8E8rja9g9hUCZPvPmw,
	javier-0uQlZySMnqxg9hUCZPvPmw, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Matthias Kaehlcke

From: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

On some boards it is possible that transitioning the regulator downwards
too fast will trigger the over voltage protection (OVP) on the
regulator. This is because until the voltage actually falls there is
time when the requested voltage is much lower than the actual voltage.

We'll fix this OVP problem by allowing users to specify the maximum
voltage that we can safely fall. The maximum safe voltage decrease
is specified as a percentage of the current voltage. The driver will
then break things into separate steps with a delay in between.

In order to figure out what the delay should be we need to figure out
how slowly the voltage rail might fall in the worst (slowest) case.
We'll assume this worst case is present and delay so we know for sure
that we've finished each step.

In this patch we actually block returning from the set_voltage() call
until we've finished delaying. A future patch atop this one might
choose to return more immediately and let the voltages fall in the
background. That would possibly allow us to cancel a slow downward
decay if there was a request to go back up.

Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
Changes in v5:
- Leave set_voltage tracepoints where they were
- Fixed error handling in code dealing with the device tree, return an error if configuration is invalid
- Fixed coding style and formatting issues
- Updated commit message

 .../devicetree/bindings/regulator/regulator.txt    |  7 ++++
 drivers/regulator/core.c                           | 49 +++++++++++++++++++---
 drivers/regulator/of_regulator.c                   | 42 ++++++++++++++++++-
 include/linux/regulator/machine.h                  |  2 +
 4 files changed, 93 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt
index 4f792d1..485f14c 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/regulator.txt
@@ -25,6 +25,13 @@ Optional properties:
   (unit: us). For regulators with a ramp delay the two values are added.
 - regulator-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.
+- regulator-safe-fall-percent:  If specified, it's not safe to transition the
+  regulator down faster than this amount and bigger jumps need to be broken into
+  more than one step.
+- regulator-slowest-decay-rate: Describes how slowly the regulator voltage will
+  decay down in the worst case (lightest expected load). Specified in uV / us
+  (like main regulator ramp rate). This is required when safe-fall-percent is
+  specified.
 - regulator-soft-start: Enable soft start so that voltage ramps slowly
 - regulator-state-mem sub-root node for Suspend-to-RAM mode
   : suspend to memory, the device goes to sleep, but all data stored in memory,
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index dbb238f..36abfdf 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -105,8 +105,8 @@ static int _regulator_get_current_limit(struct regulator_dev *rdev);
 static unsigned int _regulator_get_mode(struct regulator_dev *rdev);
 static int _notifier_call_chain(struct regulator_dev *rdev,
 				  unsigned long event, void *data);
-static int _regulator_do_set_voltage(struct regulator_dev *rdev,
-				     int min_uV, int max_uV);
+static int _regulator_set_voltage(struct regulator_dev *rdev,
+				  int min_uV, int max_uV);
 static struct regulator *create_regulator(struct regulator_dev *rdev,
 					  struct device *dev,
 					  const char *supply_name);
@@ -910,7 +910,7 @@ static int machine_constraints_voltage(struct regulator_dev *rdev,
 		if (target_min != current_uV || target_max != current_uV) {
 			rdev_info(rdev, "Bringing %duV into %d-%duV\n",
 				  current_uV, target_min, target_max);
-			ret = _regulator_do_set_voltage(
+			ret = _regulator_set_voltage(
 				rdev, target_min, target_max);
 			if (ret < 0) {
 				rdev_err(rdev,
@@ -2872,6 +2872,45 @@ out:
 	return ret;
 }
 
+static int _regulator_set_voltage(struct regulator_dev *rdev,
+				  int min_uV, int max_uV)
+{
+	int safe_fall_percent = rdev->constraints->safe_fall_percent;
+	int slowest_decay_rate = rdev->constraints->slowest_decay_rate;
+	int orig_uV = _regulator_get_voltage(rdev);
+	int uV = orig_uV;
+	int ret;
+
+	/* If we're rising or we're falling but don't need to slow; easy */
+	if (min_uV >= uV || !safe_fall_percent)
+		return _regulator_do_set_voltage(rdev, min_uV, max_uV);
+
+	while (uV > min_uV) {
+		int max_drop_uV = (uV * safe_fall_percent) / 100;
+		int next_uV;
+		int delay;
+
+		/* Make sure no infinite loop even in crazy cases */
+		if (max_drop_uV == 0)
+			max_drop_uV = 1;
+
+		next_uV = max_t(int, min_uV, uV - max_drop_uV);
+		delay = DIV_ROUND_UP(uV - next_uV, slowest_decay_rate);
+
+		ret = _regulator_do_set_voltage(rdev, uV, next_uV);
+		if (ret) {
+			/* Try to go back to original */
+			_regulator_do_set_voltage(rdev, uV, orig_uV);
+			return ret;
+		}
+
+		usleep_range(delay, delay + DIV_ROUND_UP(delay, 10));
+		uV = next_uV;
+	}
+
+	return 0;
+}
+
 static int regulator_set_voltage_unlocked(struct regulator *regulator,
 					  int min_uV, int max_uV)
 {
@@ -2962,7 +3001,7 @@ static int regulator_set_voltage_unlocked(struct regulator *regulator,
 		}
 	}
 
-	ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
+	ret = _regulator_set_voltage(rdev, min_uV, max_uV);
 	if (ret < 0)
 		goto out2;
 
@@ -3138,7 +3177,7 @@ int regulator_sync_voltage(struct regulator *regulator)
 	if (ret < 0)
 		goto out;
 
-	ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
+	ret = _regulator_set_voltage(rdev, min_uV, max_uV);
 
 out:
 	mutex_unlock(&rdev->mutex);
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index d3b20ae..d7b74b2 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -24,7 +24,7 @@ static const char *const regulator_states[PM_SUSPEND_MAX + 1] = {
 	[PM_SUSPEND_MAX]	= "regulator-state-disk",
 };
 
-static void of_get_regulation_constraints(struct device_node *np,
+static int of_get_regulation_constraints(struct device_node *np,
 					struct regulator_init_data **init_data,
 					const struct regulator_desc *desc)
 {
@@ -98,6 +98,40 @@ static void of_get_regulation_constraints(struct device_node *np,
 	if (!ret)
 		constraints->settle_time_down = pval;
 
+	ret = of_property_read_u32(np, "regulator-safe-fall-percent", &pval);
+	if (!ret) {
+		constraints->safe_fall_percent = pval;
+
+		if (constraints->safe_fall_percent > 100) {
+			pr_err("%s: regulator-safe-fall-percent (%u) > 100\n",
+			       np->name, constraints->safe_fall_percent);
+			return -EINVAL;
+		}
+	}
+
+	ret = of_property_read_u32(np, "regulator-slowest-decay-rate", &pval);
+	if (!ret) {
+		constraints->slowest_decay_rate = pval;
+
+		/* We use the value as int and as divider; sanity check */
+		if (constraints->slowest_decay_rate == 0) {
+			pr_err("%s: regulator-slowest-decay-rate must not be 0\n",
+			       np->name);
+			return -EINVAL;
+		} else if (constraints->slowest_decay_rate > INT_MAX) {
+			pr_err("%s: regulator-slowest-decay-rate (%u) too big\n",
+			       np->name, constraints->slowest_decay_rate);
+			return -EINVAL;
+		}
+	}
+
+	if (constraints->safe_fall_percent &&
+	    !constraints->slowest_decay_rate) {
+		pr_err("%s: regulator-safe-fall-percent requires regulator-slowest-decay-rate\n",
+		       np->name);
+		return -EINVAL;
+	}
+
 	constraints->soft_start = of_property_read_bool(np,
 					"regulator-soft-start");
 	ret = of_property_read_u32(np, "regulator-active-discharge", &pval);
@@ -178,6 +212,8 @@ static void of_get_regulation_constraints(struct device_node *np,
 		suspend_state = NULL;
 		suspend_np = NULL;
 	}
+
+	return 0;
 }
 
 /**
@@ -203,7 +239,9 @@ struct regulator_init_data *of_get_regulator_init_data(struct device *dev,
 	if (!init_data)
 		return NULL; /* Out of memory? */
 
-	of_get_regulation_constraints(node, &init_data, desc);
+	if (of_get_regulation_constraints(node, &init_data, desc))
+		return NULL;
+
 	return init_data;
 }
 EXPORT_SYMBOL_GPL(of_get_regulator_init_data);
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index 11ac36c..2d797dd 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -154,6 +154,8 @@ struct regulation_constraints {
 	unsigned int enable_time;
 	unsigned int settle_time_up;
 	unsigned int settle_time_down;
+	unsigned int slowest_decay_rate;
+	unsigned int safe_fall_percent;
 
 	unsigned int active_discharge;
 
-- 
2.8.0.rc3.226.g39d4020

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Applied "regulator: core: Use local ops variable in _regulator_do_set_voltage()" to the regulator tree
@ 2016-09-14 17:16   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2016-09-14 17:16 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Mark Brown, Mark Brown, lgirdwood, Douglas Anderson, briannorris,
	javier, robh+dt, mark.rutland, linux-kernel, devicetree

The patch

   regulator: core: Use local ops variable in _regulator_do_set_voltage()

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 57995a4860542ea2089558c84f739e12d5ca1059 Mon Sep 17 00:00:00 2001
From: Matthias Kaehlcke <mka@chromium.org>
Date: Wed, 14 Sep 2016 09:52:05 -0700
Subject: [PATCH] regulator: core: Use local ops variable in
 _regulator_do_set_voltage()

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index db320e8fa865..b059e8334567 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2751,6 +2751,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	int best_val = 0;
 	unsigned int selector;
 	int old_selector = -1;
+	const struct regulator_ops *ops = rdev->desc->ops;
 
 	trace_regulator_set_voltage(rdev_get_name(rdev), min_uV, max_uV);
 
@@ -2762,29 +2763,28 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	 * info to call set_voltage_time_sel().
 	 */
 	if (_regulator_is_enabled(rdev) &&
-	    rdev->desc->ops->set_voltage_time_sel &&
-	    rdev->desc->ops->get_voltage_sel) {
-		old_selector = rdev->desc->ops->get_voltage_sel(rdev);
+	    ops->set_voltage_time_sel && ops->get_voltage_sel) {
+		old_selector = ops->get_voltage_sel(rdev);
 		if (old_selector < 0)
 			return old_selector;
 	}
 
-	if (rdev->desc->ops->set_voltage) {
+	if (ops->set_voltage) {
 		ret = _regulator_call_set_voltage(rdev, min_uV, max_uV,
 						  &selector);
 
 		if (ret >= 0) {
-			if (rdev->desc->ops->list_voltage)
-				best_val = rdev->desc->ops->list_voltage(rdev,
-									 selector);
+			if (ops->list_voltage)
+				best_val = ops->list_voltage(rdev,
+							     selector);
 			else
 				best_val = _regulator_get_voltage(rdev);
 		}
 
-	} else if (rdev->desc->ops->set_voltage_sel) {
+	} else if (ops->set_voltage_sel) {
 		ret = regulator_map_voltage(rdev, min_uV, max_uV);
 		if (ret >= 0) {
-			best_val = rdev->desc->ops->list_voltage(rdev, ret);
+			best_val = ops->list_voltage(rdev, ret);
 			if (min_uV <= best_val && max_uV >= best_val) {
 				selector = ret;
 				if (old_selector == selector)
@@ -2804,7 +2804,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	if (ret == 0 && !rdev->constraints->ramp_disable && old_selector >= 0
 		&& old_selector != selector) {
 
-		delay = rdev->desc->ops->set_voltage_time_sel(rdev,
+		delay = ops->set_voltage_time_sel(rdev,
 						old_selector, selector);
 		if (delay < 0) {
 			rdev_warn(rdev, "set_voltage_time_sel() failed: %d\n",
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Applied "regulator: core: Use local ops variable in _regulator_do_set_voltage()" to the regulator tree
@ 2016-09-14 17:16   ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2016-09-14 17:16 UTC (permalink / raw)
  To: Matthias Kaehlcke; +Cc: Mark Brown

The patch

   regulator: core: Use local ops variable in _regulator_do_set_voltage()

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 57995a4860542ea2089558c84f739e12d5ca1059 Mon Sep 17 00:00:00 2001
From: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Date: Wed, 14 Sep 2016 09:52:05 -0700
Subject: [PATCH] regulator: core: Use local ops variable in
 _regulator_do_set_voltage()

Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/regulator/core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index db320e8fa865..b059e8334567 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2751,6 +2751,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	int best_val = 0;
 	unsigned int selector;
 	int old_selector = -1;
+	const struct regulator_ops *ops = rdev->desc->ops;
 
 	trace_regulator_set_voltage(rdev_get_name(rdev), min_uV, max_uV);
 
@@ -2762,29 +2763,28 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	 * info to call set_voltage_time_sel().
 	 */
 	if (_regulator_is_enabled(rdev) &&
-	    rdev->desc->ops->set_voltage_time_sel &&
-	    rdev->desc->ops->get_voltage_sel) {
-		old_selector = rdev->desc->ops->get_voltage_sel(rdev);
+	    ops->set_voltage_time_sel && ops->get_voltage_sel) {
+		old_selector = ops->get_voltage_sel(rdev);
 		if (old_selector < 0)
 			return old_selector;
 	}
 
-	if (rdev->desc->ops->set_voltage) {
+	if (ops->set_voltage) {
 		ret = _regulator_call_set_voltage(rdev, min_uV, max_uV,
 						  &selector);
 
 		if (ret >= 0) {
-			if (rdev->desc->ops->list_voltage)
-				best_val = rdev->desc->ops->list_voltage(rdev,
-									 selector);
+			if (ops->list_voltage)
+				best_val = ops->list_voltage(rdev,
+							     selector);
 			else
 				best_val = _regulator_get_voltage(rdev);
 		}
 
-	} else if (rdev->desc->ops->set_voltage_sel) {
+	} else if (ops->set_voltage_sel) {
 		ret = regulator_map_voltage(rdev, min_uV, max_uV);
 		if (ret >= 0) {
-			best_val = rdev->desc->ops->list_voltage(rdev, ret);
+			best_val = ops->list_voltage(rdev, ret);
 			if (min_uV <= best_val && max_uV >= best_val) {
 				selector = ret;
 				if (old_selector == selector)
@@ -2804,7 +2804,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	if (ret == 0 && !rdev->constraints->ramp_disable && old_selector >= 0
 		&& old_selector != selector) {
 
-		delay = rdev->desc->ops->set_voltage_time_sel(rdev,
+		delay = ops->set_voltage_time_sel(rdev,
 						old_selector, selector);
 		if (delay < 0) {
 			rdev_warn(rdev, "set_voltage_time_sel() failed: %d\n",
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 6/6] regulator: core: Prevent falling too fast
@ 2016-09-14 21:35     ` kbuild test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2016-09-14 21:35 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: kbuild-all, Mark Brown, lgirdwood, Douglas Anderson, briannorris,
	javier, robh+dt, mark.rutland, linux-kernel, devicetree,
	Matthias Kaehlcke

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

Hi Douglas,

[auto build test WARNING on regulator/for-next]
[also build test WARNING on v4.8-rc6 next-20160914]
[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/regulator-core-Use-local-ops-variable-in-_regulator_do_set_voltage/20160915-005730
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

>> include/linux/regulator/machine.h:170: warning: No description found for parameter 'slowest_decay_rate'
>> include/linux/regulator/machine.h:170: warning: No description found for parameter 'safe_fall_percent'

vim +/slowest_decay_rate +170 include/linux/regulator/machine.h

00c877c69 Laxman Dewangan      2013-09-18  154  	unsigned int enable_time;
31f8038f4 Matthias Kaehlcke    2016-09-14  155  	unsigned int settle_time_up;
31f8038f4 Matthias Kaehlcke    2016-09-14  156  	unsigned int settle_time_down;
171385d28 Douglas Anderson     2016-09-14  157  	unsigned int slowest_decay_rate;
171385d28 Douglas Anderson     2016-09-14  158  	unsigned int safe_fall_percent;
6f0b2c696 Yadwinder Singh Brar 2012-06-11  159  
670666b9e Laxman Dewangan      2016-03-02  160  	unsigned int active_discharge;
670666b9e Laxman Dewangan      2016-03-02  161  
2e7e65ce5 Wolfram Sang         2009-09-18  162  	/* constraint flags */
4c1184e85 Liam Girdwood        2008-04-30  163  	unsigned always_on:1;	/* regulator never off when system is on */
4c1184e85 Liam Girdwood        2008-04-30  164  	unsigned boot_on:1;	/* bootloader/firmware enabled regulator */
2e7e65ce5 Wolfram Sang         2009-09-18  165  	unsigned apply_uV:1;	/* apply uV constraint if min == max */
1653ccf4c Yadwinder Singh Brar 2013-06-29  166  	unsigned ramp_disable:1; /* disable ramp delay */
57f66b788 Stephen Boyd         2015-06-11  167  	unsigned soft_start:1;	/* ramp voltage slowly */
23c779b9f Stephen Boyd         2015-06-11  168  	unsigned pull_down:1;	/* pull down resistor when regulator off */
3a003baee Stephen Boyd         2015-07-17  169  	unsigned over_current_protection:1; /* auto disable on over current */
4c1184e85 Liam Girdwood        2008-04-30 @170  };
4c1184e85 Liam Girdwood        2008-04-30  171  
a5766f11c Liam Girdwood        2008-10-10  172  /**
a5766f11c Liam Girdwood        2008-10-10  173   * struct regulator_consumer_supply - supply -> device mapping
a5766f11c Liam Girdwood        2008-10-10  174   *
15c08f664 Axel Lin             2012-03-29  175   * This maps a supply name to a device. Use of dev_name allows support for
15c08f664 Axel Lin             2012-03-29  176   * buses which make struct device available late such as I2C.
c8e7e4640 Mark Brown           2008-12-31  177   *
40f9244f4 Mark Brown           2009-06-17  178   * @dev_name: Result of dev_name() for the consumer.

:::::: The code at line 170 was first introduced by commit
:::::: 4c1184e85cb381121a5273ea20ad31ca3faa0a4f regulator: machine driver interface

:::::: TO: Liam Girdwood <lg@opensource.wolfsonmicro.com>
:::::: CC: Liam Girdwood <lg@opensource.wolfsonmicro.com>

---
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: 6406 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 6/6] regulator: core: Prevent falling too fast
@ 2016-09-14 21:35     ` kbuild test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kbuild test robot @ 2016-09-14 21:35 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, Mark Brown,
	lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, Douglas Anderson,
	briannorris-F7+t8E8rja9g9hUCZPvPmw,
	javier-0uQlZySMnqxg9hUCZPvPmw, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Matthias Kaehlcke

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

Hi Douglas,

[auto build test WARNING on regulator/for-next]
[also build test WARNING on v4.8-rc6 next-20160914]
[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/regulator-core-Use-local-ops-variable-in-_regulator_do_set_voltage/20160915-005730
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next
reproduce: make htmldocs

All warnings (new ones prefixed by >>):

>> include/linux/regulator/machine.h:170: warning: No description found for parameter 'slowest_decay_rate'
>> include/linux/regulator/machine.h:170: warning: No description found for parameter 'safe_fall_percent'

vim +/slowest_decay_rate +170 include/linux/regulator/machine.h

00c877c69 Laxman Dewangan      2013-09-18  154  	unsigned int enable_time;
31f8038f4 Matthias Kaehlcke    2016-09-14  155  	unsigned int settle_time_up;
31f8038f4 Matthias Kaehlcke    2016-09-14  156  	unsigned int settle_time_down;
171385d28 Douglas Anderson     2016-09-14  157  	unsigned int slowest_decay_rate;
171385d28 Douglas Anderson     2016-09-14  158  	unsigned int safe_fall_percent;
6f0b2c696 Yadwinder Singh Brar 2012-06-11  159  
670666b9e Laxman Dewangan      2016-03-02  160  	unsigned int active_discharge;
670666b9e Laxman Dewangan      2016-03-02  161  
2e7e65ce5 Wolfram Sang         2009-09-18  162  	/* constraint flags */
4c1184e85 Liam Girdwood        2008-04-30  163  	unsigned always_on:1;	/* regulator never off when system is on */
4c1184e85 Liam Girdwood        2008-04-30  164  	unsigned boot_on:1;	/* bootloader/firmware enabled regulator */
2e7e65ce5 Wolfram Sang         2009-09-18  165  	unsigned apply_uV:1;	/* apply uV constraint if min == max */
1653ccf4c Yadwinder Singh Brar 2013-06-29  166  	unsigned ramp_disable:1; /* disable ramp delay */
57f66b788 Stephen Boyd         2015-06-11  167  	unsigned soft_start:1;	/* ramp voltage slowly */
23c779b9f Stephen Boyd         2015-06-11  168  	unsigned pull_down:1;	/* pull down resistor when regulator off */
3a003baee Stephen Boyd         2015-07-17  169  	unsigned over_current_protection:1; /* auto disable on over current */
4c1184e85 Liam Girdwood        2008-04-30 @170  };
4c1184e85 Liam Girdwood        2008-04-30  171  
a5766f11c Liam Girdwood        2008-10-10  172  /**
a5766f11c Liam Girdwood        2008-10-10  173   * struct regulator_consumer_supply - supply -> device mapping
a5766f11c Liam Girdwood        2008-10-10  174   *
15c08f664 Axel Lin             2012-03-29  175   * This maps a supply name to a device. Use of dev_name allows support for
15c08f664 Axel Lin             2012-03-29  176   * buses which make struct device available late such as I2C.
c8e7e4640 Mark Brown           2008-12-31  177   *
40f9244f4 Mark Brown           2009-06-17  178   * @dev_name: Result of dev_name() for the consumer.

:::::: The code at line 170 was first introduced by commit
:::::: 4c1184e85cb381121a5273ea20ad31ca3faa0a4f regulator: machine driver interface

:::::: TO: Liam Girdwood <lg-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
:::::: CC: Liam Girdwood <lg-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>

---
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: 6406 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Applied "regulator: core: Add set_voltage_time op" to the regulator tree
  2016-09-14 16:52 ` [PATCH v5 4/6] regulator: core: Add set_voltage_time op Matthias Kaehlcke
@ 2016-09-16 17:40     ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2016-09-16 17:40 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Mark Brown, Mark Brown, lgirdwood, Douglas Anderson, briannorris,
	javier, robh+dt, mark.rutland, linux-kernel, devicetree

The patch

   regulator: core: Add set_voltage_time op

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 73e705bf81ceb84b39ef9cf6ffb8d12ca0c58a23 Mon Sep 17 00:00:00 2001
From: Matthias Kaehlcke <mka@chromium.org>
Date: Wed, 14 Sep 2016 09:52:08 -0700
Subject: [PATCH] regulator: core: Add set_voltage_time op

The new op is analogous to set_voltage_time_sel. It can be used by
regulators which don't have a table of discrete voltages. The function
returns the time for the regulator output voltage to stabilize after
being set to a new value, in microseconds. If the op is not set a
default implementation is used to calculate the delay.

This change also removes the ramp_delay calculation in the PWM
regulator, since the driver now uses the core code for the calculation
of the delay.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c          | 86 +++++++++++++++++++++++++++------------
 drivers/regulator/pwm-regulator.c | 10 -----
 include/linux/regulator/driver.h  | 10 ++++-
 3 files changed, 67 insertions(+), 39 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index df3028b2a8e9..c52fc0ce2693 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2743,6 +2743,24 @@ static int _regulator_call_set_voltage_sel(struct regulator_dev *rdev,
 	return ret;
 }
 
+static int _regulator_set_voltage_time(struct regulator_dev *rdev,
+				       int old_uV, int new_uV)
+{
+	unsigned int ramp_delay = 0;
+
+	if (rdev->constraints->ramp_delay)
+		ramp_delay = rdev->constraints->ramp_delay;
+	else if (rdev->desc->ramp_delay)
+		ramp_delay = rdev->desc->ramp_delay;
+
+	if (ramp_delay == 0) {
+		rdev_warn(rdev, "ramp_delay not set\n");
+		return 0;
+	}
+
+	return DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
+}
+
 static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 				     int min_uV, int max_uV)
 {
@@ -2752,6 +2770,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	unsigned int selector;
 	int old_selector = -1;
 	const struct regulator_ops *ops = rdev->desc->ops;
+	int old_uV = _regulator_get_voltage(rdev);
 
 	trace_regulator_set_voltage(rdev_get_name(rdev), min_uV, max_uV);
 
@@ -2803,23 +2822,37 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	if (ret)
 		goto out;
 
-	/* Call set_voltage_time_sel if successfully obtained old_selector */
-	if (!old_selector >= 0 && old_selector != selector) {
-		delay = ops->set_voltage_time_sel(rdev,
-						old_selector, selector);
-		if (delay < 0) {
-			rdev_warn(rdev, "set_voltage_time_sel() failed: %d\n",
-				  delay);
-			delay = 0;
+	if (ops->set_voltage_time_sel) {
+		/*
+		 * Call set_voltage_time_sel if successfully obtained
+		 * old_selector
+		 */
+		if (old_selector >= 0 && old_selector != selector)
+			delay = ops->set_voltage_time_sel(rdev, old_selector,
+							  selector);
+	} else {
+		if (old_uV != best_val) {
+			if (ops->set_voltage_time)
+				delay = ops->set_voltage_time(rdev, old_uV,
+							      best_val);
+			else
+				delay = _regulator_set_voltage_time(rdev,
+								    old_uV,
+								    best_val);
 		}
+	}
 
-		/* Insert any necessary delays */
-		if (delay >= 1000) {
-			mdelay(delay / 1000);
-			udelay(delay % 1000);
-		} else if (delay) {
-			udelay(delay);
-		}
+	if (delay < 0) {
+		rdev_warn(rdev, "failed to get delay: %d\n", delay);
+		delay = 0;
+	}
+
+	/* Insert any necessary delays */
+	if (delay >= 1000) {
+		mdelay(delay / 1000);
+		udelay(delay % 1000);
+	} else if (delay) {
+		udelay(delay);
 	}
 
 	if (best_val >= 0) {
@@ -3000,9 +3033,13 @@ int regulator_set_voltage_time(struct regulator *regulator,
 	int voltage;
 	int i;
 
+	if (ops->set_voltage_time)
+		return ops->set_voltage_time(rdev, old_uV, new_uV);
+	else if (!ops->set_voltage_time_sel)
+		return _regulator_set_voltage_time(rdev, old_uV, new_uV);
+
 	/* Currently requires operations to do this */
-	if (!ops->list_voltage || !ops->set_voltage_time_sel
-	    || !rdev->desc->n_voltages)
+	if (!ops->list_voltage || !rdev->desc->n_voltages)
 		return -EINVAL;
 
 	for (i = 0; i < rdev->desc->n_voltages; i++) {
@@ -3041,17 +3078,8 @@ int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 				   unsigned int old_selector,
 				   unsigned int new_selector)
 {
-	unsigned int ramp_delay = 0;
 	int old_volt, new_volt;
 
-	if (rdev->constraints->ramp_delay)
-		ramp_delay = rdev->constraints->ramp_delay;
-	else if (rdev->desc->ramp_delay)
-		ramp_delay = rdev->desc->ramp_delay;
-
-	if (ramp_delay == 0)
-		return 0;
-
 	/* sanity check */
 	if (!rdev->desc->ops->list_voltage)
 		return -EINVAL;
@@ -3059,7 +3087,11 @@ int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 	old_volt = rdev->desc->ops->list_voltage(rdev, old_selector);
 	new_volt = rdev->desc->ops->list_voltage(rdev, new_selector);
 
-	return DIV_ROUND_UP(abs(new_volt - old_volt), ramp_delay);
+	if (rdev->desc->ops->set_voltage_time)
+		return rdev->desc->ops->set_voltage_time(rdev, old_volt,
+							 new_volt);
+	else
+		return _regulator_set_voltage_time(rdev, old_volt, new_volt);
 }
 EXPORT_SYMBOL_GPL(regulator_set_voltage_time_sel);
 
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index c24524242da2..1b88e0e15a70 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -10,7 +10,6 @@
  * published by the Free Software Foundation.
  */
 
-#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/err.h>
@@ -194,12 +193,10 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
 	unsigned int min_uV_duty = drvdata->continuous.min_uV_dutycycle;
 	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;
 	int min_uV = rdev->constraints->min_uV;
 	int max_uV = rdev->constraints->max_uV;
 	int diff_uV = max_uV - min_uV;
 	struct pwm_state pstate;
-	int old_uV = pwm_regulator_get_voltage(rdev);
 	unsigned int diff_duty;
 	unsigned int dutycycle;
 	int ret;
@@ -233,13 +230,6 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
 		return ret;
 	}
 
-	if ((ramp_delay == 0) || !pwm_regulator_is_enabled(rdev))
-		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;
 }
 
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index fcfa40a6692c..37b532410528 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -113,10 +113,14 @@ struct regulator_linear_range {
  *               stabilise after being enabled, in microseconds.
  * @set_ramp_delay: Set the ramp delay for the regulator. The driver should
  *		select ramp delay equal to or less than(closest) ramp_delay.
+ * @set_voltage_time: Time taken for the regulator voltage output voltage
+ *               to stabilise after being set to a new value, in microseconds.
+ *               The function receives the from and to voltage as input, it
+ *               should return the worst case.
  * @set_voltage_time_sel: Time taken for the regulator voltage output voltage
  *               to stabilise after being set to a new value, in microseconds.
- *               The function provides the from and to voltage selector, the
- *               function should return the worst case.
+ *               The function receives the from and to voltage selector as
+ *               input, it should return the worst case.
  * @set_soft_start: Enable soft start for the regulator.
  *
  * @set_suspend_voltage: Set the voltage for the regulator when the system
@@ -168,6 +172,8 @@ struct regulator_ops {
 	/* Time taken to enable or set voltage on the regulator */
 	int (*enable_time) (struct regulator_dev *);
 	int (*set_ramp_delay) (struct regulator_dev *, int ramp_delay);
+	int (*set_voltage_time) (struct regulator_dev *, int old_uV,
+				 int new_uV);
 	int (*set_voltage_time_sel) (struct regulator_dev *,
 				     unsigned int old_selector,
 				     unsigned int new_selector);
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Applied "regulator: core: Add set_voltage_time op" to the regulator tree
@ 2016-09-16 17:40     ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2016-09-16 17:40 UTC (permalink / raw)
  To: Matthias Kaehlcke; +Cc: Mark Brown

The patch

   regulator: core: Add set_voltage_time op

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 73e705bf81ceb84b39ef9cf6ffb8d12ca0c58a23 Mon Sep 17 00:00:00 2001
From: Matthias Kaehlcke <mka@chromium.org>
Date: Wed, 14 Sep 2016 09:52:08 -0700
Subject: [PATCH] regulator: core: Add set_voltage_time op

The new op is analogous to set_voltage_time_sel. It can be used by
regulators which don't have a table of discrete voltages. The function
returns the time for the regulator output voltage to stabilize after
being set to a new value, in microseconds. If the op is not set a
default implementation is used to calculate the delay.

This change also removes the ramp_delay calculation in the PWM
regulator, since the driver now uses the core code for the calculation
of the delay.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c          | 86 +++++++++++++++++++++++++++------------
 drivers/regulator/pwm-regulator.c | 10 -----
 include/linux/regulator/driver.h  | 10 ++++-
 3 files changed, 67 insertions(+), 39 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index df3028b2a8e9..c52fc0ce2693 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2743,6 +2743,24 @@ static int _regulator_call_set_voltage_sel(struct regulator_dev *rdev,
 	return ret;
 }
 
+static int _regulator_set_voltage_time(struct regulator_dev *rdev,
+				       int old_uV, int new_uV)
+{
+	unsigned int ramp_delay = 0;
+
+	if (rdev->constraints->ramp_delay)
+		ramp_delay = rdev->constraints->ramp_delay;
+	else if (rdev->desc->ramp_delay)
+		ramp_delay = rdev->desc->ramp_delay;
+
+	if (ramp_delay == 0) {
+		rdev_warn(rdev, "ramp_delay not set\n");
+		return 0;
+	}
+
+	return DIV_ROUND_UP(abs(new_uV - old_uV), ramp_delay);
+}
+
 static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 				     int min_uV, int max_uV)
 {
@@ -2752,6 +2770,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	unsigned int selector;
 	int old_selector = -1;
 	const struct regulator_ops *ops = rdev->desc->ops;
+	int old_uV = _regulator_get_voltage(rdev);
 
 	trace_regulator_set_voltage(rdev_get_name(rdev), min_uV, max_uV);
 
@@ -2803,23 +2822,37 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 	if (ret)
 		goto out;
 
-	/* Call set_voltage_time_sel if successfully obtained old_selector */
-	if (!old_selector >= 0 && old_selector != selector) {
-		delay = ops->set_voltage_time_sel(rdev,
-						old_selector, selector);
-		if (delay < 0) {
-			rdev_warn(rdev, "set_voltage_time_sel() failed: %d\n",
-				  delay);
-			delay = 0;
+	if (ops->set_voltage_time_sel) {
+		/*
+		 * Call set_voltage_time_sel if successfully obtained
+		 * old_selector
+		 */
+		if (old_selector >= 0 && old_selector != selector)
+			delay = ops->set_voltage_time_sel(rdev, old_selector,
+							  selector);
+	} else {
+		if (old_uV != best_val) {
+			if (ops->set_voltage_time)
+				delay = ops->set_voltage_time(rdev, old_uV,
+							      best_val);
+			else
+				delay = _regulator_set_voltage_time(rdev,
+								    old_uV,
+								    best_val);
 		}
+	}
 
-		/* Insert any necessary delays */
-		if (delay >= 1000) {
-			mdelay(delay / 1000);
-			udelay(delay % 1000);
-		} else if (delay) {
-			udelay(delay);
-		}
+	if (delay < 0) {
+		rdev_warn(rdev, "failed to get delay: %d\n", delay);
+		delay = 0;
+	}
+
+	/* Insert any necessary delays */
+	if (delay >= 1000) {
+		mdelay(delay / 1000);
+		udelay(delay % 1000);
+	} else if (delay) {
+		udelay(delay);
 	}
 
 	if (best_val >= 0) {
@@ -3000,9 +3033,13 @@ int regulator_set_voltage_time(struct regulator *regulator,
 	int voltage;
 	int i;
 
+	if (ops->set_voltage_time)
+		return ops->set_voltage_time(rdev, old_uV, new_uV);
+	else if (!ops->set_voltage_time_sel)
+		return _regulator_set_voltage_time(rdev, old_uV, new_uV);
+
 	/* Currently requires operations to do this */
-	if (!ops->list_voltage || !ops->set_voltage_time_sel
-	    || !rdev->desc->n_voltages)
+	if (!ops->list_voltage || !rdev->desc->n_voltages)
 		return -EINVAL;
 
 	for (i = 0; i < rdev->desc->n_voltages; i++) {
@@ -3041,17 +3078,8 @@ int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 				   unsigned int old_selector,
 				   unsigned int new_selector)
 {
-	unsigned int ramp_delay = 0;
 	int old_volt, new_volt;
 
-	if (rdev->constraints->ramp_delay)
-		ramp_delay = rdev->constraints->ramp_delay;
-	else if (rdev->desc->ramp_delay)
-		ramp_delay = rdev->desc->ramp_delay;
-
-	if (ramp_delay == 0)
-		return 0;
-
 	/* sanity check */
 	if (!rdev->desc->ops->list_voltage)
 		return -EINVAL;
@@ -3059,7 +3087,11 @@ int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 	old_volt = rdev->desc->ops->list_voltage(rdev, old_selector);
 	new_volt = rdev->desc->ops->list_voltage(rdev, new_selector);
 
-	return DIV_ROUND_UP(abs(new_volt - old_volt), ramp_delay);
+	if (rdev->desc->ops->set_voltage_time)
+		return rdev->desc->ops->set_voltage_time(rdev, old_volt,
+							 new_volt);
+	else
+		return _regulator_set_voltage_time(rdev, old_volt, new_volt);
 }
 EXPORT_SYMBOL_GPL(regulator_set_voltage_time_sel);
 
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index c24524242da2..1b88e0e15a70 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -10,7 +10,6 @@
  * published by the Free Software Foundation.
  */
 
-#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/err.h>
@@ -194,12 +193,10 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
 	unsigned int min_uV_duty = drvdata->continuous.min_uV_dutycycle;
 	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;
 	int min_uV = rdev->constraints->min_uV;
 	int max_uV = rdev->constraints->max_uV;
 	int diff_uV = max_uV - min_uV;
 	struct pwm_state pstate;
-	int old_uV = pwm_regulator_get_voltage(rdev);
 	unsigned int diff_duty;
 	unsigned int dutycycle;
 	int ret;
@@ -233,13 +230,6 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
 		return ret;
 	}
 
-	if ((ramp_delay == 0) || !pwm_regulator_is_enabled(rdev))
-		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;
 }
 
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index fcfa40a6692c..37b532410528 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -113,10 +113,14 @@ struct regulator_linear_range {
  *               stabilise after being enabled, in microseconds.
  * @set_ramp_delay: Set the ramp delay for the regulator. The driver should
  *		select ramp delay equal to or less than(closest) ramp_delay.
+ * @set_voltage_time: Time taken for the regulator voltage output voltage
+ *               to stabilise after being set to a new value, in microseconds.
+ *               The function receives the from and to voltage as input, it
+ *               should return the worst case.
  * @set_voltage_time_sel: Time taken for the regulator voltage output voltage
  *               to stabilise after being set to a new value, in microseconds.
- *               The function provides the from and to voltage selector, the
- *               function should return the worst case.
+ *               The function receives the from and to voltage selector as
+ *               input, it should return the worst case.
  * @set_soft_start: Enable soft start for the regulator.
  *
  * @set_suspend_voltage: Set the voltage for the regulator when the system
@@ -168,6 +172,8 @@ struct regulator_ops {
 	/* Time taken to enable or set voltage on the regulator */
 	int (*enable_time) (struct regulator_dev *);
 	int (*set_ramp_delay) (struct regulator_dev *, int ramp_delay);
+	int (*set_voltage_time) (struct regulator_dev *, int old_uV,
+				 int new_uV);
 	int (*set_voltage_time_sel) (struct regulator_dev *,
 				     unsigned int old_selector,
 				     unsigned int new_selector);
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Applied "regulator: core: Don't skip set_voltage_time when ramp delay disabled" to the regulator tree
@ 2016-09-16 17:40     ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2016-09-16 17:40 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Mark Brown, Mark Brown, lgirdwood, Douglas Anderson, briannorris,
	javier, robh+dt, mark.rutland, linux-kernel, devicetree

The patch

   regulator: core: Don't skip set_voltage_time when ramp delay disabled

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From d89564efe79419a093e966a959bf5ba2c94e693f Mon Sep 17 00:00:00 2001
From: Matthias Kaehlcke <mka@chromium.org>
Date: Wed, 14 Sep 2016 09:52:07 -0700
Subject: [PATCH] regulator: core: Don't skip set_voltage_time when ramp delay
 disabled

The current code assumes that only the ramp_delay is used to determine
the time needed for the voltage to stabilize. This may be true for the
calculation done by regulator_set_voltage_time_sel(), however regulators
can implement their own set_voltage_time_sel() op which would be skipped
if no ramp delay is specified. Remove the check in
_regulator_do_set_voltage(), the functions calculating the ramp delay
return 0 anyway when the ramp delay is not configured.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b0076ccf896b..df3028b2a8e9 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2804,9 +2804,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 		goto out;
 
 	/* Call set_voltage_time_sel if successfully obtained old_selector */
-	if (!rdev->constraints->ramp_disable && old_selector >= 0
-		&& old_selector != selector) {
-
+	if (!old_selector >= 0 && old_selector != selector) {
 		delay = ops->set_voltage_time_sel(rdev,
 						old_selector, selector);
 		if (delay < 0) {
@@ -3051,10 +3049,8 @@ int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 	else if (rdev->desc->ramp_delay)
 		ramp_delay = rdev->desc->ramp_delay;
 
-	if (ramp_delay == 0) {
-		rdev_warn(rdev, "ramp_delay not set\n");
+	if (ramp_delay == 0)
 		return 0;
-	}
 
 	/* sanity check */
 	if (!rdev->desc->ops->list_voltage)
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Applied "regulator: core: Don't skip set_voltage_time when ramp delay disabled" to the regulator tree
@ 2016-09-16 17:40     ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2016-09-16 17:40 UTC (permalink / raw)
  To: Matthias Kaehlcke; +Cc: Mark Brown

The patch

   regulator: core: Don't skip set_voltage_time when ramp delay disabled

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From d89564efe79419a093e966a959bf5ba2c94e693f Mon Sep 17 00:00:00 2001
From: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Date: Wed, 14 Sep 2016 09:52:07 -0700
Subject: [PATCH] regulator: core: Don't skip set_voltage_time when ramp delay
 disabled

The current code assumes that only the ramp_delay is used to determine
the time needed for the voltage to stabilize. This may be true for the
calculation done by regulator_set_voltage_time_sel(), however regulators
can implement their own set_voltage_time_sel() op which would be skipped
if no ramp delay is specified. Remove the check in
_regulator_do_set_voltage(), the functions calculating the ramp delay
return 0 anyway when the ramp delay is not configured.

Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/regulator/core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b0076ccf896b..df3028b2a8e9 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2804,9 +2804,7 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 		goto out;
 
 	/* Call set_voltage_time_sel if successfully obtained old_selector */
-	if (!rdev->constraints->ramp_disable && old_selector >= 0
-		&& old_selector != selector) {
-
+	if (!old_selector >= 0 && old_selector != selector) {
 		delay = ops->set_voltage_time_sel(rdev,
 						old_selector, selector);
 		if (delay < 0) {
@@ -3051,10 +3049,8 @@ int regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 	else if (rdev->desc->ramp_delay)
 		ramp_delay = rdev->desc->ramp_delay;
 
-	if (ramp_delay == 0) {
-		rdev_warn(rdev, "ramp_delay not set\n");
+	if (ramp_delay == 0)
 		return 0;
-	}
 
 	/* sanity check */
 	if (!rdev->desc->ops->list_voltage)
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Applied "regulator: core: Simplify error flow in _regulator_do_set_voltage()" to the regulator tree
@ 2016-09-16 17:40     ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2016-09-16 17:40 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Mark Brown, Mark Brown, lgirdwood, Douglas Anderson, briannorris,
	javier, robh+dt, mark.rutland, linux-kernel, devicetree

The patch

   regulator: core: Simplify error flow in _regulator_do_set_voltage()

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 31dfe686ed0ba5a796bcfc5a6745e77ddb5daa4e Mon Sep 17 00:00:00 2001
From: Matthias Kaehlcke <mka@chromium.org>
Date: Wed, 14 Sep 2016 09:52:06 -0700
Subject: [PATCH] regulator: core: Simplify error flow in
 _regulator_do_set_voltage()

If the voltage can not be set jump to the end of the function. This
avoids having to check for an error multiple times and eliminates one
level of nesting in a follow-up change.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b059e8334567..b0076ccf896b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2800,8 +2800,11 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 		ret = -EINVAL;
 	}
 
+	if (ret)
+		goto out;
+
 	/* Call set_voltage_time_sel if successfully obtained old_selector */
-	if (ret == 0 && !rdev->constraints->ramp_disable && old_selector >= 0
+	if (!rdev->constraints->ramp_disable && old_selector >= 0
 		&& old_selector != selector) {
 
 		delay = ops->set_voltage_time_sel(rdev,
@@ -2821,13 +2824,14 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 		}
 	}
 
-	if (ret == 0 && best_val >= 0) {
+	if (best_val >= 0) {
 		unsigned long data = best_val;
 
 		_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE,
 				     (void *)data);
 	}
 
+out:
 	trace_regulator_set_voltage_complete(rdev_get_name(rdev), best_val);
 
 	return ret;
-- 
2.8.1

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Applied "regulator: core: Simplify error flow in _regulator_do_set_voltage()" to the regulator tree
@ 2016-09-16 17:40     ` Mark Brown
  0 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2016-09-16 17:40 UTC (permalink / raw)
  To: Matthias Kaehlcke; +Cc: Mark Brown

The patch

   regulator: core: Simplify error flow in _regulator_do_set_voltage()

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 31dfe686ed0ba5a796bcfc5a6745e77ddb5daa4e Mon Sep 17 00:00:00 2001
From: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Date: Wed, 14 Sep 2016 09:52:06 -0700
Subject: [PATCH] regulator: core: Simplify error flow in
 _regulator_do_set_voltage()

If the voltage can not be set jump to the end of the function. This
avoids having to check for an error multiple times and eliminates one
level of nesting in a follow-up change.

Signed-off-by: Matthias Kaehlcke <mka-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/regulator/core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b059e8334567..b0076ccf896b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2800,8 +2800,11 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 		ret = -EINVAL;
 	}
 
+	if (ret)
+		goto out;
+
 	/* Call set_voltage_time_sel if successfully obtained old_selector */
-	if (ret == 0 && !rdev->constraints->ramp_disable && old_selector >= 0
+	if (!rdev->constraints->ramp_disable && old_selector >= 0
 		&& old_selector != selector) {
 
 		delay = ops->set_voltage_time_sel(rdev,
@@ -2821,13 +2824,14 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 		}
 	}
 
-	if (ret == 0 && best_val >= 0) {
+	if (best_val >= 0) {
 		unsigned long data = best_val;
 
 		_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE,
 				     (void *)data);
 	}
 
+out:
 	trace_regulator_set_voltage_complete(rdev_get_name(rdev), best_val);
 
 	return ret;
-- 
2.8.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 5/6] regulator: core: Add support for a fixed delay after voltage changes
  2016-09-14 16:52   ` Matthias Kaehlcke
  (?)
@ 2016-09-23 15:14   ` Rob Herring
  -1 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2016-09-23 15:14 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Mark Brown, lgirdwood, Douglas Anderson, briannorris, javier,
	mark.rutland, linux-kernel, devicetree

On Wed, Sep 14, 2016 at 09:52:09AM -0700, Matthias Kaehlcke wrote:
> The target voltage is not necessarily reached inmediately after
> requesting a regulator to change the voltage. In some cases the
> ramp_delay can be used to calculate the stabilisation time, in others
> there is no direct relationship between the delta in the voltage and
> the stabilisation time. This change introduces the device tree properties
> "regulator-settle-time-up-us"/"regulator-settle-time-down-us" which
> allow to specify a fixed delay after a voltage increase or decrease.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v5:
> - Added support for delay on voltage decreases
> - Don't skip set_voltage_time op if no settle time and ramp delay
> 
>  .../devicetree/bindings/regulator/regulator.txt        |  4 ++++

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/regulator/core.c                               | 18 +++++++++++-------
>  drivers/regulator/of_regulator.c                       |  8 ++++++++
>  include/linux/regulator/machine.h                      |  4 ++++
>  4 files changed, 27 insertions(+), 7 deletions(-)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 6/6] regulator: core: Prevent falling too fast
  2016-09-14 16:52   ` Matthias Kaehlcke
  (?)
  (?)
@ 2016-09-23 15:16   ` Rob Herring
  -1 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2016-09-23 15:16 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Mark Brown, lgirdwood, Douglas Anderson, briannorris, javier,
	mark.rutland, linux-kernel, devicetree

On Wed, Sep 14, 2016 at 09:52:10AM -0700, Matthias Kaehlcke wrote:
> From: Douglas Anderson <dianders@chromium.org>
> 
> On some boards it is possible that transitioning the regulator downwards
> too fast will trigger the over voltage protection (OVP) on the
> regulator. This is because until the voltage actually falls there is
> time when the requested voltage is much lower than the actual voltage.
> 
> We'll fix this OVP problem by allowing users to specify the maximum
> voltage that we can safely fall. The maximum safe voltage decrease
> is specified as a percentage of the current voltage. The driver will
> then break things into separate steps with a delay in between.
> 
> In order to figure out what the delay should be we need to figure out
> how slowly the voltage rail might fall in the worst (slowest) case.
> We'll assume this worst case is present and delay so we know for sure
> that we've finished each step.
> 
> In this patch we actually block returning from the set_voltage() call
> until we've finished delaying. A future patch atop this one might
> choose to return more immediately and let the voltages fall in the
> background. That would possibly allow us to cancel a slow downward
> decay if there was a request to go back up.
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v5:
> - Leave set_voltage tracepoints where they were
> - Fixed error handling in code dealing with the device tree, return an error if configuration is invalid
> - Fixed coding style and formatting issues
> - Updated commit message
> 
>  .../devicetree/bindings/regulator/regulator.txt    |  7 ++++

For the binding:

Acked-by: Rob Herring <robh@kernel.org>

>  drivers/regulator/core.c                           | 49 +++++++++++++++++++---
>  drivers/regulator/of_regulator.c                   | 42 ++++++++++++++++++-
>  include/linux/regulator/machine.h                  |  2 +
>  4 files changed, 93 insertions(+), 7 deletions(-)

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-09-23 15:16 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-14 16:52 [PATCH v5 1/6] regulator: core: Use local ops variable in _regulator_do_set_voltage() Matthias Kaehlcke
2016-09-14 16:52 ` [PATCH v5 2/6] regulator: core: Simplify error flow " Matthias Kaehlcke
2016-09-16 17:40   ` Applied "regulator: core: Simplify error flow in _regulator_do_set_voltage()" to the regulator tree Mark Brown
2016-09-16 17:40     ` Mark Brown
2016-09-14 16:52 ` [PATCH v5 3/6] regulator: core: Don't skip set_voltage_time when ramp delay disabled Matthias Kaehlcke
2016-09-16 17:40   ` Applied "regulator: core: Don't skip set_voltage_time when ramp delay disabled" to the regulator tree Mark Brown
2016-09-16 17:40     ` Mark Brown
2016-09-14 16:52 ` [PATCH v5 4/6] regulator: core: Add set_voltage_time op Matthias Kaehlcke
2016-09-16 17:40   ` Applied "regulator: core: Add set_voltage_time op" to the regulator tree Mark Brown
2016-09-16 17:40     ` Mark Brown
2016-09-14 16:52 ` [PATCH v5 5/6] regulator: core: Add support for a fixed delay after voltage changes Matthias Kaehlcke
2016-09-14 16:52   ` Matthias Kaehlcke
2016-09-23 15:14   ` Rob Herring
2016-09-14 16:52 ` [PATCH v5 6/6] regulator: core: Prevent falling too fast Matthias Kaehlcke
2016-09-14 16:52   ` Matthias Kaehlcke
2016-09-14 21:35   ` kbuild test robot
2016-09-14 21:35     ` kbuild test robot
2016-09-23 15:16   ` Rob Herring
2016-09-14 17:16 ` Applied "regulator: core: Use local ops variable in _regulator_do_set_voltage()" to the regulator tree Mark Brown
2016-09-14 17:16   ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.