[PATCH 1/2] drivers/regulator/core.c: Don't print error on EPROBE_DEFER

[PATCH 1/2] drivers/regulator/core.c: Don't print error on EPROBE_DEFER

[Mike Looijmans]

If a regulator depends on another regulator that happens to be called
later, the kernel always prints a message like this:
  reg-fixed-voltage regulator_sd1: Failed to find supply vin
Since the deferral is not something fatal, nor even something the user
may need to be aware about, reduce the message to debug level.

This fixes a storm of error messages at boot when a board has a power
regulator on an I2C bus which powers GPIO controlled regulators for
example.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
 drivers/regulator/core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e225711..9fb66cd 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3715,7 +3715,7 @@ regulator_register(const struct regulator_desc *regulator_desc,
 			ret = 0;
 			goto add_dev;
 		} else if (!r) {
-			dev_err(dev, "Failed to find supply %s\n", supply);
+			dev_dbg(dev, "Failed to find supply %s\n", supply);
 			ret = -EPROBE_DEFER;
 			goto scrub;
 		}
-- 
1.7.9.5

[PATCH 2/2] regulator/fixed.c: Don't report EPROBE_DEFER errors

[Mike Looijmans]

If the regulator cannot be registered because its supplier is not
available yet, don't write an error. There is no need to alert the
user of probe deferrals.
This fixes a storm of error messages at boot when a GPIO controlled
regulator is supplied by an I2C controlled supply.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
 drivers/regulator/fixed.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index d21da27..ac7c874 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -189,7 +189,9 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
 					       &cfg);
 	if (IS_ERR(drvdata->dev)) {
 		ret = PTR_ERR(drvdata->dev);
-		dev_err(&pdev->dev, "Failed to register regulator: %d\n", ret);
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
+				"Failed to register regulator: %d\n", ret);
 		return ret;
 	}
 
-- 
1.7.9.5

Re: [PATCH 1/2] drivers/regulator/core.c: Don't print error on EPROBE_DEFER

[Mark Brown]

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

On Tue, Dec 09, 2014 at 02:12:43PM +0100, Mike Looijmans wrote:
> If a regulator depends on another regulator that happens to be called
> later, the kernel always prints a message like this:
>   reg-fixed-voltage regulator_sd1: Failed to find supply vin
> Since the deferral is not something fatal, nor even something the user
> may need to be aware about, reduce the message to debug level.

No, we need to tell users if we're deferring since there's no guarantee
that the device blocking registration is ever going to be loaded - we
need to let the user know why the device isn't appearing so that they
can diagnose problems.  Yes, it's noisy and ideally we'd have something
more efficient than deferred probe but there we are.

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

Re: [PATCH 1/2] drivers/regulator/core.c: Don't print error on EPROBE_DEFER

[Mike Looijmans]

On 12/09/2014 05:14 PM, Mark Brown wrote:
>>If a regulator depends on another regulator that happens to be called
>>later, the kernel always prints a message like this:
>>   reg-fixed-voltage regulator_sd1: Failed to find supply vin
>>Since the deferral is not something fatal, nor even something the user
>>may need to be aware about, reduce the message to debug level.

Can we instead at least reduce it to WARN or INFO level then?

I have to explain over and over again that there's no problem when that 
message comes along ten times in a row. And it causes people to overlook 
the messages that really are errors.

-- 
Mike Looijmans

Re: [PATCH 1/2] drivers/regulator/core.c: Don't print error on EPROBE_DEFER

[Mark Brown]

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

On Tue, Dec 09, 2014 at 07:12:30PM +0100, Mike Looijmans wrote:
> On 12/09/2014 05:14 PM, Mark Brown wrote:

> >>If a regulator depends on another regulator that happens to be called
> >>later, the kernel always prints a message like this:
> >>  reg-fixed-voltage regulator_sd1: Failed to find supply vin
> >>Since the deferral is not something fatal, nor even something the user
> >>may need to be aware about, reduce the message to debug level.

> Can we instead at least reduce it to WARN or INFO level then?

You appear to have deleted my reply here...  one problem with your
suggestion is that it means we have to special case all error handling
on probe for deferral which isn't wonderful.

> I have to explain over and over again that there's no problem when that
> message comes along ten times in a row. And it causes people to overlook the
> messages that really are errors.

Can we do something with the log message that triggers on probe
deferral?  There tends to be a learning curve with probe deferral but
the fact that it's generally extremely noisy tends to be useful - I
usually point people at that (not just in the context at regulators) and
tell them not to worry unless debugging.

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

Re: [PATCH 1/2] drivers/regulator/core.c: Don't print error on EPROBE_DEFER

[Joe Perches]

On Tue, 2014-12-09 at 18:48 +0000, Mark Brown wrote:
> Can we do something with the log message that triggers on probe
> deferral?

Maybe add and use rdev_err_ratelimited?

Re: [PATCH 1/2] drivers/regulator/core.c: Don't print error on EPROBE_DEFER

[Mark Brown]

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

On Tue, Dec 09, 2014 at 11:14:31AM -0800, Joe Perches wrote:
> On Tue, 2014-12-09 at 18:48 +0000, Mark Brown wrote:

> > Can we do something with the log message that triggers on probe
> > deferral?

> Maybe add and use rdev_err_ratelimited?

That's not going to help, the volume isn't sufficiently high and comes
from many different devices.

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

Re: [PATCH 1/2] drivers/regulator/core.c: Don't print error on EPROBE_DEFER

[Mike Looijmans]

On 12/09/2014 07:48 PM, Mark Brown wrote:
> On Tue, Dec 09, 2014 at 07:12:30PM +0100, Mike Looijmans wrote:
>> On 12/09/2014 05:14 PM, Mark Brown wrote:
>
>>>> If a regulator depends on another regulator that happens to be called
>>>> later, the kernel always prints a message like this:
>>>>   reg-fixed-voltage regulator_sd1: Failed to find supply vin
>>>> Since the deferral is not something fatal, nor even something the user
>>>> may need to be aware about, reduce the message to debug level.
>
>> Can we instead at least reduce it to WARN or INFO level then?
>
> You appear to have deleted my reply here...  one problem with your
> suggestion is that it means we have to special case all error handling
> on probe for deferral which isn't wonderful.

(Sorry for deleting your reply. It wasn't intentional though.)

special casing deferral may not be "wonderful", but it is what currently 
happens in many places, and it's just "the best we can do for now". A similar 
patch for MMC got approval:
https://lkml.org/lkml/2014/10/27/477

>> I have to explain over and over again that there's no problem when that
>> message comes along ten times in a row. And it causes people to overlook the
>> messages that really are errors.
>
> Can we do something with the log message that triggers on probe
> deferral?  There tends to be a learning curve with probe deferral but
> the fact that it's generally extremely noisy tends to be useful - I
> usually point people at that (not just in the context at regulators) and
> tell them not to worry unless debugging.

Using "dev_err" is not really "tell them not to worry unless debugging", I 
think that is what "dev_dbg" was meant to do.

The only real solution I could come up with here is to replace "return 
-EPROBE_DEFER" with something that stores the current stack, registers the 
resource it requires and jumps back to where the driver probe originated. Once 
the resource is available, the stored stack is resumed and then the probe code 
path can continue as if nothing bad happened. This would also deliver 
excellent diagnostic data in case the resource remains absent. I've built 
something like this in Python which has a "yield" statement one can use for 
this purpose. It's a bit tougher to do in C I guess. So until then, we're 
stuck with sprinkling "if (ret == -EPROBE_DEFER)" code snippets all over the 
place.




Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Systems
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: (+31) (0) 499 33 69 79
Telefax:  (+31) (0) 499 33 69 70
E-mail: mike.looijmans@topic.nl
Website: www.topic.nl

Please consider the environment before printing this e-mail

Topic zoekt gedreven (embedded) software specialisten!
http://topic.nl/vacatures/topic-zoekt-software-engineers/

Re: [PATCH 1/2] drivers/regulator/core.c: Don't print error on EPROBE_DEFER

[Mark Brown]

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

On Tue, Dec 16, 2014 at 11:27:03AM +0100, Mike Looijmans wrote:
> On 12/09/2014 07:48 PM, Mark Brown wrote:
> >On Tue, Dec 09, 2014 at 07:12:30PM +0100, Mike Looijmans wrote:

> >>Can we instead at least reduce it to WARN or INFO level then?

> >You appear to have deleted my reply here...  one problem with your
> >suggestion is that it means we have to special case all error handling
> >on probe for deferral which isn't wonderful.

> special casing deferral may not be "wonderful", but it is what currently
> happens in many places, and it's just "the best we can do for now". A
> similar patch for MMC got approval:
> https://lkml.org/lkml/2014/10/27/477

That's just one call site, were all the other places that might see a
probe deferral updated too?

That also looks like a case of the core passing through other errors
rather than a case of primary error reporting - I guess there's a
reasonable case for the core not logging at all here since the drivers
ought to be doing it.

> >>I have to explain over and over again that there's no problem when that
> >>message comes along ten times in a row. And it causes people to overlook the
> >>messages that really are errors.

> >Can we do something with the log message that triggers on probe
> >deferral?  There tends to be a learning curve with probe deferral but
> >the fact that it's generally extremely noisy tends to be useful - I
> >usually point people at that (not just in the context at regulators) and
> >tell them not to worry unless debugging.

> Using "dev_err" is not really "tell them not to worry unless debugging", I
> think that is what "dev_dbg" was meant to do.

Well, then the core probe deferral stuff ought to be less chatty then...

> The only real solution I could come up with here is to replace "return
> -EPROBE_DEFER" with something that stores the current stack, registers the
> resource it requires and jumps back to where the driver probe originated.
> Once the resource is available, the stored stack is resumed and then the
> probe code path can continue as if nothing bad happened. This would also
> deliver excellent diagnostic data in case the resource remains absent. I've
> built something like this in Python which has a "yield" statement one can
> use for this purpose. It's a bit tougher to do in C I guess. So until then,
> we're stuck with sprinkling "if (ret == -EPROBE_DEFER)" code snippets all
> over the place.

There was a proposal the other day for a restrack framework (name might
change in future) which drivers would call in their probe and get a
callback when everything appears.

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