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: <20210707135109.191238-1-aardelean@deviqon.com>
In-Reply-To: <20210707135109.191238-1-aardelean@deviqon.com>
From:   Alexandru Ardelean <aardelean@deviqon.com>
Date:   Thu, 8 Jul 2021 10:04:28 +0300
Message-ID: <CAASAkoYknBtci5+Q5=zFdCC8mFaaw7tezfH6pKiAX0vQNKcrAQ@mail.gmail.com>
Subject: Re: [PATCH] gpio: sama5d2-piobu: remove platform_set_drvdata() +
 cleanup probe
To:     linux-gpio <linux-gpio@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Cc:     Bartosz Golaszewski <bgolaszewski@baylibre.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Ludovic Desroches <ludovic.desroches@microchip.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 7 Jul 2021 at 16:51, 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.
>
> This change also changes the probe order a bit, moving the
> devm_gpiochip_add_data() as the last call. This means that when the
> gpiochip is registered [and available to consumers], it should be
> initialized.
>
> It's still possible that the devm_gpiochip_add_data() call could fail,
> leaving the chip in a partially initialized state, but that was possible
> even before this change; it was just some other partially initialized
> state.
>

Please disregard this.
It seems I have not taken into consideration the fact that interrupts
are enabled after the gpiochip is registered.

> Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> ---
>  drivers/gpio/gpio-sama5d2-piobu.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/gpio/gpio-sama5d2-piobu.c b/drivers/gpio/gpio-sama5d2-piobu.c
> index b7c950658170..dfaa069b1466 100644
> --- a/drivers/gpio/gpio-sama5d2-piobu.c
> +++ b/drivers/gpio/gpio-sama5d2-piobu.c
> @@ -189,7 +189,6 @@ static int sama5d2_piobu_probe(struct platform_device *pdev)
>         if (!piobu)
>                 return -ENOMEM;
>
> -       platform_set_drvdata(pdev, piobu);
>         piobu->chip.label = pdev->name;
>         piobu->chip.parent = &pdev->dev;
>         piobu->chip.of_node = pdev->dev.of_node;
> @@ -210,12 +209,6 @@ static int sama5d2_piobu_probe(struct platform_device *pdev)
>                 return PTR_ERR(piobu->regmap);
>         }
>
> -       ret = devm_gpiochip_add_data(&pdev->dev, &piobu->chip, piobu);
> -       if (ret) {
> -               dev_err(&pdev->dev, "Failed to add gpiochip %d\n", ret);
> -               return ret;
> -       }
> -
>         for (i = 0; i < PIOBU_NUM; ++i) {
>                 ret = sama5d2_piobu_setup_pin(&piobu->chip, i);
>                 if (ret) {
> @@ -225,7 +218,7 @@ static int sama5d2_piobu_probe(struct platform_device *pdev)
>                 }
>         }
>
> -       return 0;
> +       return devm_gpiochip_add_data(&pdev->dev, &piobu->chip, piobu);
>  }
>
>  static const struct of_device_id sama5d2_piobu_ids[] = {
> --
> 2.31.1
>