linux-kernel - Re: [PATCH] gpio: sama5d2-piobu: remove platform_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 8 Jul 2021 10:04:28 +0300
From:   Alexandru Ardelean <aardelean@...iqon.com>
To:     linux-gpio <linux-gpio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Cc:     Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Ludovic Desroches <ludovic.desroches@...rochip.com>
Subject: Re: [PATCH] gpio: sama5d2-piobu: remove platform_set_drvdata() +
 cleanup probe

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