[PATCH 0/5] regulator: A few fixes for supply resolution

[PATCH 0/5] regulator: A few fixes for supply resolution

[Jon Hunter]

This series is to mainly address the boot failures reported on the
Tegra124 Jetson TK1 where the AS3772 PMIC is failing to probe on -next [0].
However, there are a couple other patches I am including here which
came from looking at this problem. Here is a summary of the patches ...

Patch 1: Allows additional supplies to be resolved even if one fails
Patch 2: Fixes potential crash if turning on a supply fails
Patch 3-5: Address issue with AS3722 probe failing

[0] http://marc.info/?l=linux-tegra&m=145927821311669&w=2

Jon Hunter (5):
  regulator: core: Don't terminate supply resolution early
  regulator: core: Clear the supply pointer if enabling fails
  regulator: core: Move registration of regulator device
  regulator: core: Add early supply resolution for a bypassed regulator
  regulator: helpers: Ensure bypass register field matches ON value

 drivers/regulator/core.c    | 59 ++++++++++++++++++++++++++-------------------
 drivers/regulator/helpers.c |  2 +-
 2 files changed, 35 insertions(+), 26 deletions(-)

-- 
2.1.4

[PATCH 0/5] regulator: A few fixes for supply resolution

[Jon Hunter]

This series is to mainly address the boot failures reported on the
Tegra124 Jetson TK1 where the AS3772 PMIC is failing to probe on -next [0].
However, there are a couple other patches I am including here which
came from looking at this problem. Here is a summary of the patches ...

Patch 1: Allows additional supplies to be resolved even if one fails
Patch 2: Fixes potential crash if turning on a supply fails
Patch 3-5: Address issue with AS3722 probe failing

[0] http://marc.info/?l=linux-tegra&m=145927821311669&w=2

Jon Hunter (5):
  regulator: core: Don't terminate supply resolution early
  regulator: core: Clear the supply pointer if enabling fails
  regulator: core: Move registration of regulator device
  regulator: core: Add early supply resolution for a bypassed regulator
  regulator: helpers: Ensure bypass register field matches ON value

 drivers/regulator/core.c    | 59 ++++++++++++++++++++++++++-------------------
 drivers/regulator/helpers.c |  2 +-
 2 files changed, 35 insertions(+), 26 deletions(-)

-- 
2.1.4

[PATCH 1/5] regulator: core: Don't terminate supply resolution early

[Jon Hunter]

The function regulator_register_resolve_supply() is called from the
context of class_for_each_dev() (during the regulator registration) to
resolve any supplies added. regulator_register_resolve_supply() will
return an error if a regulator's supply cannot be resolved and this will
terminate the loop in class_for_each_dev(). This means that we will not
attempt to resolve any other supplies after one has failed. Hence, this
may delay the resolution of other regulator supplies until the failing
one itself can be resolved.

Rather than terminating the loop early, don't return an error code and
keep attempting to resolve any other supplies for regulators that have
been registered.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/regulator/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 61d3918f329e..a0b52ef11c30 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3865,7 +3865,12 @@ static void rdev_init_debugfs(struct regulator_dev *rdev)
 
 static int regulator_register_resolve_supply(struct device *dev, void *data)
 {
-	return regulator_resolve_supply(dev_to_rdev(dev));
+	struct regulator_dev *rdev = dev_to_rdev(dev);
+
+	if (regulator_resolve_supply(rdev))
+		rdev_dbg(rdev, "unable to resolve supply\n");
+
+	return 0;
 }
 
 /**
-- 
2.1.4

[PATCH 1/5] regulator: core: Don't terminate supply resolution early

[Jon Hunter]

The function regulator_register_resolve_supply() is called from the
context of class_for_each_dev() (during the regulator registration) to
resolve any supplies added. regulator_register_resolve_supply() will
return an error if a regulator's supply cannot be resolved and this will
terminate the loop in class_for_each_dev(). This means that we will not
attempt to resolve any other supplies after one has failed. Hence, this
may delay the resolution of other regulator supplies until the failing
one itself can be resolved.

Rather than terminating the loop early, don't return an error code and
keep attempting to resolve any other supplies for regulators that have
been registered.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/regulator/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 61d3918f329e..a0b52ef11c30 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3865,7 +3865,12 @@ static void rdev_init_debugfs(struct regulator_dev *rdev)
 
 static int regulator_register_resolve_supply(struct device *dev, void *data)
 {
-	return regulator_resolve_supply(dev_to_rdev(dev));
+	struct regulator_dev *rdev = dev_to_rdev(dev);
+
+	if (regulator_resolve_supply(rdev))
+		rdev_dbg(rdev, "unable to resolve supply\n");
+
+	return 0;
 }
 
 /**
-- 
2.1.4

[PATCH 2/5] regulator: core: Clear the supply pointer if enabling fails

[Jon Hunter]

During the resolution of a regulator's supply, we may attempt to enable
the supply if the regulator itself is already enabled. If enabling the
supply fails, then we will call _regulator_put() for the supply.
However, the pointer to the supply has not been cleared for the
regulator and this will cause a crash if we then unregister the
regulator and attempt to call regulator_put() a second time for the
supply. Fix this by clearing the supply pointer if enabling the supply
after fails when resolving the supply for a regulator.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/regulator/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a0b52ef11c30..39d05fcc07e9 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1545,6 +1545,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 		ret = regulator_enable(rdev->supply);
 		if (ret < 0) {
 			_regulator_put(rdev->supply);
+			rdev->supply = NULL;
 			return ret;
 		}
 	}
-- 
2.1.4

[PATCH 2/5] regulator: core: Clear the supply pointer if enabling fails

[Jon Hunter]

During the resolution of a regulator's supply, we may attempt to enable
the supply if the regulator itself is already enabled. If enabling the
supply fails, then we will call _regulator_put() for the supply.
However, the pointer to the supply has not been cleared for the
regulator and this will cause a crash if we then unregister the
regulator and attempt to call regulator_put() a second time for the
supply. Fix this by clearing the supply pointer if enabling the supply
after fails when resolving the supply for a regulator.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/regulator/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a0b52ef11c30..39d05fcc07e9 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1545,6 +1545,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 		ret = regulator_enable(rdev->supply);
 		if (ret < 0) {
 			_regulator_put(rdev->supply);
+			rdev->supply = NULL;
 			return ret;
 		}
 	}
-- 
2.1.4

[PATCH 3/5] regulator: core: Move registration of regulator device

[Jon Hunter]

The public functions to acquire a regulator, such as regulator_get(),
internally look-up the regulator from the list of regulators that have
been registered with the regulator device class. The registration of
a new regulator with the regulator device class happens before the
regulator has been completely setup. Therefore, it is possible that
the regulator could be acquired before it has been setup successfully.
To avoid this move the device registration of the regulator to the end
of the regulator setup and update the error exit path accordingly.

Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 drivers/regulator/core.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 39d05fcc07e9..754f3b4c2218 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3993,14 +3993,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	if (ret < 0)
 		goto wash;
 
-	ret = device_register(&rdev->dev);
-	if (ret != 0) {
-		put_device(&rdev->dev);
-		goto wash;
-	}
-
-	dev_set_drvdata(&rdev->dev, rdev);
-
 	if (init_data && init_data->supply_regulator)
 		rdev->supply_name = init_data->supply_regulator;
 	else if (regulator_desc->supply_name)
@@ -4020,9 +4012,17 @@ regulator_register(const struct regulator_desc *regulator_desc,
 		}
 	}
 
-	rdev_init_debugfs(rdev);
 	mutex_unlock(&regulator_list_mutex);
 
+	ret = device_register(&rdev->dev);
+	if (ret != 0) {
+		put_device(&rdev->dev);
+		goto unset_supplies;
+	}
+
+	dev_set_drvdata(&rdev->dev, rdev);
+	rdev_init_debugfs(rdev);
+
 	/* try to resolve regulators supply since a new one was registered */
 	class_for_each_device(&regulator_class, NULL, NULL,
 			      regulator_register_resolve_supply);
