linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] regulator: core: Correct default return value for full constraints
@ 2014-01-27 18:07 Mark Brown
  2014-02-03  9:41 ` Jean Delvare
  0 siblings, 1 reply; 7+ messages in thread
From: Mark Brown @ 2014-01-27 18:07 UTC (permalink / raw)
  To: Liam Girdwood
  Cc: linux-kernel, linaro-kernel, Jean Delvare, Guenter Roeck, Mark Brown

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(&regulator_list_mutex);
 
 	rdev = regulator_dev_lookup(dev, id, &ret);
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] regulator: core: Correct default return value for full constraints
  2014-01-27 18:07 [PATCH] regulator: core: Correct default return value for full constraints Mark Brown
@ 2014-02-03  9:41 ` Jean Delvare
  2014-02-03 11:02   ` Mark Brown
  0 siblings, 1 reply; 7+ messages in thread
From: Jean Delvare @ 2014-02-03  9:41 UTC (permalink / raw)
  To: Mark Brown
  Cc: Liam Girdwood, linux-kernel, linaro-kernel, Guenter Roeck, Mark Brown

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;
>  		} 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(&regulator_list_mutex);
>  
>  	rdev = regulator_dev_lookup(dev, id, &ret);


-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] regulator: core: Correct default return value for full constraints
  2014-02-03  9:41 ` Jean Delvare
@ 2014-02-03 11:02   ` Mark Brown
  2014-02-03 14:35     ` Jean Delvare
  0 siblings, 1 reply; 7+ messages in thread
From: Mark Brown @ 2014-02-03 11:02 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Liam Girdwood, linux-kernel, linaro-kernel, Guenter Roeck

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

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...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] regulator: core: Correct default return value for full constraints
  2014-02-03 11:02   ` Mark Brown
@ 2014-02-03 14:35     ` Jean Delvare
  2014-02-03 15:41       ` Mark Brown
  0 siblings, 1 reply; 7+ messages in thread
From: Jean Delvare @ 2014-02-03 14:35 UTC (permalink / raw)
  To: Mark Brown; +Cc: Liam Girdwood, linux-kernel, linaro-kernel, Guenter Roeck

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] regulator: core: Correct default return value for full constraints
  2014-02-03 14:35     ` Jean Delvare
@ 2014-02-03 15:41       ` Mark Brown
  2014-02-03 16:52         ` Guenter Roeck
  0 siblings, 1 reply; 7+ messages in thread
From: Mark Brown @ 2014-02-03 15:41 UTC (permalink / raw)
  To: Jean Delvare; +Cc: Liam Girdwood, linux-kernel, linaro-kernel, Guenter Roeck

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

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] regulator: core: Correct default return value for full constraints
  2014-02-03 15:41       ` Mark Brown
@ 2014-02-03 16:52         ` Guenter Roeck
  2014-02-03 17:31           ` Mark Brown
  0 siblings, 1 reply; 7+ messages in thread
From: Guenter Roeck @ 2014-02-03 16:52 UTC (permalink / raw)
  To: Mark Brown; +Cc: Jean Delvare, Liam Girdwood, linux-kernel, linaro-kernel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] regulator: core: Correct default return value for full constraints
  2014-02-03 16:52         ` Guenter Roeck
@ 2014-02-03 17:31           ` Mark Brown
  0 siblings, 0 replies; 7+ messages in thread
From: Mark Brown @ 2014-02-03 17:31 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Jean Delvare, Liam Girdwood, linux-kernel, linaro-kernel

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

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.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-02-03 17:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-27 18:07 [PATCH] regulator: core: Correct default return value for full constraints Mark Brown
2014-02-03  9:41 ` Jean Delvare
2014-02-03 11:02   ` Mark Brown
2014-02-03 14:35     ` Jean Delvare
2014-02-03 15:41       ` Mark Brown
2014-02-03 16:52         ` Guenter Roeck
2014-02-03 17:31           ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).