2024-02-28 18:44:29

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 0/2] gpiolib: Align prototypes of *gpio_count() APIs

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/[email protected] [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(-)

--
2.43.0.rc1.1.gbec44491f096



2024-02-28 18:44:46

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 2/2] gpiolib-acpi: Make acpi_gpio_count() take firmware node as a parameter

Make acpi_gpio_count() take firmware node as a parameter in order
to be aligned with other functions and decouple form unused device
pointer. The latter helps to create a common fwnode_gpio_count()
in the future.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/gpio/gpiolib-acpi.c | 13 ++++++-------
drivers/gpio/gpiolib-acpi.h | 4 ++--
drivers/gpio/gpiolib.c | 2 +-
3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 899cd505073e..7f140df40f35 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -1402,17 +1402,17 @@ static int acpi_find_gpio_count(struct acpi_resource *ares, void *data)
}

/**
- * acpi_gpio_count - count the GPIOs associated with a device / function
- * @dev: GPIO consumer, can be %NULL for system-global GPIOs
+ * acpi_gpio_count - count the GPIOs associated with a firmware node / function
+ * @fwnode: firmware node of the GPIO consumer
* @con_id: function within the GPIO consumer
*
* Return:
- * The number of GPIOs associated with a device / function or %-ENOENT,
+ * The number of GPIOs associated with a firmware node / function or %-ENOENT,
* if no GPIO has been assigned to the requested function.
*/
-int acpi_gpio_count(struct device *dev, const char *con_id)
+int acpi_gpio_count(const struct fwnode_handle *fwnode, const char *con_id)
{
- struct acpi_device *adev = ACPI_COMPANION(dev);
+ struct acpi_device *adev = to_acpi_device_node(fwnode);
const union acpi_object *obj;
const struct acpi_gpio_mapping *gm;
int count = -ENOENT;
@@ -1429,8 +1429,7 @@ int acpi_gpio_count(struct device *dev, const char *con_id)
snprintf(propname, sizeof(propname), "%s",
gpio_suffixes[i]);

- ret = acpi_dev_get_property(adev, propname, ACPI_TYPE_ANY,
- &obj);
+ ret = acpi_dev_get_property(adev, propname, ACPI_TYPE_ANY, &obj);
if (ret == 0) {
if (obj->type == ACPI_TYPE_LOCAL_REFERENCE)
count = 1;
diff --git a/drivers/gpio/gpiolib-acpi.h b/drivers/gpio/gpiolib-acpi.h
index 0fcd7e14d7f9..6f295ea580fe 100644
--- a/drivers/gpio/gpiolib-acpi.h
+++ b/drivers/gpio/gpiolib-acpi.h
@@ -33,7 +33,7 @@ struct gpio_desc *acpi_find_gpio(struct fwnode_handle *fwnode,
enum gpiod_flags *dflags,
unsigned long *lookupflags);

-int acpi_gpio_count(struct device *dev, const char *con_id);
+int acpi_gpio_count(const struct fwnode_handle *fwnode, const char *con_id);
#else
static inline void acpi_gpiochip_add(struct gpio_chip *chip) { }
static inline void acpi_gpiochip_remove(struct gpio_chip *chip) { }
@@ -51,7 +51,7 @@ acpi_find_gpio(struct fwnode_handle *fwnode, const char *con_id,
{
return ERR_PTR(-ENOENT);
}
-static inline int acpi_gpio_count(struct device *dev, const char *con_id)
+static inline int acpi_gpio_count(const struct fwnode_handle *fwnode, const char *con_id)
{
return -ENODEV;
}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index a93271b3d538..e6c5c7894553 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4287,7 +4287,7 @@ int gpiod_count(struct device *dev, const char *con_id)
if (is_of_node(fwnode))
count = of_gpio_count(fwnode, con_id);
else if (is_acpi_node(fwnode))
- count = acpi_gpio_count(dev, con_id);
+ count = acpi_gpio_count(fwnode, con_id);
else if (is_software_node(fwnode))
count = swnode_gpio_count(fwnode, con_id);

--
2.43.0.rc1.1.gbec44491f096


2024-02-28 22:21:59

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/2] gpiolib-of: Make of_gpio_get_count() take firmware node as a parameter

Make of_gpio_get_count() take firmware node as a parameter in order
to be aligned with other functions and decouple form unused device
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 <[email protected]>
---
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))
--
2.43.0.rc1.1.gbec44491f096


2024-02-29 14:12:25

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH v1 0/2] gpiolib: Align prototypes of *gpio_count() APIs

On Wed, Feb 28, 2024 at 7:44 PM Andy Shevchenko
<[email protected]> 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 <[email protected]>

Yours,
Linus Walleij

2024-02-29 22:05:08

by Chris Packham

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] gpiolib-of: Make of_gpio_get_count() take firmware node as a parameter


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 <[email protected]>
> ---
> 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))