@@ -4031,17 +4031,11 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 unset_supplies:
 	unset_regulator_supplies(rdev);
-	regulator_ena_gpio_free(rdev);
-	device_unregister(&rdev->dev);
-	/* device core frees rdev */
-	goto out;
-
 wash:
 	kfree(rdev->constraints);
 	regulator_ena_gpio_free(rdev);
 clean:
 	kfree(rdev);
-out:
 	mutex_unlock(&regulator_list_mutex);
 	kfree(config);
 	return ERR_PTR(ret);
-- 
2.1.4

[PATCH 3/5] regulator: core: Move registration of regulator device

[Jon Hunter]

The public functions to acquire a regulator, such as regulator_get(),
internally look-up the regulator from the list of regulators that have
been registered with the regulator device class. The registration of
a new regulator with the regulator device class happens before the
regulator has been completely setup. Therefore, it is possible that
the regulator could be acquired before it has been setup successfully.
To avoid this move the device registration of the regulator to the end
of the regulator setup and update the error exit path accordingly.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/regulator/core.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 39d05fcc07e9..754f3b4c2218 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3993,14 +3993,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	if (ret < 0)
 		goto wash;
 
-	ret = device_register(&rdev->dev);
-	if (ret != 0) {
-		put_device(&rdev->dev);
-		goto wash;
-	}
-
-	dev_set_drvdata(&rdev->dev, rdev);
-
 	if (init_data && init_data->supply_regulator)
 		rdev->supply_name = init_data->supply_regulator;
 	else if (regulator_desc->supply_name)
@@ -4020,9 +4012,17 @@ regulator_register(const struct regulator_desc *regulator_desc,
 		}
 	}
 
-	rdev_init_debugfs(rdev);
 	mutex_unlock(&regulator_list_mutex);
 
+	ret = device_register(&rdev->dev);
+	if (ret != 0) {
+		put_device(&rdev->dev);
+		goto unset_supplies;
+	}
+
+	dev_set_drvdata(&rdev->dev, rdev);
+	rdev_init_debugfs(rdev);
+
 	/* try to resolve regulators supply since a new one was registered */
 	class_for_each_device(&regulator_class, NULL, NULL,
 			      regulator_register_resolve_supply);
@@ -4031,17 +4031,11 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 unset_supplies:
 	unset_regulator_supplies(rdev);
-	regulator_ena_gpio_free(rdev);
-	device_unregister(&rdev->dev);
-	/* device core frees rdev */
-	goto out;
-
 wash:
 	kfree(rdev->constraints);
 	regulator_ena_gpio_free(rdev);
 clean:
 	kfree(rdev);
-out:
 	mutex_unlock(&regulator_list_mutex);
 	kfree(config);
 	return ERR_PTR(ret);
-- 
2.1.4

[PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Jon Hunter]

The call to set_machine_constraints() in regulator_register(), will
attempt to get the voltage for the regulator.

A regulator that is in bypass will fail to be registered because we will
attempt to get the voltage of the regulator (ie. it's bypass voltage)
before the supply for the regulator has been resolved. Therefore, when
getting the voltage for a bypassed regulator, if the supply has not been
resolved, then attempt to resolve it. Additionally, move the setup of
the regulator's supply name to before the call to
set_machine_constraints() so that it can be resolved.

Please note that regulator_resolve_supply() will call
regulator_dev_lookup() which may acquire the regulator_list_mutex. To
avoid any deadlocks we cannot hold the regulator_list_mutex when calling
regulator_resolve_supply(). Following this change because
set_machine_constraints() may now result in a call to
regulator_resolve_supply() for a bypassed regulator, we can no longer
hold the regulator_list_mutex around this call. To avoid this rather
than holding the lock around a large portion of the registration code,
just hold the lock when aquiring any GPIOs and setting up supplies
because these sections may add entries to the regulator_map_list and
regulator_ena_gpio_list, respectively.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/regulator/core.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 754f3b4c2218..4f57a1832079 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3127,8 +3127,13 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)
 			return ret;
 		if (bypassed) {
 			/* if bypassed the regulator must have a supply */
-			if (!rdev->supply)
-				return -EINVAL;
+			if (!rdev->supply) {
+				ret = regulator_resolve_supply(rdev);
+				if (ret < 0)
+					return ret;
+				if (!rdev->supply)
+					return -EINVAL;
+			}
 
 			return _regulator_get_voltage(rdev->supply->rdev);
 		}
