mbox series

[v1,0/2] gpiolib: Align prototypes of *gpio_count() APIs

Message ID 20240228184412.3591847-1-andriy.shevchenko@linux.intel.com
Headers show
Series gpiolib: Align prototypes of *gpio_count() APIs | expand

Message

Andy Shevchenko Feb. 28, 2024, 6:40 p.m. UTC
Two out of three GPIO count APIs take device pointer. OF case clearly
does not need it as it immediately switches to device node inside, and
ACPI abstracts that to struct acpi_device pointer. Unify all these by
making them to take struct fwnode_handle pointer. This, in particular,
will allow to create fwnode_gpio_count() API if needed. The need of that
was discussed here [1].

Note, no functional changes intended.

Link: https://lore.kernel.org/r/2ad735ed-963c-4e75-b83e-687ea2c0aef5@alliedtelesis.co.nz [1]

Andy Shevchenko (2):
  gpiolib-of: Make of_gpio_get_count() take firmware node as a parameter
  gpiolib-acpi: Make acpi_gpio_count() take firmware node as a parameter

 drivers/gpio/gpiolib-acpi.c | 13 ++++++-------
 drivers/gpio/gpiolib-acpi.h |  4 ++--
 drivers/gpio/gpiolib-of.c   | 13 ++++++-------
 drivers/gpio/gpiolib-of.h   |  5 +++--
 drivers/gpio/gpiolib.c      |  4 ++--
 5 files changed, 19 insertions(+), 20 deletions(-)

Comments

Chris Packham Feb. 28, 2024, 8:23 p.m. UTC | #1
On 29/02/24 07:40, Andy Shevchenko wrote:
> Make of_gpio_get_count() take firmware node as a parameter in order
> to be aligned with other functions and decouple form unused device
typo: form -> from
> pointer. The latter helps to create a common fwnode_gpio_count()
> in the future.
>
> While at it, rename to be of_gpio_count() to be aligned with the others.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/gpio/gpiolib-of.c | 13 ++++++-------
>   drivers/gpio/gpiolib-of.h |  5 +++--
>   drivers/gpio/gpiolib.c    |  2 +-
>   3 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index e35a9c7da4ee..c0eae8924074 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -68,7 +68,7 @@ static int of_gpio_named_count(const struct device_node *np,
>   
>   /**
>    * of_gpio_spi_cs_get_count() - special GPIO counting for SPI
> - * @dev:    Consuming device
> + * @np:    Consuming device node
>    * @con_id: Function within the GPIO consumer
>    *
>    * Some elder GPIO controllers need special quirks. Currently we handle
> @@ -78,10 +78,8 @@ static int of_gpio_named_count(const struct device_node *np,
>    * the counting of "cs-gpios" to count "gpios" transparent to the
>    * driver.
>    */
> -static int of_gpio_spi_cs_get_count(struct device *dev, const char *con_id)
> +static int of_gpio_spi_cs_get_count(const struct device_node *np, const char *con_id)
>   {
> -	struct device_node *np = dev->of_node;
> -
>   	if (!IS_ENABLED(CONFIG_SPI_MASTER))
>   		return 0;
>   	if (!con_id || strcmp(con_id, "cs"))
> @@ -93,13 +91,14 @@ static int of_gpio_spi_cs_get_count(struct device *dev, const char *con_id)
>   	return of_gpio_named_count(np, "gpios");
>   }
>   
> -int of_gpio_get_count(struct device *dev, const char *con_id)
> +int of_gpio_count(const struct fwnode_handle *fwnode, const char *con_id)
>   {
> +	const struct device_node *np = to_of_node(fwnode);
>   	int ret;
>   	char propname[32];
>   	unsigned int i;
>   
> -	ret = of_gpio_spi_cs_get_count(dev, con_id);
> +	ret = of_gpio_spi_cs_get_count(np, con_id);
>   	if (ret > 0)
>   		return ret;
>   
> @@ -111,7 +110,7 @@ int of_gpio_get_count(struct device *dev, const char *con_id)
>   			snprintf(propname, sizeof(propname), "%s",
>   				 gpio_suffixes[i]);
>   
> -		ret = of_gpio_named_count(dev->of_node, propname);
> +		ret = of_gpio_named_count(np, propname);
>   		if (ret > 0)
>   			break;
>   	}
> diff --git a/drivers/gpio/gpiolib-of.h b/drivers/gpio/gpiolib-of.h
> index 6b3a5347c5d9..19988c1354fa 100644
> --- a/drivers/gpio/gpiolib-of.h
> +++ b/drivers/gpio/gpiolib-of.h
> @@ -9,6 +9,7 @@
>   #include <linux/notifier.h>
>   
>   struct device;
> +struct fwnode_handle;
>   
>   struct gpio_chip;
>   struct gpio_desc;
> @@ -21,7 +22,7 @@ struct gpio_desc *of_find_gpio(struct device_node *np,
>   			       unsigned long *lookupflags);
>   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);
> +int of_gpio_count(const struct fwnode_handle *fwnode, const char *con_id);
>   #else
>   static inline struct gpio_desc *of_find_gpio(struct device_node *np,
>   					     const char *con_id,
> @@ -32,7 +33,7 @@ static inline struct gpio_desc *of_find_gpio(struct device_node *np,
>   }
>   static inline int of_gpiochip_add(struct gpio_chip *gc) { return 0; }
>   static inline void of_gpiochip_remove(struct gpio_chip *gc) { }
> -static inline int of_gpio_get_count(struct device *dev, const char *con_id)
> +static inline int of_gpio_count(const struct fwnode_handle *fwnode, const char *con_id)
>   {
>   	return 0;
>   }
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 5fa3bf7b55bd..a93271b3d538 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -4285,7 +4285,7 @@ int gpiod_count(struct device *dev, const char *con_id)
>   	int count = -ENOENT;
>   
>   	if (is_of_node(fwnode))
> -		count = of_gpio_get_count(dev, con_id);
> +		count = of_gpio_count(fwnode, con_id);
>   	else if (is_acpi_node(fwnode))
>   		count = acpi_gpio_count(dev, con_id);
>   	else if (is_software_node(fwnode))
Linus Walleij Feb. 29, 2024, 2:12 p.m. UTC | #2
On Wed, Feb 28, 2024 at 7:44 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Two out of three GPIO count APIs take device pointer. OF case clearly
> does not need it as it immediately switches to device node inside, and
> ACPI abstracts that to struct acpi_device pointer. Unify all these by
> making them to take struct fwnode_handle pointer. This, in particular,
> will allow to create fwnode_gpio_count() API if needed. The need of that
> was discussed here [1].

This looks reasonable to me:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij