2021-05-15 17:56:16

by Thorsten Scherer

[permalink] [raw]
Subject: [PATCH] siox: Simplify error handling via dev_err_probe()

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

Signed-off-by: Thorsten Scherer <[email protected]>
---
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



2021-05-16 07:53:40

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH] siox: Simplify error handling via dev_err_probe()

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 <[email protected]>
> ---
> 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/ |


Attachments:
(No filename) (4.88 kB)
signature.asc (499.00 B)
Download all attachments

2021-05-16 09:08:33

by Thorsten Scherer

[permalink] [raw]
Subject: Re: [PATCH] siox: Simplify error handling via dev_err_probe()

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 <[email protected]>
> > ---
> > 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 | http://www.eckelmann.de |