From: Mark Brown broonie@linaro.org
Once we have full constraints then all supply mappings should be known to the regulator API. This means that we should treat failed lookups as fatal rather than deferring in the hope of further registrations but this was broken by commit 9b92da1f1205bd25 "regulator: core: Fix default return value for _get()" which was targeted at DT systems but unintentionally broke non-DT systems by changing the default return value.
Fix this by explicitly returning -EPROBE_DEFER from the DT lookup if we find a property but no corresponding regulator and by having the non-DT case default to -ENODEV when we have full constraints.
Fixes: 9b92da1f1205bd25 "regulator: core: Fix default return value for _get()" Signed-off-by: Mark Brown broonie@linaro.org --- drivers/regulator/core.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index b38a6b669e8c..16a309e5c024 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -1272,6 +1272,8 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev, if (r->dev.parent && node == r->dev.of_node) return r; + *ret = -EPROBE_DEFER; + return NULL; } else { /* * If we couldn't even get the node then it's @@ -1312,7 +1314,7 @@ static struct regulator *_regulator_get(struct device *dev, const char *id, struct regulator_dev *rdev; struct regulator *regulator = ERR_PTR(-EPROBE_DEFER); const char *devname = NULL; - int ret = -EPROBE_DEFER; + int ret;
if (id == NULL) { pr_err("get() with no identifier\n"); @@ -1322,6 +1324,11 @@ static struct regulator *_regulator_get(struct device *dev, const char *id, if (dev) devname = dev_name(dev);
+ if (have_full_constraints()) + ret = -ENODEV; + else + ret = -EPROBE_DEFER; + mutex_lock(®ulator_list_mutex);
rdev = regulator_dev_lookup(dev, id, &ret);
Liam,
Can you please review / apply this patch? It fixes a real bug. I also think it should go to the 3.13-stable tree.
Thanks, Jean
On Mon, 27 Jan 2014 18:07:55 +0000, Mark Brown wrote:
From: Mark Brown broonie@linaro.org
Once we have full constraints then all supply mappings should be known to the regulator API. This means that we should treat failed lookups as fatal rather than deferring in the hope of further registrations but this was broken by commit 9b92da1f1205bd25 "regulator: core: Fix default return value for _get()" which was targeted at DT systems but unintentionally broke non-DT systems by changing the default return value.
Fix this by explicitly returning -EPROBE_DEFER from the DT lookup if we find a property but no corresponding regulator and by having the non-DT case default to -ENODEV when we have full constraints.
Fixes: 9b92da1f1205bd25 "regulator: core: Fix default return value for _get()" Signed-off-by: Mark Brown broonie@linaro.org
drivers/regulator/core.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index b38a6b669e8c..16a309e5c024 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -1272,6 +1272,8 @@ static struct regulator_dev *regulator_dev_lookup(struct device *dev, if (r->dev.parent && node == r->dev.of_node) return r;
*ret = -EPROBE_DEFER;return NULL;@@ -1312,7 +1314,7 @@ static struct regulator *_regulator_get(struct device *dev, const char *id, struct regulator_dev *rdev; struct regulator *regulator = ERR_PTR(-EPROBE_DEFER); const char *devname = NULL;
- int ret = -EPROBE_DEFER;
- int ret;
if (id == NULL) { pr_err("get() with no identifier\n"); @@ -1322,6 +1324,11 @@ static struct regulator *_regulator_get(struct device *dev, const char *id, if (dev) devname = dev_name(dev);
- if (have_full_constraints())
ret = -ENODEV;- else
ret = -EPROBE_DEFER;- mutex_lock(®ulator_list_mutex);
rdev = regulator_dev_lookup(dev, id, &ret);
Hi Mark,
On Mon, 3 Feb 2014 11:02:49 +0000, Mark Brown wrote:
On Mon, Feb 03, 2014 at 10:41:41AM +0100, Jean Delvare wrote:
Can you please review / apply this patch? It fixes a real bug. I also think it should go to the 3.13-stable tree.
Uh, it is applied? I'm the one who maintains the regulator tree generally...
Ah, I see it in your tree. But it's not in 3.14-rc1. As this fixes a regression, it would be great to have it upstream ASAP, so that it can propagate to 3.13-stable.
Thanks,
On Mon, Feb 03, 2014 at 03:35:12PM +0100, Jean Delvare wrote:
Ah, I see it in your tree. But it's not in 3.14-rc1. As this fixes a regression, it would be great to have it upstream ASAP, so that it can propagate to 3.13-stable.
It's on a fixes tag so it'll go in next time I send stuff to Linus, it'll go next time I send something to him. That's usually about once per release. Especially if things are tagged for stable I like to try to avoid sending them too quickly unless they're build breaks or similar.
On Mon, Feb 03, 2014 at 03:41:09PM +0000, Mark Brown wrote:
On Mon, Feb 03, 2014 at 03:35:12PM +0100, Jean Delvare wrote:
Ah, I see it in your tree. But it's not in 3.14-rc1. As this fixes a regression, it would be great to have it upstream ASAP, so that it can propagate to 3.13-stable.
It's on a fixes tag so it'll go in next time I send stuff to Linus, it'll go next time I send something to him. That's usually about once per release. Especially if things are tagged for stable I like to try to avoid sending them too quickly unless they're build breaks or similar.
We still have the lm90 driver failing in 13.3 for non-DT platforms. One should think that this has some level of importance. If a driver doesn't build or doesn't work makes little difference for the user, after all.
Guenter
On Mon, Feb 03, 2014 at 08:52:10AM -0800, Guenter Roeck wrote:
On Mon, Feb 03, 2014 at 03:41:09PM +0000, Mark Brown wrote:
It's on a fixes tag so it'll go in next time I send stuff to Linus, it'll go next time I send something to him. That's usually about once per release. Especially if things are tagged for stable I like to try to avoid sending them too quickly unless they're build breaks or similar.
We still have the lm90 driver failing in 13.3 for non-DT platforms. One should think that this has some level of importance. If a driver doesn't build or doesn't work makes little difference for the user, after all.
On the other hand it's a hardware monitoring driver which nobody managed to notice problems with for pretty much an entire release cycle and a fix in the core regulator lookup code which has the ability to break boot prior to the console coming up if it goes wrong. An extra week is not going to be the end of the world here but helps ensure better exposure of what hits stable.
linaro-kernel@lists.linaro.org
-
Guenter Roeck -
Jean Delvare -
Mark Brown