[PATCH] regulator: Don't print error in devm_regulator_bulk_get() on -EPROBE_DEFER

[PATCH] regulator: Don't print error in devm_regulator_bulk_get() on -EPROBE_DEFER

[Javier Martinez Canillas]

The regulators may not be available just because their driver's probe was
not executed and the regulators were not registered yet. So don't print an
error in this case to avoid polluting the kernel log and confuse the users.

Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---

 drivers/regulator/devres.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 6ad8ab4c578d..3d023422e228 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -171,8 +171,9 @@ int devm_regulator_bulk_get(struct device *dev, int num_consumers,
 								NORMAL_GET);
 		if (IS_ERR(consumers[i].consumer)) {
 			ret = PTR_ERR(consumers[i].consumer);
-			dev_err(dev, "Failed to get supply '%s': %d\n",
-				consumers[i].supply, ret);
+			if (ret != -EPROBE_DEFER)
+				dev_err(dev, "Failed to get supply '%s': %d\n",
+					consumers[i].supply, ret);
 			consumers[i].consumer = NULL;
 			goto err;
 		}
-- 
2.5.0

Re: [PATCH] regulator: Don't print error in devm_regulator_bulk_get() on -EPROBE_DEFER

[Mark Brown]

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

On Tue, Mar 15, 2016 at 05:35:14PM -0300, Javier Martinez Canillas wrote:

> The regulators may not be available just because their driver's probe was
> not executed and the regulators were not registered yet. So don't print an
> error in this case to avoid polluting the kernel log and confuse the users.

We've been through this repeatedly - this is the case for all probe
deferrals and if we just don't print the errors at all then people won't
be able to tell if they've got a missing dependency.  We need people to
work on things like allowing the kernel to make use of the dependency
information it has to order probes more sensibly.

Raphael had some ideas he was talking about at Kernel Summit for this,
Russell King also proposed just suppressing the errors until we're at
the last deferred probe run at the end of init.

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

Re: [PATCH] regulator: Don't print error in devm_regulator_bulk_get() on -EPROBE_DEFER

[Javier Martinez Canillas]

Hello Mark,

On 03/16/2016 06:59 AM, Mark Brown wrote:
> On Tue, Mar 15, 2016 at 05:35:14PM -0300, Javier Martinez Canillas wrote:
> 
>> The regulators may not be available just because their driver's probe was
>> not executed and the regulators were not registered yet. So don't print an
>> error in this case to avoid polluting the kernel log and confuse the users.
> 
> We've been through this repeatedly - this is the case for all probe
> deferrals and if we just don't print the errors at all then people won't
> be able to tell if they've got a missing dependency.  We need people to

I wonder if we can use a dev_dbg() then so it will be printed at debug level.
That way users can have that info if they want but not pollute the boot log.

Basically I want to get rid of things like this:

[    1.438422] exynos-hdmi 14530000.hdmi: Failed to get supply 'vdd': -517
[    1.443638] [drm:hdmi_probe] *ERROR* failed to get regulators
[    1.449326] [drm:hdmi_probe] *ERROR* hdmi_resources_init failed

The HDMI driver probed correctly but looking at the log it seems that things
failed so can mislead users not familiar with the probe deferral mechanism.

> work on things like allowing the kernel to make use of the dependency
> information it has to order probes more sensibly.
>

Agreed, I've seen Tomeu's patches and the following discussions.
 
> Raphael had some ideas he was talking about at Kernel Summit for this,
> Russell King also proposed just suppressing the errors until we're at
> the last deferred probe run at the end of init.
> 

Suppressing errors is a nice idea but will again lead to false positives
when the drivers are built as a module AFAICT.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

Re: [PATCH] regulator: Don't print error in devm_regulator_bulk_get() on -EPROBE_DEFER

[Mark Brown]

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

On Wed, Mar 16, 2016 at 08:19:42AM -0300, Javier Martinez Canillas wrote:
> On 03/16/2016 06:59 AM, Mark Brown wrote:
> > On Tue, Mar 15, 2016 at 05:35:14PM -0300, Javier Martinez Canillas wrote:

> > We've been through this repeatedly - this is the case for all probe
> > deferrals and if we just don't print the errors at all then people won't
> > be able to tell if they've got a missing dependency.  We need people to

> I wonder if we can use a dev_dbg() then so it will be printed at debug level.
> That way users can have that info if they want but not pollute the boot log.

No, that has exactly the same problems.

> Basically I want to get rid of things like this:

> [    1.438422] exynos-hdmi 14530000.hdmi: Failed to get supply 'vdd': -517
> [    1.443638] [drm:hdmi_probe] *ERROR* failed to get regulators
> [    1.449326] [drm:hdmi_probe] *ERROR* hdmi_resources_init failed

> The HDMI driver probed correctly but looking at the log it seems that things
> failed so can mislead users not familiar with the probe deferral mechanism.

So fix probe deferral.  Don't go through the entire kernel hacking in
bodges everywhere, if we have to hack the error handling for almost all
resource acquisition errors during probe that's obviously not sensible.

> > work on things like allowing the kernel to make use of the dependency
> > information it has to order probes more sensibly.

> Agreed, I've seen Tomeu's patches and the following discussions.

Raphael was proposing something a bit different there.

> > Raphael had some ideas he was talking about at Kernel Summit for this,
> > Russell King also proposed just suppressing the errors until we're at
> > the last deferred probe run at the end of init.

> Suppressing errors is a nice idea but will again lead to false positives
> when the drivers are built as a module AFAICT.

It's not a fix, it's just a 90% masking of the problem that does
everything in one place rather than being lots of individual changes.
Modules are always going to be problematic for this and in any case the
thing there is that we need similar enhancements in the userspace
utilities.

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

Re: [PATCH] regulator: Don't print error in devm_regulator_bulk_get() on -EPROBE_DEFER

[Javier Martinez Canillas]

Hello Mark,

On 03/16/2016 08:37 AM, Mark Brown wrote:
> On Wed, Mar 16, 2016 at 08:19:42AM -0300, Javier Martinez Canillas wrote:

[snip]

> 
>> Basically I want to get rid of things like this:
> 
>> [    1.438422] exynos-hdmi 14530000.hdmi: Failed to get supply 'vdd': -517
>> [    1.443638] [drm:hdmi_probe] *ERROR* failed to get regulators
>> [    1.449326] [drm:hdmi_probe] *ERROR* hdmi_resources_init failed
> 
>> The HDMI driver probed correctly but looking at the log it seems that things
>> failed so can mislead users not familiar with the probe deferral mechanism.
> 
> So fix probe deferral.  Don't go through the entire kernel hacking in
> bodges everywhere, if we have to hack the error handling for almost all
> resource acquisition errors during probe that's obviously not sensible.
> 

Got it, thanks a lot for your explanations.

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America