[PATCH 1/6] regulator: Also lift apply_uV into machine_constraints_voltage()

[PATCH 1/6] regulator: Also lift apply_uV into machine_constraints_voltage()

[Mark Brown]

It makes sense to do all the voltage configuration in the one split
out function.

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

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index c1a4991..1848a3f 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -676,6 +676,22 @@ static int machine_constraints_voltage(struct regulator_dev *rdev,
 	const char *name, struct regulation_constraints *constraints)
 {
 	struct regulator_ops *ops = rdev->desc->ops;
+	int ret;
+
+	/* 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);
+			if (ret < 0) {
+				printk(KERN_ERR "%s: failed to apply %duV constraint to %s\n",
+				       __func__,
+				       rdev->constraints->min_uV, name);
+				rdev->constraints = NULL;
+				return ret;
+			}
+	}
 
 	/* constrain machine-level voltage specs to fit
 	 * the actual range supported by this regulator.
@@ -773,27 +789,12 @@ static int set_machine_constraints(struct regulator_dev *rdev,
 	else
 		name = "regulator";
 
+	rdev->constraints = constraints;
+
 	ret = machine_constraints_voltage(rdev, name, constraints);
 	if (ret != 0)
 		goto out;
 
-	rdev->constraints = constraints;
-
-	/* 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);
-			if (ret < 0) {
-				printk(KERN_ERR "%s: failed to apply %duV constraint to %s\n",
-				       __func__,
-				       rdev->constraints->min_uV, name);
-				rdev->constraints = NULL;
-				goto out;
-			}
-	}
-
 	/* do we need to setup our suspend state */
 	if (constraints->initial_state) {
 		ret = suspend_prepare(rdev, constraints->initial_state);
-- 
1.6.5

[PATCH 2/6] regulator: Display actual settings with constraints

[Mark Brown]

When voltage or current constraints are either missing or specify
a range display the actual setting along with the constraints if
we can. This can aid debugging of configuration problems.

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

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 1848a3f..d3be67e 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -641,25 +641,43 @@ static void print_constraints(struct regulator_dev *rdev)
 {
 	struct regulation_constraints *constraints = rdev->constraints;
 	char buf[80];
-	int count;
+	int count = 0;
+	int ret;
 
-	if (rdev->desc->type == REGULATOR_VOLTAGE) {
+	if (constraints->min_uV && constraints->max_uV) {
 		if (constraints->min_uV == constraints->max_uV)
-			count = sprintf(buf, "%d mV ",
-					constraints->min_uV / 1000);
+			count += sprintf(buf + count, "%d mV ",
+					 constraints->min_uV / 1000);
 		else
-			count = sprintf(buf, "%d <--> %d mV ",
-					constraints->min_uV / 1000,
-					constraints->max_uV / 1000);
-	} else {
+			count += sprintf(buf + count, "%d <--> %d mV ",
+					 constraints->min_uV / 1000,
+					 constraints->max_uV / 1000);
+	}
+
+	if (!constraints->min_uV ||
+	    constraints->min_uV != constraints->max_uV) {
+		ret = _regulator_get_voltage(rdev);
+		if (ret > 0)
+			count += sprintf(buf + count, "at %d mV ", ret / 1000);
+	}
+
+	if (constraints->min_uA && constraints->max_uA) {
 		if (constraints->min_uA == constraints->max_uA)
-			count = sprintf(buf, "%d mA ",
-					constraints->min_uA / 1000);
+			count += sprintf(buf + count, "%d mA ",
+					 constraints->min_uA / 1000);
 		else
-			count = sprintf(buf, "%d <--> %d mA ",
-					constraints->min_uA / 1000,
-					constraints->max_uA / 1000);
+			count += sprintf(buf + count, "%d <--> %d mA ",
+					 constraints->min_uA / 1000,
+					 constraints->max_uA / 1000);
 	}
+
+	if (!constraints->min_uA ||
+	    constraints->min_uA != constraints->max_uA) {
+		ret = _regulator_get_current_limit(rdev);
+		if (ret > 0)
+			count += sprintf(buf + count, "at %d uA ", ret / 1000);
+	}
+
 	if (constraints->valid_modes_mask & REGULATOR_MODE_FAST)
 		count += sprintf(buf + count, "fast ");
 	if (constraints->valid_modes_mask & REGULATOR_MODE_NORMAL)
-- 
1.6.5

[PATCH 3/6] regulator: Factor out regulator name pretty printing

[Mark Brown]

Some of the regulator API functions have code to allow the machine
constraints to override the device supplied name for the regulator
in the constraints in order to help tie logging to supplies on the
board and disambiguate when there is more than one regulator chip
in the system. Factor this code out into a new rdev_get_name()
function and use it throughout the regulator API so that we always
use the same name.

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

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index d3be67e..7d0c0d7 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -66,6 +66,16 @@ 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 const char *rdev_get_name(struct regulator_dev *rdev)
+{
+	if (rdev->constraints && rdev->constraints->name)
+		return rdev->constraints->name;
+	else if (rdev->desc->name)
+		return rdev->desc->name;
+	else
+		return "";
+}
+
 /* gets the regulator for a given consumer device */
 static struct regulator *get_device_regulator(struct device *dev)
 {
@@ -96,12 +106,12 @@ static int regulator_check_voltage(struct regulator_dev *rdev,
 
 	if (!rdev->constraints) {
 		printk(KERN_ERR "%s: no constraints for %s\n", __func__,
-		       rdev->desc->name);
+		       rdev_get_name(rdev));
 		return -ENODEV;
 	}
 	if (!(rdev->constraints->valid_ops_mask & REGULATOR_CHANGE_VOLTAGE)) {
 		printk(KERN_ERR "%s: operation not allowed for %s\n",
-		       __func__, rdev->desc->name);
+		       __func__, rdev_get_name(rdev));
 		return -EPERM;
 	}
 
@@ -124,12 +134,12 @@ static int regulator_check_current_limit(struct regulator_dev *rdev,
 
 	if (!rdev->constraints) {
 		printk(KERN_ERR "%s: no constraints for %s\n", __func__,
-		       rdev->desc->name);
+		       rdev_get_name(rdev));
 		return -ENODEV;
 	}
 	if (!(rdev->constraints->valid_ops_mask & REGULATOR_CHANGE_CURRENT)) {
 		printk(KERN_ERR "%s: operation not allowed for %s\n",
-		       __func__, rdev->desc->name);
+		       __func__, rdev_get_name(rdev));
 		return -EPERM;
 	}
 
@@ -159,17 +169,17 @@ static int regulator_check_mode(struct regulator_dev *rdev, int mode)
 
 	if (!rdev->constraints) {
 		printk(KERN_ERR "%s: no constraints for %s\n", __func__,
-		       rdev->desc->name);
+		       rdev_get_name(rdev));
 		return -ENODEV;
 	}
 	if (!(rdev->constraints->valid_ops_mask & REGULATOR_CHANGE_MODE)) {
 		printk(KERN_ERR "%s: operation not allowed for %s\n",
-		       __func__, rdev->desc->name);
+		       __func__, rdev_get_name(rdev));
 		return -EPERM;
 	}
 	if (!(rdev->constraints->valid_modes_mask & mode)) {
 		printk(KERN_ERR "%s: invalid mode %x for %s\n",
-		       __func__, mode, rdev->desc->name);
+		       __func__, mode, rdev_get_name(rdev));
 		return -EINVAL;
 	}
 	return 0;
@@ -180,12 +190,12 @@ static int regulator_check_drms(struct regulator_dev *rdev)
 {
 	if (!rdev->constraints) {
 		printk(KERN_ERR "%s: no constraints for %s\n", __func__,
-		       rdev->desc->name);
+		       rdev_get_name(rdev));
 		return -ENODEV;
 	}
 	if (!(rdev->constraints->valid_ops_mask & REGULATOR_CHANGE_DRMS)) {
 		printk(KERN_ERR "%s: operation not allowed for %s\n",
-		       __func__, rdev->desc->name);
+		       __func__, rdev_get_name(rdev));
 		return -EPERM;
 	}
 	return 0;
@@ -230,16 +240,8 @@ static ssize_t regulator_name_show(struct device *dev,
 			     struct device_attribute *attr, char *buf)
 {
 	struct regulator_dev *rdev = dev_get_drvdata(dev);
-	const char *name;
 
-	if (rdev->constraints && rdev->constraints->name)
-		name = rdev->constraints->name;
-	else if (rdev->desc->name)
-		name = rdev->desc->name;
-	else
-		name = "";
-
-	return sprintf(buf, "%s\n", name);
+	return sprintf(buf, "%s\n", rdev_get_name(rdev));
 }
 
 static ssize_t regulator_print_opmode(char *buf, int mode)
@@ -687,13 +689,14 @@ static void print_constraints(struct regulator_dev *rdev)
 	if (constraints->valid_modes_mask & REGULATOR_MODE_STANDBY)
 		count += sprintf(buf + count, "standby");
 
-	printk(KERN_INFO "regulator: %s: %s\n", rdev->desc->name, buf);
+	printk(KERN_INFO "regulator: %s: %s\n", rdev_get_name(rdev), buf);
 }
 
 static int machine_constraints_voltage(struct regulator_dev *rdev,
-	const char *name, struct regulation_constraints *constraints)
+	struct regulation_constraints *constraints)
 {
 	struct regulator_ops *ops = rdev->desc->ops;
+	const char *name = rdev_get_name(rdev);
 	int ret;
 
 	/* do we need to apply the constraint voltage */
@@ -800,16 +803,11 @@ static int set_machine_constraints(struct regulator_dev *rdev,
 	const char *name;
 	struct regulator_ops *ops = rdev->desc->ops;
 
-	if (constraints->name)
-		name = constraints->name;
-	else if (rdev->desc->name)
-		name = rdev->desc->name;
-	else
-		name = "regulator";
-
 	rdev->constraints = constraints;
 
-	ret = machine_constraints_voltage(rdev, name, constraints);
+	name = rdev_get_name(rdev);
+
+	ret = machine_constraints_voltage(rdev, constraints);
 	if (ret != 0)
 		goto out;
 
@@ -932,7 +930,7 @@ static int set_consumer_device_supply(struct regulator_dev *rdev,
 				dev_name(&node->regulator->dev),
 				node->regulator->desc->name,
 				supply,
-				dev_name(&rdev->dev), rdev->desc->name);
+				dev_name(&rdev->dev), rdev_get_name(rdev));
 		return -EBUSY;
 	}
 
@@ -1241,7 +1239,7 @@ static int _regulator_enable(struct regulator_dev *rdev)
 		ret = _regulator_enable(rdev->supply);
 		if (ret < 0) {
 			printk(KERN_ERR "%s: failed to enable %s: %d\n",
-			       __func__, rdev->desc->name, ret);
+			       __func__, rdev_get_name(rdev), ret);
 			return ret;
 		}
 	}
@@ -1267,7 +1265,7 @@ static int _regulator_enable(struct regulator_dev *rdev)
 			}
 		} else if (ret < 0) {
 			printk(KERN_ERR "%s: is_enabled() failed for %s: %d\n",
-			       __func__, rdev->desc->name, ret);
+			       __func__, rdev_get_name(rdev), ret);
 			return ret;
 		}
 		/* Fallthrough on positive return values - already enabled */
@@ -1308,7 +1306,7 @@ static int _regulator_disable(struct regulator_dev *rdev)
 
 	if (WARN(rdev->use_count <= 0,
 			"unbalanced disables for %s\n",
-			rdev->desc->name))
+			rdev_get_name(rdev)))
 		return -EIO;
 
 	/* are we the last user and permitted to disable ? */
@@ -1321,7 +1319,7 @@ static int _regulator_disable(struct regulator_dev *rdev)
 			ret = rdev->desc->ops->disable(rdev);
 			if (ret < 0) {
 				printk(KERN_ERR "%s: failed to disable %s\n",
-				       __func__, rdev->desc->name);
+				       __func__, rdev_get_name(rdev));
 				return ret;
 			}
 		}
@@ -1378,7 +1376,7 @@ static int _regulator_force_disable(struct regulator_dev *rdev)
 		ret = rdev->desc->ops->disable(rdev);
 		if (ret < 0) {
 			printk(KERN_ERR "%s: failed to force disable %s\n",
-			       __func__, rdev->desc->name);
+			       __func__, rdev_get_name(rdev));
 			return ret;
 		}
 		/* notify other consumers that power has been forced off */