@@ -3945,8 +3950,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
 		rdev->dev.of_node = of_node_get(config->of_node);
 	}
 
-	mutex_lock(&regulator_list_mutex);
-
 	mutex_init(&rdev->mutex);
 	rdev->reg_data = config->driver_data;
 	rdev->owner = regulator_desc->owner;
@@ -3971,7 +3974,9 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 	if ((config->ena_gpio || config->ena_gpio_initialized) &&
 	    gpio_is_valid(config->ena_gpio)) {
+		mutex_lock(&regulator_list_mutex);
 		ret = regulator_ena_gpio_request(rdev, config);
+		mutex_unlock(&regulator_list_mutex);
 		if (ret != 0) {
 			rdev_err(rdev, "Failed to request enable GPIO%d: %d\n",
 				 config->ena_gpio, ret);
@@ -3989,31 +3994,32 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	if (init_data)
 		constraints = &init_data->constraints;
 
-	ret = set_machine_constraints(rdev, constraints);
-	if (ret < 0)
-		goto wash;
-
 	if (init_data && init_data->supply_regulator)
 		rdev->supply_name = init_data->supply_regulator;
 	else if (regulator_desc->supply_name)
 		rdev->supply_name = regulator_desc->supply_name;
 
+	ret = set_machine_constraints(rdev, constraints);
+	if (ret < 0)
+		goto wash;
+
 	/* add consumers devices */
 	if (init_data) {
+		mutex_lock(&regulator_list_mutex);
 		for (i = 0; i < init_data->num_consumer_supplies; i++) {
 			ret = set_consumer_device_supply(rdev,
 				init_data->consumer_supplies[i].dev_name,
 				init_data->consumer_supplies[i].supply);
 			if (ret < 0) {
+				mutex_unlock(&regulator_list_mutex);
 				dev_err(dev, "Failed to set supply %s\n",
 					init_data->consumer_supplies[i].supply);
 				goto unset_supplies;
 			}
 		}
+		mutex_unlock(&regulator_list_mutex);
 	}
 
-	mutex_unlock(&regulator_list_mutex);
-
 	ret = device_register(&rdev->dev);
 	if (ret != 0) {
 		put_device(&rdev->dev);
@@ -4030,13 +4036,16 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	return rdev;
 
 unset_supplies:
+	mutex_lock(&regulator_list_mutex);
 	unset_regulator_supplies(rdev);
+	mutex_unlock(&regulator_list_mutex);
 wash:
 	kfree(rdev->constraints);
+	mutex_lock(&regulator_list_mutex);
 	regulator_ena_gpio_free(rdev);
+	mutex_unlock(&regulator_list_mutex);
 clean:
 	kfree(rdev);
-	mutex_unlock(&regulator_list_mutex);
 	kfree(config);
 	return ERR_PTR(ret);
 }
-- 
2.1.4

[PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Jon Hunter]

The call to set_machine_constraints() in regulator_register(), will
attempt to get the voltage for the regulator.

A regulator that is in bypass will fail to be registered because we will
attempt to get the voltage of the regulator (ie. it's bypass voltage)
before the supply for the regulator has been resolved. Therefore, when
getting the voltage for a bypassed regulator, if the supply has not been
resolved, then attempt to resolve it. Additionally, move the setup of
the regulator's supply name to before the call to
set_machine_constraints() so that it can be resolved.

Please note that regulator_resolve_supply() will call
regulator_dev_lookup() which may acquire the regulator_list_mutex. To
avoid any deadlocks we cannot hold the regulator_list_mutex when calling
regulator_resolve_supply(). Following this change because
set_machine_constraints() may now result in a call to
regulator_resolve_supply() for a bypassed regulator, we can no longer
hold the regulator_list_mutex around this call. To avoid this rather
than holding the lock around a large portion of the registration code,
just hold the lock when aquiring any GPIOs and setting up supplies
because these sections may add entries to the regulator_map_list and
regulator_ena_gpio_list, respectively.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/regulator/core.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 754f3b4c2218..4f57a1832079 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3127,8 +3127,13 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)
 			return ret;
 		if (bypassed) {
 			/* if bypassed the regulator must have a supply */
-			if (!rdev->supply)
-				return -EINVAL;
+			if (!rdev->supply) {
+				ret = regulator_resolve_supply(rdev);
+				if (ret < 0)
+					return ret;
+				if (!rdev->supply)
+					return -EINVAL;
+			}
 
 			return _regulator_get_voltage(rdev->supply->rdev);
 		}
@@ -3945,8 +3950,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
 		rdev->dev.of_node = of_node_get(config->of_node);
 	}
 
-	mutex_lock(&regulator_list_mutex);
-
 	mutex_init(&rdev->mutex);
 	rdev->reg_data = config->driver_data;
 	rdev->owner = regulator_desc->owner;
@@ -3971,7 +3974,9 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 	if ((config->ena_gpio || config->ena_gpio_initialized) &&
 	    gpio_is_valid(config->ena_gpio)) {
+		mutex_lock(&regulator_list_mutex);
 		ret = regulator_ena_gpio_request(rdev, config);
+		mutex_unlock(&regulator_list_mutex);
 		if (ret != 0) {
 			rdev_err(rdev, "Failed to request enable GPIO%d: %d\n",
 				 config->ena_gpio, ret);
@@ -3989,31 +3994,32 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	if (init_data)
 		constraints = &init_data->constraints;
 
-	ret = set_machine_constraints(rdev, constraints);
-	if (ret < 0)
-		goto wash;
-
 	if (init_data && init_data->supply_regulator)
 		rdev->supply_name = init_data->supply_regulator;
 	else if (regulator_desc->supply_name)
 		rdev->supply_name = regulator_desc->supply_name;
 
+	ret = set_machine_constraints(rdev, constraints);
+	if (ret < 0)
+		goto wash;
+
 	/* add consumers devices */
 	if (init_data) {
+		mutex_lock(&regulator_list_mutex);
 		for (i = 0; i < init_data->num_consumer_supplies; i++) {
 			ret = set_consumer_device_supply(rdev,
 				init_data->consumer_supplies[i].dev_name,
 				init_data->consumer_supplies[i].supply);
 			if (ret < 0) {
+				mutex_unlock(&regulator_list_mutex);
 				dev_err(dev, "Failed to set supply %s\n",
 					init_data->consumer_supplies[i].supply);
 				goto unset_supplies;
 			}
 		}
+		mutex_unlock(&regulator_list_mutex);
 	}
 
-	mutex_unlock(&regulator_list_mutex);
-
 	ret = device_register(&rdev->dev);
 	if (ret != 0) {
 		put_device(&rdev->dev);
@@ -4030,13 +4036,16 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	return rdev;
 
 unset_supplies:
+	mutex_lock(&regulator_list_mutex);
 	unset_regulator_supplies(rdev);
+	mutex_unlock(&regulator_list_mutex);
 wash:
 	kfree(rdev->constraints);
+	mutex_lock(&regulator_list_mutex);
 	regulator_ena_gpio_free(rdev);
+	mutex_unlock(&regulator_list_mutex);
 clean:
 	kfree(rdev);
-	mutex_unlock(&regulator_list_mutex);
 	kfree(config);
 	return ERR_PTR(ret);
 }
-- 
2.1.4

[PATCH 5/5] regulator: helpers: Ensure bypass register field matches ON value

[Jon Hunter]

When checking bypass state for a regulator, we check to see if any bits
in the bypass mask are set. For most cases this is fine because there is
typically, only a single bit used to determine if the regulator is in
bypass. However, for some regulators, such as LDO6 on AS3722, the bypass
state is indicate by a value rather than a single bit. Therefore, when
checking the bypass state, check that the bypass field matches the ON
value.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/regulator/helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/helpers.c b/drivers/regulator/helpers.c
index b1e32e7482e9..bcf38fd5106a 100644
--- a/drivers/regulator/helpers.c
+++ b/drivers/regulator/helpers.c
@@ -460,7 +460,7 @@ int regulator_get_bypass_regmap(struct regulator_dev *rdev, bool *enable)
 	if (ret != 0)
 		return ret;
 
-	*enable = val & rdev->desc->bypass_mask;
+	*enable = (val & rdev->desc->bypass_mask) == rdev->desc->bypass_val_on;
 
 	return 0;
 }
-- 
2.1.4

[PATCH 5/5] regulator: helpers: Ensure bypass register field matches ON value

[Jon Hunter]

When checking bypass state for a regulator, we check to see if any bits
in the bypass mask are set. For most cases this is fine because there is
typically, only a single bit used to determine if the regulator is in
bypass. However, for some regulators, such as LDO6 on AS3722, the bypass
state is indicate by a value rather than a single bit. Therefore, when
checking the bypass state, check that the bypass field matches the ON
value.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/regulator/helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/helpers.c b/drivers/regulator/helpers.c
index b1e32e7482e9..bcf38fd5106a 100644
--- a/drivers/regulator/helpers.c
+++ b/drivers/regulator/helpers.c
@@ -460,7 +460,7 @@ int regulator_get_bypass_regmap(struct regulator_dev *rdev, bool *enable)
 	if (ret != 0)
 		return ret;
 
-	*enable = val & rdev->desc->bypass_mask;
+	*enable = (val & rdev->desc->bypass_mask) == rdev->desc->bypass_val_on;
 
 	return 0;
 }
-- 
2.1.4

Re: [PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Mark Brown]

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

On Thu, Apr 21, 2016 at 05:12:00PM +0100, Jon Hunter wrote:

> A regulator that is in bypass will fail to be registered because we will
> attempt to get the voltage of the regulator (ie. it's bypass voltage)
> before the supply for the regulator has been resolved. Therefore, when
> getting the voltage for a bypassed regulator, if the supply has not been
> resolved, then attempt to resolve it. Additionally, move the setup of
> the regulator's supply name to before the call to
> set_machine_constraints() so that it can be resolved.

The basic pattern here makes sense but rather than doing this
specifically in the case where we have a bypassed supply we didn't 
resolve yet I think we should instead always try to resolve the supply
but ignore the error unless we actively need the supply.  I'd be
surprised if we didn't run into other cases where we need to do this so
it seems better to try the resolution in one place.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Mark Brown]

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

On Thu, Apr 21, 2016 at 05:12:00PM +0100, Jon Hunter wrote:

> A regulator that is in bypass will fail to be registered because we will
> attempt to get the voltage of the regulator (ie. it's bypass voltage)
> before the supply for the regulator has been resolved. Therefore, when
> getting the voltage for a bypassed regulator, if the supply has not been
> resolved, then attempt to resolve it. Additionally, move the setup of
> the regulator's supply name to before the call to
> set_machine_constraints() so that it can be resolved.

The basic pattern here makes sense but rather than doing this
specifically in the case where we have a bypassed supply we didn't 
resolve yet I think we should instead always try to resolve the supply
but ignore the error unless we actively need the supply.  I'd be
surprised if we didn't run into other cases where we need to do this so
it seems better to try the resolution in one place.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Applied "regulator: core: Move registration of regulator device" to the regulator tree

[Mark Brown]

The patch

   regulator: core: Move registration of regulator device

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 c438b9d017362b65f6b1a9e54f7f35e7f873dc7c Mon Sep 17 00:00:00 2001
From: Jon Hunter <jonathanh@nvidia.com>
Date: Thu, 21 Apr 2016 17:11:59 +0100
Subject: [PATCH] regulator: core: Move registration of regulator device

The public functions to acquire a regulator, such as regulator_get(),
internally look-up the regulator from the list of regulators that have
been registered with the regulator device class. The registration of
a new regulator with the regulator device class happens before the
regulator has been completely setup. Therefore, it is possible that
the regulator could be acquired before it has been setup successfully.
To avoid this move the device registration of the regulator to the end
of the regulator setup and update the error exit path accordingly.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a17ce6cbbe77..8362a0a5105d 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3970,14 +3970,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	if (ret < 0)
 		goto wash;
 
-	ret = device_register(&rdev->dev);
-	if (ret != 0) {
-		put_device(&rdev->dev);
-		goto wash;
-	}
-
-	dev_set_drvdata(&rdev->dev, rdev);
-
 	if (init_data && init_data->supply_regulator)
 		rdev->supply_name = init_data->supply_regulator;
 	else if (regulator_desc->supply_name)
@@ -3997,9 +3989,17 @@ regulator_register(const struct regulator_desc *regulator_desc,
 		}
 	}
 
-	rdev_init_debugfs(rdev);
 	mutex_unlock(&regulator_list_mutex);
 
+	ret = device_register(&rdev->dev);
+	if (ret != 0) {
+		put_device(&rdev->dev);
+		goto unset_supplies;
+	}
+
+	dev_set_drvdata(&rdev->dev, rdev);
+	rdev_init_debugfs(rdev);
+
 	/* try to resolve regulators supply since a new one was registered */
 	class_for_each_device(&regulator_class, NULL, NULL,
 			      regulator_register_resolve_supply);
@@ -4008,17 +4008,11 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 unset_supplies:
 	unset_regulator_supplies(rdev);
-	regulator_ena_gpio_free(rdev);
-	device_unregister(&rdev->dev);
-	/* device core frees rdev */
-	goto out;
-
 wash:
 	kfree(rdev->constraints);
 	regulator_ena_gpio_free(rdev);
 clean:
 	kfree(rdev);
-out:
 	mutex_unlock(&regulator_list_mutex);
 	kfree(config);
 	return ERR_PTR(ret);
-- 
2.8.0.rc3

Applied "regulator: core: Move registration of regulator device" to the regulator tree

[Mark Brown]

The patch

   regulator: core: Move registration of regulator device

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 c438b9d017362b65f6b1a9e54f7f35e7f873dc7c Mon Sep 17 00:00:00 2001
From: Jon Hunter <jonathanh@nvidia.com>
Date: Thu, 21 Apr 2016 17:11:59 +0100
Subject: [PATCH] regulator: core: Move registration of regulator device

The public functions to acquire a regulator, such as regulator_get(),
internally look-up the regulator from the list of regulators that have
been registered with the regulator device class. The registration of
a new regulator with the regulator device class happens before the
regulator has been completely setup. Therefore, it is possible that
the regulator could be acquired before it has been setup successfully.
To avoid this move the device registration of the regulator to the end
of the regulator setup and update the error exit path accordingly.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index a17ce6cbbe77..8362a0a5105d 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3970,14 +3970,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	if (ret < 0)
 		goto wash;
 
-	ret = device_register(&rdev->dev);
-	if (ret != 0) {
-		put_device(&rdev->dev);
-		goto wash;
-	}
-
-	dev_set_drvdata(&rdev->dev, rdev);
-
 	if (init_data && init_data->supply_regulator)
 		rdev->supply_name = init_data->supply_regulator;
 	else if (regulator_desc->supply_name)
@@ -3997,9 +3989,17 @@ regulator_register(const struct regulator_desc *regulator_desc,
 		}
 	}
 
