[PATCH 1/5] regulator: More explict error reporting for fixed regulator

[PATCH 1/5] regulator: More explict error reporting for fixed regulator

[Mark Brown]

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

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index cdc674f..9c7f956 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -70,12 +70,14 @@ static int regulator_fixed_voltage_probe(struct platform_device *pdev)
 
 	drvdata = kzalloc(sizeof(struct fixed_voltage_data), GFP_KERNEL);
 	if (drvdata == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate device data\n");
 		ret = -ENOMEM;
 		goto err;
 	}
 
 	drvdata->desc.name = kstrdup(config->supply_name, GFP_KERNEL);
 	if (drvdata->desc.name == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate supply name\n");
 		ret = -ENOMEM;
 		goto err;
 	}
@@ -90,6 +92,7 @@ static int regulator_fixed_voltage_probe(struct platform_device *pdev)
 					  config->init_data, drvdata);
 	if (IS_ERR(drvdata->dev)) {
 		ret = PTR_ERR(drvdata->dev);
+		dev_err(&pdev->dev, "Failed to register regulator: %d\n", ret);
 		goto err_name;
 	}
 
-- 
1.6.3.3

[PATCH 2/5] regulator: Check for constraints before using them for name

[Mark Brown]

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

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e38db55..6e0c723 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -232,7 +232,7 @@ static ssize_t regulator_name_show(struct device *dev,
 	struct regulator_dev *rdev = dev_get_drvdata(dev);
 	const char *name;
 
-	if (rdev->constraints->name)
+	if (rdev->constraints && rdev->constraints->name)
 		name = rdev->constraints->name;
 	else if (rdev->desc->name)
 		name = rdev->desc->name;
-- 
1.6.3.3

[PATCH 3/5] regulator: Check for constraints in regulator_init_complete()

[Mark Brown]

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

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 6e0c723..dfbf431 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2409,14 +2409,14 @@ static int __init regulator_init_complete(void)
 		ops = rdev->desc->ops;
 		c = rdev->constraints;
 
-		if (c->name)
+		if (c && c->name)
 			name = c->name;
 		else if (rdev->desc->name)
 			name = rdev->desc->name;
 		else
 			name = "regulator";
 
-		if (!ops->disable || c->always_on)
+		if (!ops->disable || (c && c->always_on))
 			continue;
 
 		mutex_lock(&rdev->mutex);
-- 
1.6.3.3

[PATCH 4/5] regulator: Push locking for regulator_is_enabled() out

[Mark Brown]

Allows use by more of the internal regulator API code.

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

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index dfbf431..60fcd98 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -280,8 +280,13 @@ static ssize_t regulator_state_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {
 	struct regulator_dev *rdev = dev_get_drvdata(dev);
+	ssize_t ret;
+
+	mutex_lock(&rdev->mutex);
+	ret = regulator_print_state(buf, _regulator_is_enabled(rdev));
+	mutex_unlock(&rdev->mutex);
 
-	return regulator_print_state(buf, _regulator_is_enabled(rdev));
+	return ret;
 }
 static DEVICE_ATTR(state, 0444, regulator_state_show, NULL);
 
@@ -1365,20 +1370,11 @@ EXPORT_SYMBOL_GPL(regulator_force_disable);
 
 static int _regulator_is_enabled(struct regulator_dev *rdev)
 {
-	int ret;
-
-	mutex_lock(&rdev->mutex);
-
 	/* sanity check */
-	if (!rdev->desc->ops->is_enabled) {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (!rdev->desc->ops->is_enabled)
+		return -EINVAL;
 
-	ret = rdev->desc->ops->is_enabled(rdev);
-out:
-	mutex_unlock(&rdev->mutex);
-	return ret;
+	return rdev->desc->ops->is_enabled(rdev);
 }
 
 /**
@@ -1395,7 +1391,13 @@ out:
  */
 int regulator_is_enabled(struct regulator *regulator)
 {
-	return _regulator_is_enabled(regulator->rdev);
+	int ret;
+
+	mutex_lock(&regulator->rdev->mutex);
+	ret = _regulator_is_enabled(regulator->rdev);
+	mutex_unlock(&regulator->rdev->mutex);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(regulator_is_enabled);
 
-- 
1.6.3.3

[PATCH 5/5] regulator: regulator_enable() permission checking

[Mark Brown]

The regulator_enable() code wasn't actually checking that the
machine constraints had given permission to enable the regulator.
Add code to do that, but only if the regulator is not already on
due to something like always_on or being left on at startup since
in those cases there's no physical change being introduced and the
constraint wouldn't make any sense.

Also add matching code for disable(). We need to do less there since
either regulator_enable() should have succeeded first or the board
setup makes no sense.

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

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 60fcd98..dbf27bf 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1191,16 +1191,21 @@ void regulator_put(struct regulator *regulator)
 }
 EXPORT_SYMBOL_GPL(regulator_put);
 
+static int _regulator_can_change_status(struct regulator_dev *rdev)
+{
+	if (!rdev->constraints)
+		return 0;
+
+	if (rdev->constraints->valid_ops_mask & REGULATOR_CHANGE_STATUS)
+		return 1;
+	else
+		return 0;
+}
+
 /* locks held by regulator_enable() */
 static int _regulator_enable(struct regulator_dev *rdev)
 {
-	int ret = -EINVAL;
-
-	if (!rdev->constraints) {
-		printk(KERN_ERR "%s: %s has no constraints\n",
-		       __func__, rdev->desc->name);
-		return ret;
-	}
+	int ret;
 
 	/* do we need to enable the supply regulator first */
 	if (rdev->supply) {
@@ -1213,24 +1218,34 @@ static int _regulator_enable(struct regulator_dev *rdev)
 	}
 
 	/* check voltage and requested load before enabling */
-	if (rdev->desc->ops->enable) {
+	if (rdev->constraints &&
+	    (rdev->constraints->valid_ops_mask & REGULATOR_CHANGE_DRMS))
+		drms_uA_update(rdev);
 
-		if (rdev->constraints &&
-			(rdev->constraints->valid_ops_mask &
-			REGULATOR_CHANGE_DRMS))
-			drms_uA_update(rdev);
-
-		ret = rdev->desc->ops->enable(rdev);
-		if (ret < 0) {
-			printk(KERN_ERR "%s: failed to enable %s: %d\n",
+	if (rdev->use_count == 0) {
+		/* The regulator may on if it's not switchable or left on */
+		ret = _regulator_is_enabled(rdev);
+		if (ret == -EINVAL || ret == 0) {
+			if (!_regulator_can_change_status(rdev))
+				return -EPERM;
+
+			if (rdev->desc->ops->enable) {
+				ret = rdev->desc->ops->enable(rdev);
+				if (ret < 0)
+					return ret;
+			} else {
+				return -EINVAL;
+			}
+		} else {
+			printk(KERN_ERR "%s: is_enabled() failed for %s: %d\n",
 			       __func__, rdev->desc->name, ret);
 			return ret;
 		}
-		rdev->use_count++;
-		return ret;
 	}
 
-	return ret;
+	rdev->use_count++;
+
+	return 0;
 }
 
 /**
@@ -1270,7 +1285,8 @@ static int _regulator_disable(struct regulator_dev *rdev)
 	if (rdev->use_count == 1 && !rdev->constraints->always_on) {
 
 		/* we are last user */
-		if (rdev->desc->ops->disable) {
+		if (_regulator_can_change_status(rdev) &&
+		    rdev->desc->ops->disable) {
 			ret = rdev->desc->ops->disable(rdev);
 			if (ret < 0) {
 				printk(KERN_ERR "%s: failed to disable %s\n",
-- 
1.6.3.3

Re: [PATCH 1/5] regulator: More explict error reporting for fixed regulator

[Liam Girdwood]

On Mon, 2009-08-03 at 18:49 +0100, Mark Brown wrote:
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/regulator/fixed.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Applied All.

Thanks !

Liam