[PATCH 1/2] regulator: Factor out voltage set operation into a separate function

[PATCH 1/2] regulator: Factor out voltage set operation into a separate function

[Mark Brown]

Push all the callers of the chip set_voltage() operation out into a single
function to facilitiate future refactoring.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/regulator/core.c |   63 ++++++++++++++++++++++++++++-----------------
 1 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index b362dbd..23c5f7c 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -83,6 +83,8 @@ static int _regulator_get_current_limit(struct regulator_dev *rdev);
 static unsigned int _regulator_get_mode(struct regulator_dev *rdev);
 static void _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 const char *rdev_get_name(struct regulator_dev *rdev)
 {
@@ -745,22 +747,19 @@ static int machine_constraints_voltage(struct regulator_dev *rdev,
 {
 	struct regulator_ops *ops = rdev->desc->ops;
 	int ret;
-	unsigned selector;
 
 	/* do we need to apply the constraint voltage */
 	if (rdev->constraints->apply_uV &&
-		rdev->constraints->min_uV == rdev->constraints->max_uV &&
-		ops->set_voltage) {
-		ret = ops->set_voltage(rdev,
-				       rdev->constraints->min_uV,
-				       rdev->constraints->max_uV,
-				       &selector);
-			if (ret < 0) {
-				rdev_err(rdev, "failed to apply %duV constraint\n",
-					 rdev->constraints->min_uV);
-				rdev->constraints = NULL;
-				return ret;
-			}
+	    rdev->constraints->min_uV == rdev->constraints->max_uV) {
+		ret = _regulator_do_set_voltage(rdev,
+						rdev->constraints->min_uV,
+						rdev->constraints->max_uV);
+		if (ret < 0) {
+			rdev_err(rdev, "failed to apply %duV constraint\n",
+				 rdev->constraints->min_uV);
+			rdev->constraints = NULL;
+			return ret;
+		}
 	}
 
 	/* constrain machine-level voltage specs to fit
@@ -1621,6 +1620,32 @@ int regulator_is_supported_voltage(struct regulator *regulator,
 	return 0;
 }
 
+static int _regulator_do_set_voltage(struct regulator_dev *rdev,
+				     int min_uV, int max_uV)
+{
+	int ret;
+	unsigned int selector;
+
+	trace_regulator_set_voltage(rdev_get_name(rdev), min_uV, max_uV);
+
+	if (rdev->desc->ops->set_voltage) {
+		ret = rdev->desc->ops->set_voltage(rdev, min_uV, max_uV,
+						   &selector);
+
+		if (rdev->desc->ops->list_voltage)
+			selector = rdev->desc->ops->list_voltage(rdev,
+								 selector);
+		else
+			selector = -1;
+	} else {
+		ret = -EINVAL;
+	}
+
+	trace_regulator_set_voltage_complete(rdev_get_name(rdev), selector);
+
+	return ret;
+}
+
 /**
  * regulator_set_voltage - set regulator output voltage
  * @regulator: regulator source
@@ -1643,7 +1668,6 @@ int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
 {
 	struct regulator_dev *rdev = regulator->rdev;
 	int ret;
-	unsigned selector;
 
 	mutex_lock(&rdev->mutex);
 
@@ -1664,16 +1688,7 @@ int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
 	if (ret < 0)
 		goto out;
 
-	trace_regulator_set_voltage(rdev_get_name(rdev), min_uV, max_uV);
-
-	ret = rdev->desc->ops->set_voltage(rdev, min_uV, max_uV, &selector);
-
-	if (rdev->desc->ops->list_voltage)
-		selector = rdev->desc->ops->list_voltage(rdev, selector);
-	else
-		selector = -1;
-
-	trace_regulator_set_voltage_complete(rdev_get_name(rdev), selector);
+	ret = _regulator_do_set_voltage(rdev, min_uV, max_uV);
 
 out:
 	_notifier_call_chain(rdev, REGULATOR_EVENT_VOLTAGE_CHANGE, NULL);
-- 
1.7.1

[PATCH 2/2] regulator: Provide a selector based set_voltage_sel() operation

[Mark Brown]

Many regulator drivers implement voltage setting by looping through a
table of possible values, normally because the set of available voltages
can't be mapped onto selectors with simple calcuation. Factor out these
loops by providing a variant of set_voltage() which takes a selector rather
than a voltage range as an argument and implementing a loop through the
available selectors in the core.

This is not going to be suitable for use with all devices as when the
regulator voltage can be mapped onto selector values with a simple
calculation the linear scan through the available values will be more
expensive than just doing the calculation, especially for regulators
that provide fine grained voltage control.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/regulator/core.c         |   37 +++++++++++++++++++++++++++++++++++--
 include/linux/regulator/driver.h |    3 +++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 23c5f7c..a0579f0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1637,6 +1637,32 @@ static int _regulator_do_set_voltage(struct regulator_dev *rdev,
 								 selector);
 		else
 			selector = -1;
+	} else if (rdev->desc->ops->set_voltage_sel) {
+		int best_val = INT_MAX;
+		int i;
+
+		selector = 0;
+
+		/* Find the smallest voltage that falls within the specified
+		 * range.
+		 */
+		for (i = 0; i < rdev->desc->n_voltages; i++) {
+			ret = rdev->desc->ops->list_voltage(rdev, i);
+			if (ret < 0)
+				continue;
+
+			if (ret < best_val && ret >= min_uV && ret <= max_uV) {
+				best_val = ret;
+				selector = i;
+			}
+		}
+
+		if (best_val != INT_MAX) {
+			ret = rdev->desc->ops->set_voltage_sel(rdev, selector);
+			selector = best_val;
+		} else {
+			ret = -EINVAL;
+		}
 	} else {
 		ret = -EINVAL;
 	}
@@ -1672,7 +1698,8 @@ int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV)
 	mutex_lock(&rdev->mutex);
 
 	/* sanity check */
-	if (!rdev->desc->ops->set_voltage) {
+	if (!rdev->desc->ops->set_voltage &&
+	    !rdev->desc->ops->set_voltage_sel) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -2256,7 +2283,7 @@ static int add_regulator_attributes(struct regulator_dev *rdev)
 		return status;
 
 	/* constraints need specific supporting methods */
-	if (ops->set_voltage) {
+	if (ops->set_voltage || ops->set_voltage_sel) {
 		status = device_create_file(dev, &dev_attr_min_microvolts);
 		if (status < 0)
 			return status;
@@ -2354,12 +2381,18 @@ struct regulator_dev *regulator_register(struct regulator_desc *regulator_desc,
 	/* Only one of each should be implemented */
 	WARN_ON(regulator_desc->ops->get_voltage &&
 		regulator_desc->ops->get_voltage_sel);
+	WARN_ON(regulator_desc->ops->set_voltage &&
+		regulator_desc->ops->set_voltage_sel);
 
 	/* If we're using selectors we must implement list_voltage. */
 	if (regulator_desc->ops->get_voltage_sel &&
 	    !regulator_desc->ops->list_voltage) {
 		return ERR_PTR(-EINVAL);
 	}
+	if (regulator_desc->ops->set_voltage_sel &&
+	    !regulator_desc->ops->list_voltage) {
+		return ERR_PTR(-EINVAL);
+	}
 
 	rdev = kzalloc(sizeof(struct regulator_dev), GFP_KERNEL);
 	if (rdev == NULL)
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index bf3e653..975ae06 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -42,6 +42,8 @@ enum regulator_status {
  *
  * @set_voltage: Set the voltage for the regulator within the range specified.
  *               The driver should select the voltage closest to min_uV.
+ * @set_voltage_sel: Set the voltage for the regulator using the specified
+ *                   selector.
  * @get_voltage: Return the currently configured voltage for the regulator.
  * @get_voltage_sel: Return the currently configured voltage selector for the
  *                   regulator.
@@ -83,6 +85,7 @@ struct regulator_ops {
 	/* get/set regulator voltage */
 	int (*set_voltage) (struct regulator_dev *, int min_uV, int max_uV,
 			    unsigned *selector);
+	int (*set_voltage_sel) (struct regulator_dev *, unsigned selector);
 	int (*get_voltage) (struct regulator_dev *);
 	int (*get_voltage_sel) (struct regulator_dev *);
 
-- 
1.7.1

Re: [PATCH 1/2] regulator: Factor out voltage set operation into a separate function

[Liam Girdwood]

On Mon, 2010-12-13 at 15:38 +0000, Mark Brown wrote:
> Push all the callers of the chip set_voltage() operation out into a single
> function to facilitiate future refactoring.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/regulator/core.c |   63 ++++++++++++++++++++++++++++-----------------
>  1 files changed, 39 insertions(+), 24 deletions(-)
> 

Applied.

Thanks

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk