Re: [PATCH] gpio: gpiolib: Simplify gpiochip_add_data_with_key() fwnode

[Anders Roxell <anders.roxell@xxxxxxxxxx>]

On Tue, 11 Apr 2023 at 10:28, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> The code defaulting to the parents fwnode if no fwnode was assigned
> is unnecessarily convoluted, probably due to refactoring. Simplify
> it and make it more human-readable.
>
> Cc: Anders Roxell <anders.roxell@xxxxxxxxxx>
> Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>

Reviewed-by: Anders Roxell <anders.roxell@xxxxxxxxxx>

> ---
> Anders: you can test this but I don't think it fixes the
> regression you have pointing to commit
> 24c94060fc9b4e0f19e6e018869db46db21d6bc7

Did not fix the issue though.


Cheers,
Anders



> ---
>  drivers/gpio/gpiolib.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 19bd23044b01..5801d682c12b 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -667,7 +667,6 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
>                                struct lock_class_key *lock_key,
>                                struct lock_class_key *request_key)
>  {
> -       struct fwnode_handle *fwnode = NULL;
>         struct gpio_device *gdev;
>         unsigned long flags;
>         unsigned int i;
> @@ -675,12 +674,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
>         int base = 0;
>         int ret = 0;
>
> -       /* If the calling driver did not initialize firmware node, do it here */
> -       if (gc->fwnode)
> -               fwnode = gc->fwnode;
> -       else if (gc->parent)
> -               fwnode = dev_fwnode(gc->parent);
> -       gc->fwnode = fwnode;
> +       /*
> +        * If the calling driver did not initialize firmware node, do it here
> +        * using the parent device, if any.
> +        */
> +       if (!gc->fwnode && gc->parent)
> +               gc->fwnode = dev_fwnode(gc->parent);
>
>         /*
>          * First: allocate and populate the internal stat container, and
> --
> 2.39.2
>