linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] siox: Simplify error handling via dev_err_probe()
@ 2021-05-15  8:20 Thorsten Scherer
  2021-05-15 18:00 ` Uwe Kleine-König
  0 siblings, 1 reply; 3+ messages in thread
From: Thorsten Scherer @ 2021-05-15  8:20 UTC (permalink / raw)
  To: u.kleine-koenig, gregkh; +Cc: kernel, linux-kernel, t.scherer

a787e5400a1c ("driver core: add device probe log helper") introduced a
helper for a common error checking pattern.  Use it.

Signed-off-by: Thorsten Scherer <t.scherer@eckelmann.de>
---
 drivers/siox/siox-bus-gpio.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/siox/siox-bus-gpio.c b/drivers/siox/siox-bus-gpio.c
index 46b4cda36bac..aeefeb725524 100644
--- a/drivers/siox/siox-bus-gpio.c
+++ b/drivers/siox/siox-bus-gpio.c
@@ -102,29 +102,29 @@ static int siox_gpio_probe(struct platform_device *pdev)
 
 	ddata->din = devm_gpiod_get(dev, "din", GPIOD_IN);
 	if (IS_ERR(ddata->din)) {
-		ret = PTR_ERR(ddata->din);
-		dev_err(dev, "Failed to get %s GPIO: %d\n", "din", ret);
+		ret = dev_err_probe(dev, PTR_ERR(ddata->din),
+				    "Failed to get din GPIO\n");
 		goto err;
 	}
 
 	ddata->dout = devm_gpiod_get(dev, "dout", GPIOD_OUT_LOW);
 	if (IS_ERR(ddata->dout)) {
-		ret = PTR_ERR(ddata->dout);
-		dev_err(dev, "Failed to get %s GPIO: %d\n", "dout", ret);
+		ret = dev_err_probe(dev, PTR_ERR(ddata->dout),
+				    "Failed to get dout GPIO\n");
 		goto err;
 	}
 
 	ddata->dclk = devm_gpiod_get(dev, "dclk", GPIOD_OUT_LOW);
 	if (IS_ERR(ddata->dclk)) {
-		ret = PTR_ERR(ddata->dclk);
-		dev_err(dev, "Failed to get %s GPIO: %d\n", "dclk", ret);
+		ret = dev_err_probe(dev, PTR_ERR(ddata->dclk),
+				    "Failed to get dclk GPIO\n");
 		goto err;
 	}
 
 	ddata->dld = devm_gpiod_get(dev, "dld", GPIOD_OUT_LOW);
 	if (IS_ERR(ddata->dld)) {
-		ret = PTR_ERR(ddata->dld);
-		dev_err(dev, "Failed to get %s GPIO: %d\n", "dld", ret);
+		ret = dev_err_probe(dev, PTR_ERR(ddata->dld),
+				    "Failed to get dld GPIO\n");
 		goto err;
 	}
 
@@ -134,7 +134,8 @@ static int siox_gpio_probe(struct platform_device *pdev)
 
 	ret = siox_master_register(smaster);
 	if (ret) {
-		dev_err(dev, "Failed to register siox master: %d\n", ret);
+		dev_err_probe(dev, ret,
+			      "Failed to register siox master\n");
 err:
 		siox_master_put(smaster);
 	}
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] siox: Simplify error handling via dev_err_probe()
  2021-05-15  8:20 [PATCH] siox: Simplify error handling via dev_err_probe() Thorsten Scherer
@ 2021-05-15 18:00 ` Uwe Kleine-König
  2021-05-15 19:47   ` Thorsten Scherer
  0 siblings, 1 reply; 3+ messages in thread
From: Uwe Kleine-König @ 2021-05-15 18:00 UTC (permalink / raw)
  To: Thorsten Scherer; +Cc: gregkh, linux-kernel, kernel

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

Hello Thorsten,

your mail is whitespace damaged and cannot be applied directly. As you
used git-send-email this is probably a case for Eckelmann IT ...

On Sat, May 15, 2021 at 10:20:17AM +0200, Thorsten Scherer wrote:
> a787e5400a1c ("driver core: add device probe log helper") introduced a
> helper for a common error checking pattern.  Use it.

Please test your patch using scripts/checkpatch and fix the issued
errors (or argument why you chose not to follow its recommendations).

> Signed-off-by: Thorsten Scherer <t.scherer@eckelmann.de>
> ---
>  drivers/siox/siox-bus-gpio.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/siox/siox-bus-gpio.c b/drivers/siox/siox-bus-gpio.c
> index 46b4cda36bac..aeefeb725524 100644
> --- a/drivers/siox/siox-bus-gpio.c
> +++ b/drivers/siox/siox-bus-gpio.c
> @@ -102,29 +102,29 @@ static int siox_gpio_probe(struct platform_device *pdev)
> 
>         ddata->din = devm_gpiod_get(dev, "din", GPIOD_IN);
>         if (IS_ERR(ddata->din)) {
> -               ret = PTR_ERR(ddata->din);
> -               dev_err(dev, "Failed to get %s GPIO: %d\n", "din", ret);
> +               ret = dev_err_probe(dev, PTR_ERR(ddata->din),
> +                                   "Failed to get din GPIO\n");

Huh, I'm surprised. I did

diff --git a/drivers/siox/siox-bus-gpio.c b/drivers/siox/siox-bus-gpio.c
index aeefeb725524..b97fde71a6a0 100644
--- a/drivers/siox/siox-bus-gpio.c
+++ b/drivers/siox/siox-bus-gpio.c
@@ -103,28 +103,28 @@ static int siox_gpio_probe(struct platform_device *pdev)
 	ddata->din = devm_gpiod_get(dev, "din", GPIOD_IN);
 	if (IS_ERR(ddata->din)) {
 		ret = dev_err_probe(dev, PTR_ERR(ddata->din),
-				    "Failed to get din GPIO\n");
+				    "Failed to get %s GPIO\n", "din");
 		goto err;
 	}
 
 	ddata->dout = devm_gpiod_get(dev, "dout", GPIOD_OUT_LOW);
 	if (IS_ERR(ddata->dout)) {
 		ret = dev_err_probe(dev, PTR_ERR(ddata->dout),
-				    "Failed to get dout GPIO\n");
+				    "Failed to get %s GPIO\n", "dout");
 		goto err;
 	}
 
 	ddata->dclk = devm_gpiod_get(dev, "dclk", GPIOD_OUT_LOW);
 	if (IS_ERR(ddata->dclk)) {
 		ret = dev_err_probe(dev, PTR_ERR(ddata->dclk),
-				    "Failed to get dclk GPIO\n");
+				    "Failed to get %s GPIO\n", "dclk");
 		goto err;
 	}
 
 	ddata->dld = devm_gpiod_get(dev, "dld", GPIOD_OUT_LOW);
 	if (IS_ERR(ddata->dld)) {
 		ret = dev_err_probe(dev, PTR_ERR(ddata->dld),
-				    "Failed to get dld GPIO\n");
+				    "Failed to get %s GPIO\n", "dld");
 		goto err;
 	}
 
on top of your patch and the binary size increased (using ARCH=arm and
gcc 7.3.1). So no objection from me to get rid of this idiom.

>                 goto err;
>         }
> 
>         ddata->dout = devm_gpiod_get(dev, "dout", GPIOD_OUT_LOW);
>         if (IS_ERR(ddata->dout)) {
> -               ret = PTR_ERR(ddata->dout);
> -               dev_err(dev, "Failed to get %s GPIO: %d\n", "dout", ret);
> +               ret = dev_err_probe(dev, PTR_ERR(ddata->dout),
> +                                   "Failed to get dout GPIO\n");
>                 goto err;
>         }
> 
>         ddata->dclk = devm_gpiod_get(dev, "dclk", GPIOD_OUT_LOW);
>         if (IS_ERR(ddata->dclk)) {
> -               ret = PTR_ERR(ddata->dclk);
> -               dev_err(dev, "Failed to get %s GPIO: %d\n", "dclk", ret);
> +               ret = dev_err_probe(dev, PTR_ERR(ddata->dclk),
> +                                   "Failed to get dclk GPIO\n");
>                 goto err;
>         }
> 
>         ddata->dld = devm_gpiod_get(dev, "dld", GPIOD_OUT_LOW);
>         if (IS_ERR(ddata->dld)) {
> -               ret = PTR_ERR(ddata->dld);
> -               dev_err(dev, "Failed to get %s GPIO: %d\n", "dld", ret);
> +               ret = dev_err_probe(dev, PTR_ERR(ddata->dld),
> +                                   "Failed to get dld GPIO\n");
>                 goto err;
>         }
> 
> @@ -134,7 +134,8 @@ static int siox_gpio_probe(struct platform_device *pdev)
> 
>         ret = siox_master_register(smaster);
>         if (ret) {
> -               dev_err(dev, "Failed to register siox master: %d\n", ret);
> +               dev_err_probe(dev, ret,
> +                             "Failed to register siox master\n");
>  err:
>                 siox_master_put(smaster);
>         }
> --
> 2.29.2
> 
> Eckelmann AG
> Vorstand: Dipl.-Ing. Peter Frankenbach (Sprecher) Dipl.-Wi.-Ing. Philipp Eckelmann
> Dr.-Ing. Marco M?nchhof

Another issue for your IT department: Tell them please to not append
latin1 encoded footers to mails that don't declare an encoding (and so
are implicitly ASCII only). I didn't check but I assume this will earn
you a few spam assassin points ...

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] siox: Simplify error handling via dev_err_probe()
  2021-05-15 18:00 ` Uwe Kleine-König
