gpio: gpio-xgene: simplify probe, return devm_gpiochip_add_data() directly
diff mbox series

Message ID 20210521181042.7726-1-aardelean@deviqon.com
State Accepted
Commit 94a7b66910ea6e4583addd8212aa8cd3f7a8c9c1
Headers show
Series
  • gpio: gpio-xgene: simplify probe, return devm_gpiochip_add_data() directly
Related show

Commit Message

Alexandru Ardelean May 21, 2021, 6:10 p.m. UTC
The handling of the return value from devm_gpiochip_add_data() is a bit
redundant. It prints messages on error and success cases.
While the success message may be useful, it is more in the area of log
spam, and these can be printed with other forms of kernel logging.

This change does a direct return with devm_gpiochip_add_data() in the probe
function.

The platform_set_drvdata() is needed, as this driver uses the stored
private date in the PM suspend/resume routines.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/gpio/gpio-xgene.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

Comments

Bartosz Golaszewski May 23, 2021, 6:31 p.m. UTC | #1
On Fri, May 21, 2021 at 8:10 PM Alexandru Ardelean
<aardelean@deviqon.com> wrote:
>
> The handling of the return value from devm_gpiochip_add_data() is a bit
> redundant. It prints messages on error and success cases.
> While the success message may be useful, it is more in the area of log
> spam, and these can be printed with other forms of kernel logging.
>
> This change does a direct return with devm_gpiochip_add_data() in the probe
> function.
>
> The platform_set_drvdata() is needed, as this driver uses the stored
> private date in the PM suspend/resume routines.
>
> Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> ---
>  drivers/gpio/gpio-xgene.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/gpio/gpio-xgene.c b/drivers/gpio/gpio-xgene.c
> index 532b0df8a1f2..fb4b0c67aeef 100644
> --- a/drivers/gpio/gpio-xgene.c
> +++ b/drivers/gpio/gpio-xgene.c
> @@ -159,7 +159,6 @@ static SIMPLE_DEV_PM_OPS(xgene_gpio_pm, xgene_gpio_suspend, xgene_gpio_resume);
>  static int xgene_gpio_probe(struct platform_device *pdev)
>  {
>         struct xgene_gpio *gpio;
> -       int err = 0;
>
>         gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
>         if (!gpio)
> @@ -183,15 +182,7 @@ static int xgene_gpio_probe(struct platform_device *pdev)
>
>         platform_set_drvdata(pdev, gpio);
>
> -       err = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
> -       if (err) {
> -               dev_err(&pdev->dev,
> -                       "failed to register gpiochip.\n");
> -               return err;
> -       }
> -
> -       dev_info(&pdev->dev, "X-Gene GPIO driver registered.\n");
> -       return 0;
> +       return devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
>  }
>
>  static const struct of_device_id xgene_gpio_of_match[] = {
> --
> 2.31.1
>

Applied, thanks.

For the future: the subject should be: "gpio: xgene: ..." here and
everywhere else.

Bart
Alexandru Ardelean May 24, 2021, 7:21 a.m. UTC | #2
On Sun, 23 May 2021 at 21:31, Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
>
> On Fri, May 21, 2021 at 8:10 PM Alexandru Ardelean
> <aardelean@deviqon.com> wrote:
> >
> > The handling of the return value from devm_gpiochip_add_data() is a bit
> > redundant. It prints messages on error and success cases.
> > While the success message may be useful, it is more in the area of log
> > spam, and these can be printed with other forms of kernel logging.
> >
> > This change does a direct return with devm_gpiochip_add_data() in the probe
> > function.
> >
> > The platform_set_drvdata() is needed, as this driver uses the stored
> > private date in the PM suspend/resume routines.
> >
> > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> > ---
> >  drivers/gpio/gpio-xgene.c | 11 +----------
> >  1 file changed, 1 insertion(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-xgene.c b/drivers/gpio/gpio-xgene.c
> > index 532b0df8a1f2..fb4b0c67aeef 100644
> > --- a/drivers/gpio/gpio-xgene.c
> > +++ b/drivers/gpio/gpio-xgene.c
> > @@ -159,7 +159,6 @@ static SIMPLE_DEV_PM_OPS(xgene_gpio_pm, xgene_gpio_suspend, xgene_gpio_resume);
> >  static int xgene_gpio_probe(struct platform_device *pdev)
> >  {
> >         struct xgene_gpio *gpio;
> > -       int err = 0;
> >
> >         gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
> >         if (!gpio)
> > @@ -183,15 +182,7 @@ static int xgene_gpio_probe(struct platform_device *pdev)
> >
> >         platform_set_drvdata(pdev, gpio);
> >
> > -       err = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
> > -       if (err) {
> > -               dev_err(&pdev->dev,
> > -                       "failed to register gpiochip.\n");
> > -               return err;
> > -       }
> > -
> > -       dev_info(&pdev->dev, "X-Gene GPIO driver registered.\n");
> > -       return 0;
> > +       return devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
> >  }
> >
> >  static const struct of_device_id xgene_gpio_of_match[] = {
> > --
> > 2.31.1
> >
>
> Applied, thanks.
>
> For the future: the subject should be: "gpio: xgene: ..." here and
> everywhere else.

ack
will keep that in mind

thanks :)
Alex

>
> Bart

Patch
diff mbox series

diff --git a/drivers/gpio/gpio-xgene.c b/drivers/gpio/gpio-xgene.c
index 532b0df8a1f2..fb4b0c67aeef 100644
--- a/drivers/gpio/gpio-xgene.c
+++ b/drivers/gpio/gpio-xgene.c
@@ -159,7 +159,6 @@  static SIMPLE_DEV_PM_OPS(xgene_gpio_pm, xgene_gpio_suspend, xgene_gpio_resume);
 static int xgene_gpio_probe(struct platform_device *pdev)
 {
 	struct xgene_gpio *gpio;
-	int err = 0;
 
 	gpio = devm_kzalloc(&pdev->dev, sizeof(*gpio), GFP_KERNEL);
 	if (!gpio)
@@ -183,15 +182,7 @@  static int xgene_gpio_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, gpio);
 
-	err = devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
-	if (err) {
-		dev_err(&pdev->dev,
-			"failed to register gpiochip.\n");
-		return err;
-	}
-
-	dev_info(&pdev->dev, "X-Gene GPIO driver registered.\n");
-	return 0;
+	return devm_gpiochip_add_data(&pdev->dev, &gpio->chip, gpio);
 }
 
 static const struct of_device_id xgene_gpio_of_match[] = {