-	rdev_init_debugfs(rdev);
 	mutex_unlock(&regulator_list_mutex);
 
+	ret = device_register(&rdev->dev);
+	if (ret != 0) {
+		put_device(&rdev->dev);
+		goto unset_supplies;
+	}
+
+	dev_set_drvdata(&rdev->dev, rdev);
+	rdev_init_debugfs(rdev);
+
 	/* try to resolve regulators supply since a new one was registered */
 	class_for_each_device(&regulator_class, NULL, NULL,
 			      regulator_register_resolve_supply);
@@ -4008,17 +4008,11 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 unset_supplies:
 	unset_regulator_supplies(rdev);
-	regulator_ena_gpio_free(rdev);
-	device_unregister(&rdev->dev);
-	/* device core frees rdev */
-	goto out;
-
 wash:
 	kfree(rdev->constraints);
 	regulator_ena_gpio_free(rdev);
 clean:
 	kfree(rdev);
-out:
 	mutex_unlock(&regulator_list_mutex);
 	kfree(config);
 	return ERR_PTR(ret);
-- 
2.8.0.rc3

Applied "regulator: core: Clear the supply pointer if enabling fails" to the regulator tree

[Mark Brown]

The patch

   regulator: core: Clear the supply pointer if enabling fails

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 8e5356a73604f53da6a1e0756727cb8f9f7bba17 Mon Sep 17 00:00:00 2001
From: Jon Hunter <jonathanh@nvidia.com>
Date: Thu, 21 Apr 2016 17:11:58 +0100
Subject: [PATCH] regulator: core: Clear the supply pointer if enabling fails

