From: Mark Brown <[email protected]>
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 <[email protected]>
---
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);
--
1.8.5.3
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 <[email protected]>
>
> 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 <[email protected]>
> ---
> 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);
--
Jean Delvare
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...
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,
--
Jean Delvare
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.