@@ -1795,7 +1793,7 @@ int regulator_set_optimum_mode(struct regulator *regulator, int uA_load)
 	output_uV = rdev->desc->ops->get_voltage(rdev);
 	if (output_uV <= 0) {
 		printk(KERN_ERR "%s: invalid output voltage found for %s\n",
-			__func__, rdev->desc->name);
+			__func__, rdev_get_name(rdev));
 		goto out;
 	}
 
@@ -1806,7 +1804,7 @@ int regulator_set_optimum_mode(struct regulator *regulator, int uA_load)
 		input_uV = rdev->constraints->input_uV;
 	if (input_uV <= 0) {
 		printk(KERN_ERR "%s: invalid input voltage found for %s\n",
-			__func__, rdev->desc->name);
+			__func__, rdev_get_name(rdev));
 		goto out;
 	}
 
@@ -1820,7 +1818,7 @@ int regulator_set_optimum_mode(struct regulator *regulator, int uA_load)
 	ret = regulator_check_mode(rdev, mode);
 	if (ret < 0) {
 		printk(KERN_ERR "%s: failed to get optimum mode for %s @"
-			" %d uA %d -> %d uV\n", __func__, rdev->desc->name,
+			" %d uA %d -> %d uV\n", __func__, rdev_get_name(rdev),
 			total_uA_load, input_uV, output_uV);
 		goto out;
 	}
@@ -1828,7 +1826,7 @@ int regulator_set_optimum_mode(struct regulator *regulator, int uA_load)
 	ret = rdev->desc->ops->set_mode(rdev, mode);
 	if (ret < 0) {
 		printk(KERN_ERR "%s: failed to set optimum mode %x for %s\n",
-			__func__, mode, rdev->desc->name);
+			__func__, mode, rdev_get_name(rdev));
 		goto out;
 	}
 	ret = mode;
@@ -2346,7 +2344,7 @@ int regulator_suspend_prepare(suspend_state_t state)
 
 		if (ret < 0) {
 			printk(KERN_ERR "%s: failed to prepare %s\n",
-				__func__, rdev->desc->name);
+				__func__, rdev_get_name(rdev));
 			goto out;
 		}
 	}
@@ -2459,12 +2457,7 @@ static int __init regulator_init_complete(void)
 		ops = rdev->desc->ops;
 		c = rdev->constraints;
 
-		if (c && c->name)
-			name = c->name;
-		else if (rdev->desc->name)
-			name = rdev->desc->name;
-		else
-			name = "regulator";
+		name = rdev_get_name(rdev);
 
 		if (!ops->disable || (c && c->always_on))
 			continue;
-- 
1.6.5

[PATCH 4/6] regulator: Handle regulators without suspend mode configuration

[Mark Brown]

Since some regulators in the system may not support suspend mode
configuration we need to allow some regulators to have a missing
suspend mode configuration. Do this by requiring that disabled
regulators are explicitly flagged and then skip over regulators
that have no state specified.

Try to avoid surprises by warning the if we could set the state
but no configuration is provided.  This also ensures that an all
zeros configuration generates a warning rather than silently
disabling the regulator.