During the resolution of a regulator's supply, we may attempt to enable
the supply if the regulator itself is already enabled. If enabling the
supply fails, then we will call _regulator_put() for the supply.
However, the pointer to the supply has not been cleared for the
regulator and this will cause a crash if we then unregister the
regulator and attempt to call regulator_put() a second time for the
supply. Fix this by clearing the supply pointer if enabling the supply
after fails when resolving the supply for a regulator.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 9922922ce6bd..bd9ec309b707 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1536,6 +1536,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 		ret = regulator_enable(rdev->supply);
 		if (ret < 0) {
 			_regulator_put(rdev->supply);
+			rdev->supply = NULL;
 			return ret;
 		}
 	}
-- 
2.8.0.rc3

Applied "regulator: core: Clear the supply pointer if enabling fails" to the regulator tree

[Mark Brown]

The patch

   regulator: core: Clear the supply pointer if enabling fails

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 8e5356a73604f53da6a1e0756727cb8f9f7bba17 Mon Sep 17 00:00:00 2001
From: Jon Hunter <jonathanh@nvidia.com>
Date: Thu, 21 Apr 2016 17:11:58 +0100
Subject: [PATCH] regulator: core: Clear the supply pointer if enabling fails

During the resolution of a regulator's supply, we may attempt to enable
the supply if the regulator itself is already enabled. If enabling the
supply fails, then we will call _regulator_put() for the supply.
However, the pointer to the supply has not been cleared for the
regulator and this will cause a crash if we then unregister the
regulator and attempt to call regulator_put() a second time for the
supply. Fix this by clearing the supply pointer if enabling the supply
after fails when resolving the supply for a regulator.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 9922922ce6bd..bd9ec309b707 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1536,6 +1536,7 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 		ret = regulator_enable(rdev->supply);
 		if (ret < 0) {
 			_regulator_put(rdev->supply);
+			rdev->supply = NULL;
 			return ret;
 		}
 	}
