Re: [PATCH] gpio: sama5d2-piobu: remove platform_set_drvdata() + cleanup probe

[Alexandru Ardelean]

On Wed, 7 Jul 2021 at 16:51, Alexandru Ardelean <aardelean@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> ---
>  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
>