| Message ID | 20240801091255.1410027-1-martin.kepplinger-novakovic@ginzinger.com |
|---|---|
| State | New |
| Headers | show |
| Series | backlight: pwm_bl: print errno for probe errors | expand |
Hello Martin, On Thu, Aug 01, 2024 at 11:12:55AM +0200, Martin Kepplinger-Novaković wrote: > This makes debugging often easier. > > Signed-off-by: Martin Kepplinger-Novaković <martin.kepplinger-novakovic@ginzinger.com> > --- > drivers/video/backlight/pwm_bl.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c > index f1005bd0c41e3..cc7e7af71891f 100644 > --- a/drivers/video/backlight/pwm_bl.c > +++ b/drivers/video/backlight/pwm_bl.c > @@ -502,7 +502,8 @@ static int pwm_backlight_probe(struct platform_device *pdev) > GPIOD_ASIS); > if (IS_ERR(pb->enable_gpio)) { > ret = dev_err_probe(&pdev->dev, PTR_ERR(pb->enable_gpio), > - "failed to acquire enable GPIO\n"); > + "failed to acquire enable GPIO: %ld\n", > + PTR_ERR(pb->enable_gpio)); AFAIK dev_err_probe already emits the error code passed as 2nd parameter. So I wonder about this patch's benefit. Best regards Uwe
Hello Martin, On Thu, Aug 01, 2024 at 09:52:01AM +0000, Kepplinger-Novakovic Martin wrote: > Am Donnerstag, dem 01.08.2024 um 11:26 +0200 schrieb Uwe Kleine-König: > > On Thu, Aug 01, 2024 at 11:12:55AM +0200, Martin Kepplinger-Novaković > > wrote: > > > diff --git a/drivers/video/backlight/pwm_bl.c > > > b/drivers/video/backlight/pwm_bl.c > > > index f1005bd0c41e3..cc7e7af71891f 100644 > > > --- a/drivers/video/backlight/pwm_bl.c > > > +++ b/drivers/video/backlight/pwm_bl.c > > > @@ -502,7 +502,8 @@ static int pwm_backlight_probe(struct > > > platform_device *pdev) > > > GPIOD_ASIS); > > > if (IS_ERR(pb->enable_gpio)) { > > > ret = dev_err_probe(&pdev->dev, PTR_ERR(pb- > > > >enable_gpio), > > > - "failed to acquire enable > > > GPIO\n"); > > > + "failed to acquire enable GPIO: > > > %ld\n", > > > + PTR_ERR(pb->enable_gpio)); > > > > AFAIK dev_err_probe already emits the error code passed as 2nd > > parameter. So I wonder about this patch's benefit. > > > > It does. Other messages only take the deferred_probe_reason without the > error code. It's actually fine if users properly enable debugging after > seeing an error and then this change is not needed :) I'm unsure what you intend to say here. Do you agree that this patch doesn't need to be applied as it doesn't add any information to the emitted messages? Or do you think there is a value because "users don't need to enable debugging" then. In the latter case I don't see where users would see "failed to acquire enable GPIO" before, but not the value of the error code. Best regards Uwe
On Thu, 01 Aug 2024, Martin Kepplinger-Novaković wrote: > This makes debugging often easier. > > Signed-off-by: Martin Kepplinger-Novaković <martin.kepplinger-novakovic@ginzinger.com> > --- > drivers/video/backlight/pwm_bl.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) Please refrain from signing your mails like this. It means that some of us have to physically click a pop-up box as we are parsing our inboxes. I'm deleting all mails in this thread.
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index f1005bd0c41e3..cc7e7af71891f 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -502,7 +502,8 @@ static int pwm_backlight_probe(struct platform_device *pdev) GPIOD_ASIS); if (IS_ERR(pb->enable_gpio)) { ret = dev_err_probe(&pdev->dev, PTR_ERR(pb->enable_gpio), - "failed to acquire enable GPIO\n"); + "failed to acquire enable GPIO: %ld\n", + PTR_ERR(pb->enable_gpio)); goto err_alloc; } @@ -513,7 +514,8 @@ static int pwm_backlight_probe(struct platform_device *pdev) pb->power_supply = NULL; } else { dev_err_probe(&pdev->dev, ret, - "failed to acquire power regulator\n"); + "failed to acquire power regulator: %d\n", + ret); goto err_alloc; } } @@ -521,7 +523,8 @@ static int pwm_backlight_probe(struct platform_device *pdev) pb->pwm = devm_pwm_get(&pdev->dev, NULL); if (IS_ERR(pb->pwm)) { ret = dev_err_probe(&pdev->dev, PTR_ERR(pb->pwm), - "unable to request PWM\n"); + "unable to request PWM: %ld\n", + PTR_ERR(pb->pwm)); goto err_alloc; }
This makes debugging often easier. Signed-off-by: Martin Kepplinger-Novaković <martin.kepplinger-novakovic@ginzinger.com> --- drivers/video/backlight/pwm_bl.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)