Reported-by: Joonyoung Shim <jy0922.shim@samsung.com>
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/regulator/core.c          |   25 ++++++++++++++++++++++---
 include/linux/regulator/machine.h |    6 +++++-
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 7d0c0d7..2dab0d9 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -581,10 +581,29 @@ static int suspend_set_state(struct regulator_dev *rdev,
 	struct regulator_state *rstate)
 {
 	int ret = 0;
+	bool can_set_state;
 
-	/* enable & disable are mandatory for suspend control */
-	if (!rdev->desc->ops->set_suspend_enable ||
-		!rdev->desc->ops->set_suspend_disable) {
+	can_set_state = rdev->desc->ops->set_suspend_enable &&
+		rdev->desc->ops->set_suspend_disable;
+
+	/* If we have no suspend mode configration don't set anything;
+	 * only warn if the driver actually makes the suspend mode
+	 * configurable.
+	 */
+	if (!rstate->enabled && !rstate->disabled) {
+		if (can_set_state)
+			printk(KERN_WARNING "%s: No configuration for %s\n",
+			       __func__, rdev_get_name(rdev));
+		return 0;
+	}
+
+	if (rstate->enabled && rstate->disabled) {
+		printk(KERN_ERR "%s: invalid configuration for %s\n",
+		       __func__, rdev_get_name(rdev));
+		return -EINVAL;
+	}
+
+	if (!can_set_state) {
 		printk(KERN_ERR "%s: no way to set suspend state\n",
 			__func__);
 		return -EINVAL;
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index 87f5f17..234a847 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -43,16 +43,20 @@ struct regulator;
 /**
  * struct regulator_state - regulator state during low power system states
  *
- * This describes a regulators state during a system wide low power state.
+ * This describes a regulators state during a system wide low power
+ * state.  One of enabled or disabled must be set for the
+ * configuration to be applied.
  *
  * @uV: Operating voltage during suspend.
  * @mode: Operating mode during suspend.
  * @enabled: Enabled during suspend.
+ * @disabled: Disabled during suspend.
  */
 struct regulator_state {
 	int uV;	/* suspend voltage */
 	unsigned int mode; /* suspend regulator operating mode */
 	int enabled; /* is regulator enabled in this suspend state */
+	int disabled; /* is the regulator disbled in this suspend state */
 };
 
 /**
-- 
1.6.5

[PATCH 5/6] regulator: Remove duplicate consts from ab3100

[Mark Brown]

'static const int const' means the same thing as 'static const int'
and sparse complains about this.

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

diff --git a/drivers/regulator/ab3100.c b/drivers/regulator/ab3100.c
index 49aeee8..5da127b 100644
--- a/drivers/regulator/ab3100.c
+++ b/drivers/regulator/ab3100.c
@@ -81,7 +81,7 @@ static const u8 ab3100_reg_init_order[AB3100_NUM_REGULATORS+2] = {
 #define LDO_C_VOLTAGE 2650000
 #define LDO_D_VOLTAGE 2650000
 
-static const int const ldo_e_buck_typ_voltages[] = {
+static const int ldo_e_buck_typ_voltages[] = {
 	1800000,
 	1400000,
 	1300000,
@@ -91,7 +91,7 @@ static const int const ldo_e_buck_typ_voltages[] = {
 	900000,
 };
 
-static const int const ldo_f_typ_voltages[] = {
+static const int ldo_f_typ_voltages[] = {
 	1800000,
 	1400000,
 	1300000,
@@ -102,21 +102,21 @@ static const int const ldo_f_typ_voltages[] = {
 	2650000,
 };
 
-static const int const ldo_g_typ_voltages[] = {
+static const int ldo_g_typ_voltages[] = {
 	2850000,
 	2750000,
 	1800000,
 	1500000,
 };
 
-static const int const ldo_h_typ_voltages[] = {
+static const int ldo_h_typ_voltages[] = {
 	2750000,
 	1800000,
 	1500000,
 	1200000,
 };
 
-static const int const ldo_k_typ_voltages[] = {
+static const int ldo_k_typ_voltages[] = {
 	2750000,
 	1800000,
 };
-- 
1.6.5

[PATCH 6/6] regulator: Ensure val is initialised in 88pm8607 choose_voltage()

[Mark Brown]

If we fall through it means that we hit an unknown regulator/chip
combination so set -ENOENT as an explicit flag (the return code
is only used internally).

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/regulator/88pm8607.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/88pm8607.c b/drivers/regulator/88pm8607.c
index e1aabda..0471955 100644
--- a/drivers/regulator/88pm8607.c
+++ b/drivers/regulator/88pm8607.c
@@ -170,7 +170,8 @@ static int choose_voltage(struct regulator_dev *rdev, int min_uV, int max_uV)
 {
 	struct pm8607_regulator_info *info = rdev_get_drvdata(rdev);
 	uint8_t chip_id = info->chip->chip_id;
-	int val, ret;
+	int val = -ENOENT;
+	int ret;
 
 	switch (info->desc.id) {
 	case PM8607_ID_BUCK1:
-- 
1.6.5

Re: [PATCH 1/6] regulator: Also lift apply_uV into machine_constraints_voltage()

[Liam Girdwood]

On Thu, 2009-10-22 at 16:31 +0100, Mark Brown wrote:
> It makes sense to do all the voltage configuration in the one split
> out function.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/regulator/core.c |   35 ++++++++++++++++++-----------------
>  1 files changed, 18 insertions(+), 17 deletions(-)
> 

All applied.

Thanks

Liam