-- 
2.8.0.rc3

Applied "regulator: core: Don't terminate supply resolution early" to the regulator tree

[Mark Brown]

The patch

   regulator: core: Don't terminate supply resolution early

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 7ddede6a58a0bd26efcfd2a5055611195411f514 Mon Sep 17 00:00:00 2001
From: Jon Hunter <jonathanh@nvidia.com>
Date: Thu, 21 Apr 2016 17:11:57 +0100
Subject: [PATCH] regulator: core: Don't terminate supply resolution early

The function regulator_register_resolve_supply() is called from the
context of class_for_each_dev() (during the regulator registration) to
resolve any supplies added. regulator_register_resolve_supply() will
return an error if a regulator's supply cannot be resolved and this will
terminate the loop in class_for_each_dev(). This means that we will not
attempt to resolve any other supplies after one has failed. Hence, this
may delay the resolution of other regulator supplies until the failing
one itself can be resolved.

Rather than terminating the loop early, don't return an error code and
keep attempting to resolve any other supplies for regulators that have
been registered.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index fd0e4e37f4e1..9922922ce6bd 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3842,7 +3842,12 @@ static void rdev_init_debugfs(struct regulator_dev *rdev)
 
 static int regulator_register_resolve_supply(struct device *dev, void *data)
 {
-	return regulator_resolve_supply(dev_to_rdev(dev));
+	struct regulator_dev *rdev = dev_to_rdev(dev);
+
+	if (regulator_resolve_supply(rdev))
+		rdev_dbg(rdev, "unable to resolve supply\n");
+
+	return 0;
 }
 
 /**
-- 
2.8.0.rc3

Applied "regulator: core: Don't terminate supply resolution early" to the regulator tree

[Mark Brown]

The patch

   regulator: core: Don't terminate supply resolution early

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 7ddede6a58a0bd26efcfd2a5055611195411f514 Mon Sep 17 00:00:00 2001
From: Jon Hunter <jonathanh@nvidia.com>
Date: Thu, 21 Apr 2016 17:11:57 +0100
Subject: [PATCH] regulator: core: Don't terminate supply resolution early

The function regulator_register_resolve_supply() is called from the
context of class_for_each_dev() (during the regulator registration) to
resolve any supplies added. regulator_register_resolve_supply() will
return an error if a regulator's supply cannot be resolved and this will
terminate the loop in class_for_each_dev(). This means that we will not
attempt to resolve any other supplies after one has failed. Hence, this
may delay the resolution of other regulator supplies until the failing
one itself can be resolved.

Rather than terminating the loop early, don't return an error code and
keep attempting to resolve any other supplies for regulators that have
been registered.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index fd0e4e37f4e1..9922922ce6bd 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3842,7 +3842,12 @@ static void rdev_init_debugfs(struct regulator_dev *rdev)
 
 static int regulator_register_resolve_supply(struct device *dev, void *data)
 {
-	return regulator_resolve_supply(dev_to_rdev(dev));
+	struct regulator_dev *rdev = dev_to_rdev(dev);
+
+	if (regulator_resolve_supply(rdev))
+		rdev_dbg(rdev, "unable to resolve supply\n");
+
+	return 0;
 }
 
 /**
-- 
2.8.0.rc3

Re: [PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Jon Hunter]

On 22/04/16 11:48, Mark Brown wrote:
> * PGP Signed by an unknown key
> 
> On Thu, Apr 21, 2016 at 05:12:00PM +0100, Jon Hunter wrote:
> 
>> A regulator that is in bypass will fail to be registered because we will
>> attempt to get the voltage of the regulator (ie. it's bypass voltage)
>> before the supply for the regulator has been resolved. Therefore, when
>> getting the voltage for a bypassed regulator, if the supply has not been
>> resolved, then attempt to resolve it. Additionally, move the setup of
>> the regulator's supply name to before the call to
>> set_machine_constraints() so that it can be resolved.
> 
> The basic pattern here makes sense but rather than doing this
> specifically in the case where we have a bypassed supply we didn't 
> resolve yet I think we should instead always try to resolve the supply
> but ignore the error unless we actively need the supply.  I'd be
> surprised if we didn't run into other cases where we need to do this so
> it seems better to try the resolution in one place.

OK. Sorry if I have misunderstood you here, but this sounds more like
Thierry's initial proposal [0] but ignoring the any errors returned (and
we need to fix-up the locking in this patch). In the discussion that
followed I thought we agreed to only do this for the bypass case [1]. As
far as I am concerned either will work, but to confirm we should just
always try to resolve the supply early during regulator_register(), correct?

Cheers
Jon

[0] http://marc.info/?t=146003907800001&r=1&w=2
[1] http://marc.info/?l=linux-kernel&m=146038421710211&w=2	

Re: [PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Jon Hunter]

On 22/04/16 11:48, Mark Brown wrote:
> * PGP Signed by an unknown key
> 
> On Thu, Apr 21, 2016 at 05:12:00PM +0100, Jon Hunter wrote:
> 
>> A regulator that is in bypass will fail to be registered because we will
>> attempt to get the voltage of the regulator (ie. it's bypass voltage)
>> before the supply for the regulator has been resolved. Therefore, when
>> getting the voltage for a bypassed regulator, if the supply has not been
>> resolved, then attempt to resolve it. Additionally, move the setup of
>> the regulator's supply name to before the call to
>> set_machine_constraints() so that it can be resolved.
> 
> The basic pattern here makes sense but rather than doing this
> specifically in the case where we have a bypassed supply we didn't 
> resolve yet I think we should instead always try to resolve the supply
> but ignore the error unless we actively need the supply.  I'd be
> surprised if we didn't run into other cases where we need to do this so
> it seems better to try the resolution in one place.

OK. Sorry if I have misunderstood you here, but this sounds more like
Thierry's initial proposal [0] but ignoring the any errors returned (and
we need to fix-up the locking in this patch). In the discussion that
followed I thought we agreed to only do this for the bypass case [1]. As
far as I am concerned either will work, but to confirm we should just
always try to resolve the supply early during regulator_register(), correct?

Cheers
Jon

[0] http://marc.info/?t=146003907800001&r=1&w=2
[1] http://marc.info/?l=linux-kernel&m=146038421710211&w=2	

Re: [PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Mark Brown]

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

On Fri, Apr 22, 2016 at 12:26:57PM +0100, Jon Hunter wrote:

> OK. Sorry if I have misunderstood you here, but this sounds more like
> Thierry's initial proposal [0] but ignoring the any errors returned (and
> we need to fix-up the locking in this patch). In the discussion that

Yes!

> followed I thought we agreed to only do this for the bypass case [1]. As
> far as I am concerned either will work, but to confirm we should just
> always try to resolve the supply early during regulator_register(), correct?

We need to only *fail* in the bypass case.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Mark Brown]

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

On Fri, Apr 22, 2016 at 12:26:57PM +0100, Jon Hunter wrote:

> OK. Sorry if I have misunderstood you here, but this sounds more like
> Thierry's initial proposal [0] but ignoring the any errors returned (and
> we need to fix-up the locking in this patch). In the discussion that

Yes!

> followed I thought we agreed to only do this for the bypass case [1]. As
> far as I am concerned either will work, but to confirm we should just
> always try to resolve the supply early during regulator_register(), correct?

We need to only *fail* in the bypass case.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Jon Hunter]

On 22/04/16 14:53, Mark Brown wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Apr 22, 2016 at 12:26:57PM +0100, Jon Hunter wrote:
> 
>> OK. Sorry if I have misunderstood you here, but this sounds more like
>> Thierry's initial proposal [0] but ignoring the any errors returned (and
>> we need to fix-up the locking in this patch). In the discussion that
> 
> Yes!
> 
>> followed I thought we agreed to only do this for the bypass case [1]. As
>> far as I am concerned either will work, but to confirm we should just
>> always try to resolve the supply early during regulator_register(), correct?
> 
> We need to only *fail* in the bypass case.

OK. So this is what I have now. Is it weird to return EPROBE_DEFER in 
_regulator_get_voltage()? If so, I could add a test for bypass in the
regulator_register().

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 5b46d907e61d..7a6b7f667bcb 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3126,7 +3126,7 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)
 		if (bypassed) {
 			/* if bypassed the regulator must have a supply */
 			if (!rdev->supply)
