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

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Tue, Apr 11, 2023 at 10:28:06AM +0200, Linus Walleij wrote:
> The code defaulting to the parents fwnode if no fwnode was assigned
> is unnecessarily convoluted, probably due to refactoring.

Yes, the refactoring patches tried to avoid unneeded churn as you now
consolidated into a separate change.

> Simplify
> it and make it more human-readable.

...

> -	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.
> +	 */

I would prefer to have this comment either untouched or being changed where it
appears to be the same (e.g. in IIO core).

> +	if (!gc->fwnode && gc->parent)
> +		gc->fwnode = dev_fwnode(gc->parent);

Otherwise fine by me.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

-- 
With Best Regards,
Andy Shevchenko