| Message ID | 20240801091255.1410027-1-martin.kepplinger-novakovic@ginzinger.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| 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
Am Donnerstag, dem 01.08.2024 um 11:26 +0200 schrieb Uwe Kleine-König: > 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. > 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 :) thanks, martin
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
Am Donnerstag, dem 01.08.2024 um 12:09 +0200 schrieb Uwe Kleine-König: > 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. > hi Uwe, sorry, I agree that this patch doesn't add any information. I think it can be expected to look at debug when drivers don't probe. thanks, martin
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(-)