Deferred probe is an expected return value for devm_pwm_get(). Given
that the driver deals with it properly, there's no need to output a
warning that may potentially confuse users.
Signed-off-by: Jon Hunter <[email protected]>
---
drivers/regulator/pwm-regulator.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index e74e11101fc1..fb25777a7d47 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -354,7 +354,8 @@ static int pwm_regulator_probe(struct platform_device *pdev)
drvdata->pwm = devm_pwm_get(&pdev->dev, NULL);
if (IS_ERR(drvdata->pwm)) {
ret = PTR_ERR(drvdata->pwm);
- dev_err(&pdev->dev, "Failed to get PWM: %d\n", ret);
+ if (ret != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Failed to get PWM: %d\n", ret);
return ret;
}
--
2.17.1
On Mon, Feb 24, 2020 at 02:40:48PM +0000, Jon Hunter wrote:
> Deferred probe is an expected return value for devm_pwm_get(). Given
> that the driver deals with it properly, there's no need to output a
> warning that may potentially confuse users.
>
> Signed-off-by: Jon Hunter <[email protected]>
> ---
> drivers/regulator/pwm-regulator.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Thierry Reding <[email protected]>
On Mon, Feb 24, 2020 at 02:40:48PM +0000, Jon Hunter wrote:
> Deferred probe is an expected return value for devm_pwm_get(). Given
> that the driver deals with it properly, there's no need to output a
> warning that may potentially confuse users.
> ret = PTR_ERR(drvdata->pwm);
> - dev_err(&pdev->dev, "Failed to get PWM: %d\n", ret);
> + if (ret != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "Failed to get PWM: %d\n", ret);
This then means that there's no way for users to determine why the
driver has failed to instantiate which can be frustrating. It'd be
better to at least have some dev_dbg() output when deferring so that
there's something for people to go on without having to instrument the
code.
On Mon, Feb 24, 2020 at 04:58:59PM +0000, Mark Brown wrote:
> On Mon, Feb 24, 2020 at 02:40:48PM +0000, Jon Hunter wrote:
> > Deferred probe is an expected return value for devm_pwm_get(). Given
> > that the driver deals with it properly, there's no need to output a
> > warning that may potentially confuse users.
>
> > ret = PTR_ERR(drvdata->pwm);
> > - dev_err(&pdev->dev, "Failed to get PWM: %d\n", ret);
> > + if (ret != -EPROBE_DEFER)
> > + dev_err(&pdev->dev, "Failed to get PWM: %d\n", ret);
>
> This then means that there's no way for users to determine why the
> driver has failed to instantiate which can be frustrating. It'd be
> better to at least have some dev_dbg() output when deferring so that
> there's something for people to go on without having to instrument the
> code.
Not printing an error message is quite usual however. I think a generic
approach that for example makes the list of devices that should be
retried to probe on the next opportunity inspectable would be nice.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Wed, Feb 26, 2020 at 05:17:57PM +0100, Uwe Kleine-K?nig wrote:
> On Mon, Feb 24, 2020 at 04:58:59PM +0000, Mark Brown wrote:
> > This then means that there's no way for users to determine why the
> > driver has failed to instantiate which can be frustrating. It'd be
> > better to at least have some dev_dbg() output when deferring so that
> > there's something for people to go on without having to instrument the
> > code.
> Not printing an error message is quite usual however. I think a generic
Usual yet also frustraing.
> approach that for example makes the list of devices that should be
> retried to probe on the next opportunity inspectable would be nice.
That's not really the issue, the bigger issue is trying to figure out
why things are stuck - what exactly caused things to fail to
instantiate.
On Wed, Feb 26, 2020 at 04:39:05PM +0000, Mark Brown wrote:
> On Wed, Feb 26, 2020 at 05:17:57PM +0100, Uwe Kleine-K?nig wrote:
> > On Mon, Feb 24, 2020 at 04:58:59PM +0000, Mark Brown wrote:
>
> > > This then means that there's no way for users to determine why the
> > > driver has failed to instantiate which can be frustrating. It'd be
> > > better to at least have some dev_dbg() output when deferring so that
> > > there's something for people to go on without having to instrument the
> > > code.
>
> > Not printing an error message is quite usual however. I think a generic
>
> Usual yet also frustraing.
>
> > approach that for example makes the list of devices that should be
> > retried to probe on the next opportunity inspectable would be nice.
>
> That's not really the issue, the bigger issue is trying to figure out
> why things are stuck - what exactly caused things to fail to
> instantiate.
I wonder if we should do something like:
ret = some_call(some, args);
if (ret) {
if (emit_errmsg_for_err(ret))
dev_err(dev, "some_call failed: %pE\n", ERR_PTR(ret));
return ret;
}
and have emit_errmsg_for_err return true if ret != -EPROBE_DEFER or some
kernel parameter is given.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Fri, Mar 06, 2020 at 08:51:29AM +0100, Uwe Kleine-K?nig wrote:
> I wonder if we should do something like:
> ret = some_call(some, args);
> if (ret) {
> if (emit_errmsg_for_err(ret))
> dev_err(dev, "some_call failed: %pE\n", ERR_PTR(ret));
> return ret;
> }
> and have emit_errmsg_for_err return true if ret != -EPROBE_DEFER or some
> kernel parameter is given.
There was some effort in the past to have a dev_probe_err() or something
which could have a similar implementation but that didn't end up going
anywhere I think. I do prefer the debug level log since it's much
easier to have the information there without having to ask for it, that
design would've supported that.