-				return -EINVAL;
+				return -EPROBE_DEFER;
 
 			return _regulator_get_voltage(rdev->supply->rdev);
 		}
@@ -3943,8 +3943,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
 		rdev->dev.of_node = of_node_get(config->of_node);
 	}
 
-	mutex_lock(&regulator_list_mutex);
-
 	mutex_init(&rdev->mutex);
 	rdev->reg_data = config->driver_data;
 	rdev->owner = regulator_desc->owner;
@@ -3969,7 +3967,9 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 	if ((config->ena_gpio || config->ena_gpio_initialized) &&
 	    gpio_is_valid(config->ena_gpio)) {
+		mutex_lock(&regulator_list_mutex);
 		ret = regulator_ena_gpio_request(rdev, config);
+		mutex_unlock(&regulator_list_mutex);
 		if (ret != 0) {
 			rdev_err(rdev, "Failed to request enable GPIO%d: %d\n",
 				 config->ena_gpio, ret);
@@ -3987,31 +3987,40 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	if (init_data)
 		constraints = &init_data->constraints;
 
-	ret = set_machine_constraints(rdev, constraints);
-	if (ret < 0)
-		goto wash;
-
 	if (init_data && init_data->supply_regulator)
 		rdev->supply_name = init_data->supply_regulator;
 	else if (regulator_desc->supply_name)
 		rdev->supply_name = regulator_desc->supply_name;
 
+	/*
+	 * Attempt to resolve the regulator supply, if specified,
+	 * but don't return an error if we fail because we will try
+	 * to resolve it again later as more regulators are added.
+	 */
+	if (regulator_resolve_supply(rdev))
+		rdev_dbg(rdev, "unable to resolve supply\n");
+
+	ret = set_machine_constraints(rdev, constraints);
+	if (ret < 0)
+		goto wash;
+
 	/* add consumers devices */
 	if (init_data) {
+		mutex_lock(&regulator_list_mutex);
 		for (i = 0; i < init_data->num_consumer_supplies; i++) {
 			ret = set_consumer_device_supply(rdev,
 				init_data->consumer_supplies[i].dev_name,
 				init_data->consumer_supplies[i].supply);
 			if (ret < 0) {
+				mutex_unlock(&regulator_list_mutex);
 				dev_err(dev, "Failed to set supply %s\n",
 					init_data->consumer_supplies[i].supply);
 				goto unset_supplies;
 			}
 		}
+		mutex_unlock(&regulator_list_mutex);
 	}
 
-	mutex_unlock(&regulator_list_mutex);
-
 	ret = device_register(&rdev->dev);
 	if (ret != 0) {
 		put_device(&rdev->dev);
@@ -4028,13 +4037,16 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	return rdev;
 
 unset_supplies:
+	mutex_lock(&regulator_list_mutex);
 	unset_regulator_supplies(rdev);
+	mutex_unlock(&regulator_list_mutex);
 wash:
 	kfree(rdev->constraints);
+	mutex_lock(&regulator_list_mutex);
 	regulator_ena_gpio_free(rdev);
+	mutex_unlock(&regulator_list_mutex);
 clean:
 	kfree(rdev);
-	mutex_unlock(&regulator_list_mutex);
 	kfree(config);
 	return ERR_PTR(ret);
 }
-- 
2.1.4

Re: [PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Jon Hunter]

On 22/04/16 14:53, Mark Brown wrote:
> * PGP Signed by an unknown key
> 
> On Fri, Apr 22, 2016 at 12:26:57PM +0100, Jon Hunter wrote:
> 
>> OK. Sorry if I have misunderstood you here, but this sounds more like
>> Thierry's initial proposal [0] but ignoring the any errors returned (and
>> we need to fix-up the locking in this patch). In the discussion that
> 
> Yes!
> 
>> followed I thought we agreed to only do this for the bypass case [1]. As
>> far as I am concerned either will work, but to confirm we should just
>> always try to resolve the supply early during regulator_register(), correct?
> 
> We need to only *fail* in the bypass case.

OK. So this is what I have now. Is it weird to return EPROBE_DEFER in 
_regulator_get_voltage()? If so, I could add a test for bypass in the
regulator_register().

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 5b46d907e61d..7a6b7f667bcb 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3126,7 +3126,7 @@ static int _regulator_get_voltage(struct regulator_dev *rdev)
 		if (bypassed) {
 			/* if bypassed the regulator must have a supply */
 			if (!rdev->supply)
-				return -EINVAL;
+				return -EPROBE_DEFER;
 
 			return _regulator_get_voltage(rdev->supply->rdev);
 		}
@@ -3943,8 +3943,6 @@ regulator_register(const struct regulator_desc *regulator_desc,
 		rdev->dev.of_node = of_node_get(config->of_node);
 	}
 
-	mutex_lock(&regulator_list_mutex);
-
 	mutex_init(&rdev->mutex);
 	rdev->reg_data = config->driver_data;
 	rdev->owner = regulator_desc->owner;
@@ -3969,7 +3967,9 @@ regulator_register(const struct regulator_desc *regulator_desc,
 
 	if ((config->ena_gpio || config->ena_gpio_initialized) &&
 	    gpio_is_valid(config->ena_gpio)) {
+		mutex_lock(&regulator_list_mutex);
 		ret = regulator_ena_gpio_request(rdev, config);
+		mutex_unlock(&regulator_list_mutex);
 		if (ret != 0) {
 			rdev_err(rdev, "Failed to request enable GPIO%d: %d\n",
 				 config->ena_gpio, ret);
@@ -3987,31 +3987,40 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	if (init_data)
 		constraints = &init_data->constraints;
 
-	ret = set_machine_constraints(rdev, constraints);
-	if (ret < 0)
-		goto wash;
-
 	if (init_data && init_data->supply_regulator)
 		rdev->supply_name = init_data->supply_regulator;
 	else if (regulator_desc->supply_name)
 		rdev->supply_name = regulator_desc->supply_name;
 
+	/*
+	 * Attempt to resolve the regulator supply, if specified,
+	 * but don't return an error if we fail because we will try
+	 * to resolve it again later as more regulators are added.
+	 */
+	if (regulator_resolve_supply(rdev))
+		rdev_dbg(rdev, "unable to resolve supply\n");
+
+	ret = set_machine_constraints(rdev, constraints);
+	if (ret < 0)
+		goto wash;
+
 	/* add consumers devices */
 	if (init_data) {
+		mutex_lock(&regulator_list_mutex);
 		for (i = 0; i < init_data->num_consumer_supplies; i++) {
 			ret = set_consumer_device_supply(rdev,
 				init_data->consumer_supplies[i].dev_name,
 				init_data->consumer_supplies[i].supply);
 			if (ret < 0) {
+				mutex_unlock(&regulator_list_mutex);
 				dev_err(dev, "Failed to set supply %s\n",
 					init_data->consumer_supplies[i].supply);
 				goto unset_supplies;
 			}
 		}
+		mutex_unlock(&regulator_list_mutex);
 	}
 
-	mutex_unlock(&regulator_list_mutex);
-
 	ret = device_register(&rdev->dev);
 	if (ret != 0) {
 		put_device(&rdev->dev);
@@ -4028,13 +4037,16 @@ regulator_register(const struct regulator_desc *regulator_desc,
 	return rdev;
 
 unset_supplies:
+	mutex_lock(&regulator_list_mutex);
 	unset_regulator_supplies(rdev);
+	mutex_unlock(&regulator_list_mutex);
 wash:
 	kfree(rdev->constraints);
+	mutex_lock(&regulator_list_mutex);
 	regulator_ena_gpio_free(rdev);
+	mutex_unlock(&regulator_list_mutex);
 clean:
 	kfree(rdev);
-	mutex_unlock(&regulator_list_mutex);
 	kfree(config);
 	return ERR_PTR(ret);
 }
-- 
2.1.4

Re: [PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Mark Brown]

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

On Mon, Apr 25, 2016 at 03:44:24PM +0100, Jon Hunter wrote:
> On 22/04/16 14:53, Mark Brown wrote:

> > We need to only *fail* in the bypass case.

> OK. So this is what I have now. Is it weird to return EPROBE_DEFER in 
> _regulator_get_voltage()? If so, I could add a test for bypass in the
> regulator_register().

It should be fine but perhaps print an error saying that it's a bypassed
regulator with no supply?  Deferring for no explicit reason can be
confusing.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 4/5] regulator: core: Add early supply resolution for a bypassed regulator

[Mark Brown]

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

On Mon, Apr 25, 2016 at 03:44:24PM +0100, Jon Hunter wrote:
> On 22/04/16 14:53, Mark Brown wrote:

> > We need to only *fail* in the bypass case.

> OK. So this is what I have now. Is it weird to return EPROBE_DEFER in 
> _regulator_get_voltage()? If so, I could add a test for bypass in the
> regulator_register().

It should be fine but perhaps print an error saying that it's a bypassed
regulator with no supply?  Deferring for no explicit reason can be
confusing.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]