Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210514094108.28890-1-aardelean@deviqon.com> <CAMpxmJWmc-8cCZ5EQcnBKSAmXPbcVUnX7GdhKgKWFp5i=B6y1w@mail.gmail.com>
In-Reply-To: <CAMpxmJWmc-8cCZ5EQcnBKSAmXPbcVUnX7GdhKgKWFp5i=B6y1w@mail.gmail.com>
From:   Alexandru Ardelean <aardelean@deviqon.com>
Date:   Fri, 21 May 2021 20:46:03 +0300
Message-ID: <CAASAkobrt=t0ObR+_nn6g403VAMP3f7uh7VmVDAo7S0PgpLE-A@mail.gmail.com>
Subject: Re: [PATCH] gpio: gpio-tps68470: remove platform_set_drvdata() +
 cleanup probe
To:     Bartosz Golaszewski <bgolaszewski@baylibre.com>
Cc:     linux-gpio <linux-gpio@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 21 May 2021 at 16:52, Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
>
> On Fri, May 14, 2021 at 11:41 AM Alexandru Ardelean
> <aardelean@deviqon.com> wrote:
> >
> > The platform_set_drvdata() call is only useful if we need to retrieve back
> > the private information.
> > Since the driver doesn't do that, it's not useful to have it.
> >
> > If this is removed, we can also just do a direct return on
> > devm_gpiochip_add_data(). We don't need to print that this call failed as
> > there are other ways to log/see this during probe.
> >
> > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> > ---
> >  drivers/gpio/gpio-tps68470.c | 12 +-----------
> >  1 file changed, 1 insertion(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpio/gpio-tps68470.c b/drivers/gpio/gpio-tps68470.c
> > index f7f5f770e0fb..423b7bc30ae8 100644
> > --- a/drivers/gpio/gpio-tps68470.c
> > +++ b/drivers/gpio/gpio-tps68470.c
> > @@ -125,7 +125,6 @@ static const char *tps68470_names[TPS68470_N_GPIO] = {
> >  static int tps68470_gpio_probe(struct platform_device *pdev)
> >  {
> >         struct tps68470_gpio_data *tps68470_gpio;
> > -       int ret;
> >
> >         tps68470_gpio = devm_kzalloc(&pdev->dev, sizeof(*tps68470_gpio),
> >                                      GFP_KERNEL);
> > @@ -146,16 +145,7 @@ static int tps68470_gpio_probe(struct platform_device *pdev)
> >         tps68470_gpio->gc.base = -1;
> >         tps68470_gpio->gc.parent = &pdev->dev;
> >
> > -       ret = devm_gpiochip_add_data(&pdev->dev, &tps68470_gpio->gc,
> > -                                    tps68470_gpio);
> > -       if (ret < 0) {
> > -               dev_err(&pdev->dev, "Failed to register gpio_chip: %d\n", ret);
> > -               return ret;
> > -       }
> > -
> > -       platform_set_drvdata(pdev, tps68470_gpio);
> > -
> > -       return ret;
> > +       return devm_gpiochip_add_data(&pdev->dev, &tps68470_gpio->gc, tps68470_gpio);
> >  }
> >
> >  static struct platform_driver tps68470_gpio_driver = {
> > --
> > 2.31.1
> >
>
> Applied. I got confused by the dev_get_drvdata() call earlier in probe
> but this one's for the parent.

Yes.
I initially got confused as well about it.

This pattern seems to be similar in other drivers as well; usually MFD devices.

>
> Thanks,
> Bart