@ 2021-05-15 19:47   ` Thorsten Scherer
  0 siblings, 0 replies; 3+ messages in thread
From: Thorsten Scherer @ 2021-05-15 19:47 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: gregkh, linux-kernel, kernel

Hello Uwe,

On Sat, May 15, 2021 at 08:00:47PM +0200, Uwe Kleine-König wrote:
> Hello Thorsten,
> 
> your mail is whitespace damaged and cannot be applied directly. As you
> used git-send-email this is probably a case for Eckelmann IT ...

I will resend the patch as soon as the issues with our IT are solved.

> On Sat, May 15, 2021 at 10:20:17AM +0200, Thorsten Scherer wrote:
> > a787e5400a1c ("driver core: add device probe log helper") introduced a
> > helper for a common error checking pattern.  Use it.
> 
> Please test your patch using scripts/checkpatch and fix the issued
> errors (or argument why you chose not to follow its recommendations).

I will fix this.

> > Signed-off-by: Thorsten Scherer <t.scherer@eckelmann.de>
> > ---
> >  drivers/siox/siox-bus-gpio.c | 19 ++++++++++---------
> >  1 file changed, 10 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/siox/siox-bus-gpio.c b/drivers/siox/siox-bus-gpio.c
> > index 46b4cda36bac..aeefeb725524 100644
> > --- a/drivers/siox/siox-bus-gpio.c
> > +++ b/drivers/siox/siox-bus-gpio.c
> > @@ -102,29 +102,29 @@ static int siox_gpio_probe(struct platform_device *pdev)
> > 
> >         ddata->din = devm_gpiod_get(dev, "din", GPIOD_IN);
> >         if (IS_ERR(ddata->din)) {
> > -               ret = PTR_ERR(ddata->din);
> > -               dev_err(dev, "Failed to get %s GPIO: %d\n", "din", ret);
> > +               ret = dev_err_probe(dev, PTR_ERR(ddata->din),
> > +                                   "Failed to get din GPIO\n");
> 
> Huh, I'm surprised. I did
> 
> diff --git a/drivers/siox/siox-bus-gpio.c b/drivers/siox/siox-bus-gpio.c
> index aeefeb725524..b97fde71a6a0 100644
> --- a/drivers/siox/siox-bus-gpio.c
> +++ b/drivers/siox/siox-bus-gpio.c
> @@ -103,28 +103,28 @@ static int siox_gpio_probe(struct platform_device *pdev)
>  	ddata->din = devm_gpiod_get(dev, "din", GPIOD_IN);
>  	if (IS_ERR(ddata->din)) {
>  		ret = dev_err_probe(dev, PTR_ERR(ddata->din),
> -				    "Failed to get din GPIO\n");
> +				    "Failed to get %s GPIO\n", "din");
>  		goto err;
>  	}
>  
>  	ddata->dout = devm_gpiod_get(dev, "dout", GPIOD_OUT_LOW);
>  	if (IS_ERR(ddata->dout)) {
>  		ret = dev_err_probe(dev, PTR_ERR(ddata->dout),
> -				    "Failed to get dout GPIO\n");
> +				    "Failed to get %s GPIO\n", "dout");
>  		goto err;
>  	}
>  
>  	ddata->dclk = devm_gpiod_get(dev, "dclk", GPIOD_OUT_LOW);
>  	if (IS_ERR(ddata->dclk)) {
>  		ret = dev_err_probe(dev, PTR_ERR(ddata->dclk),
> -				    "Failed to get dclk GPIO\n");
> +				    "Failed to get %s GPIO\n", "dclk");
>  		goto err;
>  	}
>  
>  	ddata->dld = devm_gpiod_get(dev, "dld", GPIOD_OUT_LOW);
>  	if (IS_ERR(ddata->dld)) {
>  		ret = dev_err_probe(dev, PTR_ERR(ddata->dld),
> -				    "Failed to get dld GPIO\n");
> +				    "Failed to get %s GPIO\n", "dld");
>  		goto err;
>  	}
>  
> on top of your patch and the binary size increased (using ARCH=arm and
> gcc 7.3.1). So no objection from me to get rid of this idiom.
> 
> >                 goto err;
> >         }
> > 
> >         ddata->dout = devm_gpiod_get(dev, "dout", GPIOD_OUT_LOW);
> >         if (IS_ERR(ddata->dout)) {
> > -               ret = PTR_ERR(ddata->dout);
> > -               dev_err(dev, "Failed to get %s GPIO: %d\n", "dout", ret);
> > +               ret = dev_err_probe(dev, PTR_ERR(ddata->dout),
> > +                                   "Failed to get dout GPIO\n");
> >                 goto err;
> >         }
> > 
> >         ddata->dclk = devm_gpiod_get(dev, "dclk", GPIOD_OUT_LOW);
> >         if (IS_ERR(ddata->dclk)) {
> > -               ret = PTR_ERR(ddata->dclk);
> > -               dev_err(dev, "Failed to get %s GPIO: %d\n", "dclk", ret);
> > +               ret = dev_err_probe(dev, PTR_ERR(ddata->dclk),
> > +                                   "Failed to get dclk GPIO\n");
> >                 goto err;
> >         }
> > 
> >         ddata->dld = devm_gpiod_get(dev, "dld", GPIOD_OUT_LOW);
> >         if (IS_ERR(ddata->dld)) {
> > -               ret = PTR_ERR(ddata->dld);
> > -               dev_err(dev, "Failed to get %s GPIO: %d\n", "dld", ret);
> > +               ret = dev_err_probe(dev, PTR_ERR(ddata->dld),
> > +                                   "Failed to get dld GPIO\n");
> >                 goto err;
> >         }
> > 
> > @@ -134,7 +134,8 @@ static int siox_gpio_probe(struct platform_device *pdev)
> > 
> >         ret = siox_master_register(smaster);
> >         if (ret) {
> > -               dev_err(dev, "Failed to register siox master: %d\n", ret);
> > +               dev_err_probe(dev, ret,
> > +                             "Failed to register siox master\n");
> >  err:
> >                 siox_master_put(smaster);
> >         }
> > --
> > 2.29.2
> > 
> > Eckelmann AG
> > Vorstand: Dipl.-Ing. Peter Frankenbach (Sprecher) Dipl.-Wi.-Ing. Philipp Eckelmann
> > Dr.-Ing. Marco M?nchhof
> 
> Another issue for your IT department: Tell them please to not append
> latin1 encoded footers to mails that don't declare an encoding (and so
> are implicitly ASCII only).

Will do so.

> I didn't check but I assume this will earn
> you a few spam assassin points ...

Thank you for pointing things out.

> Best regards
> Uwe
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Best regards
Thorsten

--
Thorsten Scherer | Eckelmann AG | www.eckelmann.de |

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-15 19:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-15  8:20 [PATCH] siox: Simplify error handling via dev_err_probe() Thorsten Scherer
2021-05-15 18:00 ` Uwe Kleine-König
2021-05-15 19:47   ` Thorsten Scherer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).