Re: [rft, PATCH v3 1/1] gpiolib: Get rid of not used of_node member

[Andy Shevchenko]

On Wed, Nov 16, 2022 at 11:18:59AM +0200, Andy Shevchenko wrote:
> All new drivers should use fwnode and / or parent to provide the
> necessary information to the GPIO library.
> 
> Cc: Thierry Reding <treding@xxxxxxxxxx>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> 
> v3: rebased against latest Linux Next: expected not to fail now
>     (Also keeping in mind Thierry's report, so reworked a bit)
> v2: resent against latest Linux Next: expected not to fail now
>     (Linux Next has no more users of of_node member of gpio_chip)
> v1: to test for now (using CIs and build bots) what is left unconverted
>     (Expected to fail in some configurations!)
> 
> The idea is to send this after v6.2-rc1, so we will have a full cycle
> to test. Currently one patch is in pin control tree prevents us doing
> this during v6.1 cycle.
> 
>  drivers/gpio/gpiolib-acpi.c | 10 ----------
>  drivers/gpio/gpiolib-acpi.h |  4 ----
>  drivers/gpio/gpiolib-of.c   | 24 ++++--------------------
>  drivers/gpio/gpiolib-of.h   |  5 -----
>  drivers/gpio/gpiolib.c      | 11 +++--------
>  include/linux/gpio/driver.h |  7 -------
>  6 files changed, 7 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index bed0380c5136..466622a8fb36 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -1387,16 +1387,6 @@ void acpi_gpiochip_remove(struct gpio_chip *chip)
>  	kfree(acpi_gpio);
>  }
>  
> -void acpi_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev)
> -{
> -	/* Set default fwnode to parent's one if present */
> -	if (gc->parent)
> -		ACPI_COMPANION_SET(&gdev->dev, ACPI_COMPANION(gc->parent));
> -
> -	if (gc->fwnode)
> -		device_set_node(&gdev->dev, gc->fwnode);
> -}
> -
>  static int acpi_gpio_package_count(const union acpi_object *obj)
>  {
>  	const union acpi_object *element = obj->package.elements;
> diff --git a/drivers/gpio/gpiolib-acpi.h b/drivers/gpio/gpiolib-acpi.h
> index 9475f99a9694..5fa315b3c912 100644
> --- a/drivers/gpio/gpiolib-acpi.h
> +++ b/drivers/gpio/gpiolib-acpi.h
> @@ -26,8 +26,6 @@ struct gpio_device;
>  void acpi_gpiochip_add(struct gpio_chip *chip);
>  void acpi_gpiochip_remove(struct gpio_chip *chip);
>  
> -void acpi_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev);
> -
>  void acpi_gpiochip_request_interrupts(struct gpio_chip *chip);
>  void acpi_gpiochip_free_interrupts(struct gpio_chip *chip);
>  
> @@ -42,8 +40,6 @@ int acpi_gpio_count(struct device *dev, const char *con_id);
>  static inline void acpi_gpiochip_add(struct gpio_chip *chip) { }
>  static inline void acpi_gpiochip_remove(struct gpio_chip *chip) { }
>  
> -static inline void acpi_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev) { }
> -
>  static inline void
>  acpi_gpiochip_request_interrupts(struct gpio_chip *chip) { }
>  
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 4fff7258ee41..00b55dbe0323 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -668,7 +668,7 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
>  	u32 tmp;
>  	int ret;
>  
> -	chip_np = chip->of_node;
> +	chip_np = to_of_node(chip->fwnode);
>  	if (!chip_np)
>  		return ERR_PTR(-EINVAL);
>  
> @@ -760,7 +760,7 @@ static int of_gpiochip_scan_gpios(struct gpio_chip *chip)
>  	struct device_node *np;
>  	int ret;
>  
> -	for_each_available_child_of_node(chip->of_node, np) {
> +	for_each_available_child_of_node(to_of_node(chip->fwnode), np) {
>  		if (!of_property_read_bool(np, "gpio-hog"))
>  			continue;
>  
> @@ -970,7 +970,7 @@ EXPORT_SYMBOL_GPL(of_mm_gpiochip_remove);
>  #ifdef CONFIG_PINCTRL
>  static int of_gpiochip_add_pin_range(struct gpio_chip *chip)
>  {
> -	struct device_node *np = chip->of_node;
> +	struct device_node *np = to_of_node(chip->fwnode);
>  	struct of_phandle_args pinspec;
>  	struct pinctrl_dev *pctldev;
>  	int index = 0, ret;
> @@ -1063,7 +1063,7 @@ int of_gpiochip_add(struct gpio_chip *chip)
>  	struct device_node *np;
>  	int ret;
>  
> -	np = to_of_node(dev_fwnode(&chip->gpiodev->dev));
> +	np = to_of_node(chip->fwnode);
>  	if (!np)
>  		return 0;
>  
> @@ -1092,19 +1092,3 @@ void of_gpiochip_remove(struct gpio_chip *chip)
>  {
>  	fwnode_handle_put(chip->fwnode);
>  }
> -
> -void of_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev)
> -{
> -	/* Set default OF node to parent's one if present */
> -	if (gc->parent)
> -		gdev->dev.of_node = gc->parent->of_node;
> -
> -	if (gc->fwnode)
> -		gc->of_node = to_of_node(gc->fwnode);
> -
> -	/* If the gpiochip has an assigned OF node this takes precedence */
> -	if (gc->of_node)
> -		gdev->dev.of_node = gc->of_node;
> -	else
> -		gc->of_node = gdev->dev.of_node;
> -}
> diff --git a/drivers/gpio/gpiolib-of.h b/drivers/gpio/gpiolib-of.h
> index a6c593e6766c..e5bb065d82ef 100644
> --- a/drivers/gpio/gpiolib-of.h
> +++ b/drivers/gpio/gpiolib-of.h
> @@ -23,7 +23,6 @@ struct gpio_desc *of_find_gpio(struct device_node *np,
>  int of_gpiochip_add(struct gpio_chip *gc);
>  void of_gpiochip_remove(struct gpio_chip *gc);
>  int of_gpio_get_count(struct device *dev, const char *con_id);
> -void of_gpio_dev_init(struct gpio_chip *gc, struct gpio_device *gdev);
>  #else
>  static inline struct gpio_desc *of_find_gpio(struct device_node *np,
>  					     const char *con_id,
> @@ -38,10 +37,6 @@ static inline int of_gpio_get_count(struct device *dev, const char *con_id)
>  {
>  	return 0;
>  }
> -static inline void of_gpio_dev_init(struct gpio_chip *gc,
> -				    struct gpio_device *gdev)
> -{
> -}
>  #endif /* CONFIG_OF_GPIO */
>  
>  extern struct notifier_block gpio_of_notifier;
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 7936d54a2e30..3df482d36366 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -655,10 +655,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
>  	int ret = 0;
>  	u32 ngpios;
>  
> +	/* 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;
>  
>  	/*
>  	 * First: allocate and populate the internal stat container, and
> @@ -672,14 +674,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
>  	gdev->chip = gc;
>  	gc->gpiodev = gdev;
>  
> -	of_gpio_dev_init(gc, gdev);
> -	acpi_gpio_dev_init(gc, gdev);
> -
> -	/*
> -	 * Assign fwnode depending on the result of the previous calls,
> -	 * if none of them succeed, assign it to the parent's one.
> -	 */
> -	gdev->dev.fwnode = dev_fwnode(&gdev->dev) ?: fwnode;
> +	device_set_node(&gdev->dev, gc->fwnode);
>  
>  	gdev->id = ida_alloc(&gpio_ida, GFP_KERNEL);
>  	if (gdev->id < 0) {
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index 2a44600b01f7..ee382712f066 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -503,13 +503,6 @@ struct gpio_chip {
>  	 * the device tree automatically may have an OF translation
>  	 */
>  
> -	/**
> -	 * @of_node:
> -	 *
> -	 * Pointer to a device tree node representing this GPIO controller.
> -	 */
> -	struct device_node *of_node;
> -
>  	/**
>  	 * @of_gpio_n_cells:
>  	 *
> 
> base-commit: 8dab99c9eab3162bfb4326c35579a3388dbf68f2

Uhpffhh, sorry for this, the base is today's Linux Next.

-- 
With Best Regards,
Andy Shevchenko