gpiolib: Add and utilise for_each_gpio_property_name()

PATCH Applied Andy Shevchenko 4 participants 9 messages Aug 19, 2024 – Aug 29, 2024

Subsystems (2)

ACPI GPIO

lore

Share & tools

lore

Andy Shevchenko

author

2024-08-19 14:29 UTC #0 lore raw

v2 cover gpiolib: Add and utilise for_each_gpio_property_name()

There are a few of duplication of the same for-loop against GPIO suffixes. This series addresses that along with proposal to eliminate the exported gpio_suffix_count by converting the array to be NULL-terminated.

v2:
- fixed a rebase issue (LKP)

Andy Shevchenko (5):
gpiolib: Introduce for_each_gpio_property_name() helper
gpiolib: swnode: Unify return code variable name
gpiolib: swnode: Introduce swnode_gpio_get_reference() helper
gpiolib: swnode: Make use of for_each_gpio_property_name()
gpiolib: Replace gpio_suffix_count with NULL-terminated array

 drivers/gpio/gpiolib-acpi.c   | 21 ++----------
 drivers/gpio/gpiolib-of.c     | 25 +++------------
 drivers/gpio/gpiolib-swnode.c | 60 ++++++++++++++++-------------------
 drivers/gpio/gpiolib.c        |  3 +-
 drivers/gpio/gpiolib.h        | 16 ++++++++--
 5 files changed, 49 insertions(+), 76 deletions(-)

Signature · 1 line

2.43.0.rc1.1336.g36b5255a03ac

Andy Shevchenko

author

2024-08-19 14:29 UTC #1 lore raw

PATCH v2 1/5 gpiolib: Introduce for_each_gpio_property_name() helper

Introduce a helper macro for_each_gpio_property_name(). With that in place, update users. This, in particular, will help making the following simplifications easier.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/gpio/gpiolib-acpi.c | 21 ++-------------------
 drivers/gpio/gpiolib-of.c   | 25 ++++---------------------
 drivers/gpio/gpiolib.h      | 13 +++++++++++++
 3 files changed, 19 insertions(+), 40 deletions(-)

drivers/gpio/gpiolib-acpi.c +2 ~~-19~~

index 69cd2be9c7f3..cf4b1f068bac 100644
@@ -973,18 +973,9 @@ __acpi_find_gpio(struct fwnode_handle *fwnode, const char *con_id, unsigned int
 	struct acpi_device *adev = to_acpi_device_node(fwnode);
 	struct gpio_desc *desc;
 	char propname[32];
-	int i;
 
 	/* Try first from _DSD */
-	for (i = 0; i < gpio_suffix_count; i++) {
-		if (con_id) {
-			snprintf(propname, sizeof(propname), "%s-%s",
-				 con_id, gpio_suffixes[i]);
-		} else {
-			snprintf(propname, sizeof(propname), "%s",
-				 gpio_suffixes[i]);
-		}
-
+	for_each_gpio_property_name(propname, con_id) {
 		if (adev)
 			desc = acpi_get_gpiod_by_index(adev,
 						       propname, idx, info);

@@ -1450,17 +1441,9 @@ int acpi_gpio_count(const struct fwnode_handle *fwnode, const char *con_id)
 	int count = -ENOENT;
 	int ret;
 	char propname[32];
-	unsigned int i;
 
 	/* Try first from _DSD */
-	for (i = 0; i < gpio_suffix_count; i++) {
-		if (con_id)
-			snprintf(propname, sizeof(propname), "%s-%s",
-				 con_id, gpio_suffixes[i]);
-		else
-			snprintf(propname, sizeof(propname), "%s",
-				 gpio_suffixes[i]);
-
+	for_each_gpio_property_name(propname, con_id) {
 		ret = acpi_dev_get_property(adev, propname, ACPI_TYPE_ANY, &obj);
 		if (ret == 0) {
 			if (obj->type == ACPI_TYPE_LOCAL_REFERENCE)

drivers/gpio/gpiolib-of.c +4 ~~-21~~

index 6683e531df52..92984f6d2aa8 100644
@@ -97,20 +97,12 @@ 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(np, con_id);
 	if (ret > 0)
 		return ret;
 
-	for (i = 0; i < gpio_suffix_count; i++) {
-		if (con_id)
-			snprintf(propname, sizeof(propname), "%s-%s",
-				 con_id, gpio_suffixes[i]);
-		else
-			snprintf(propname, sizeof(propname), "%s",
-				 gpio_suffixes[i]);
-
+	for_each_gpio_property_name(propname, con_id) {
 		ret = of_gpio_named_count(np, propname);
 		if (ret > 0)
 			break;

@@ -687,23 +679,14 @@ static const of_find_gpio_quirk of_find_gpio_quirks[] = {
 struct gpio_desc *of_find_gpio(struct device_node *np, const char *con_id,
 			       unsigned int idx, unsigned long *flags)
 {
-	char prop_name[32]; /* 32 is max size of property name */
+	char propname[32]; /* 32 is max size of property name */
 	enum of_gpio_flags of_flags;
 	const of_find_gpio_quirk *q;
 	struct gpio_desc *desc;
-	unsigned int i;
 
 	/* Try GPIO property "foo-gpios" and "foo-gpio" */
-	for (i = 0; i < gpio_suffix_count; i++) {
-		if (con_id)
-			snprintf(prop_name, sizeof(prop_name), "%s-%s", con_id,
-				 gpio_suffixes[i]);
-		else
-			snprintf(prop_name, sizeof(prop_name), "%s",
-				 gpio_suffixes[i]);
-
-		desc = of_get_named_gpiod_flags(np, prop_name, idx, &of_flags);
-
+	for_each_gpio_property_name(propname, con_id) {
+		desc = of_get_named_gpiod_flags(np, propname, idx, &of_flags);
 		if (!gpiod_not_found(desc))
 			break;
 	}

drivers/gpio/gpiolib.h +13 -0

index 4de0bf1a62d3..0271e747fb6e 100644
@@ -93,6 +93,19 @@ static inline struct gpio_device *to_gpio_device(struct device *dev)
 extern const char *const gpio_suffixes[];
 extern const size_t gpio_suffix_count;
 
+#define for_each_gpio_property_name(propname, con_id)					\
+	for (unsigned int __i = 0;							\
+	     __i < gpio_suffix_count && ({						\
+		     const char *__gs = gpio_suffixes[__i];				\
+											\
+		if (con_id)								\
+			snprintf(propname, sizeof(propname), "%s-%s", con_id, __gs);	\
+		else									\
+			snprintf(propname, sizeof(propname), "%s", __gs);		\
+		1;									\
+	     });									\
+	     __i++)
+
 /**
  * struct gpio_array - Opaque descriptor for a structure of GPIO array attributes
  *

Signature · 1 line

2.43.0.rc1.1336.g36b5255a03ac

Andy Shevchenko

author

2024-08-19 14:29 UTC #2 lore raw

PATCH v2 2/5 gpiolib: swnode: Unify return code variable name

In reply to Andy Shevchenko

In one case 'ret' is used in the other 'error'. Make the latter use the former, i.e. 'ret'.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/gpio/gpiolib-swnode.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/gpio/gpiolib-swnode.c +4 -4

index cec1ab878af8..e7ba6cc73966 100644
@@ -67,7 +67,7 @@ struct gpio_desc *swnode_find_gpio(struct fwnode_handle *fwnode,
 	struct fwnode_reference_args args;
 	struct gpio_desc *desc;
 	char propname[32]; /* 32 is max size of property name */
-	int error;
+	int ret;
 
 	swnode = to_software_node(fwnode);
 	if (!swnode)

@@ -79,11 +79,11 @@ struct gpio_desc *swnode_find_gpio(struct fwnode_handle *fwnode,
 	 * We expect all swnode-described GPIOs have GPIO number and
 	 * polarity arguments, hence nargs is set to 2.
 	 */
-	error = fwnode_property_get_reference_args(fwnode, propname, NULL, 2, idx, &args);
-	if (error) {
+	ret = fwnode_property_get_reference_args(fwnode, propname, NULL, 2, idx, &args);
+	if (ret) {
 		pr_debug("%s: can't parse '%s' property of node '%pfwP[%d]'\n",
 			__func__, propname, fwnode, idx);
-		return ERR_PTR(error);
+		return ERR_PTR(ret);
 	}
 
 	struct gpio_device *gdev __free(gpio_device_put) =

Signature · 1 line

2.43.0.rc1.1336.g36b5255a03ac

Andy Shevchenko

author

2024-08-19 14:29 UTC #3 lore raw

PATCH v2 3/5 gpiolib: swnode: Introduce swnode_gpio_get_reference() helper

In reply to Andy Shevchenko

Instead of the spreading simlar code over the file, introduce a helper. It also enforces the nargs validation for all GPIO software node APIs.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/gpio/gpiolib-swnode.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

drivers/gpio/gpiolib-swnode.c +13 -7

index e7ba6cc73966..d5e58a9673b5 100644
@@ -59,6 +59,17 @@ static struct gpio_device *swnode_get_gpio_device(struct fwnode_handle *fwnode)
 	return gdev ?: ERR_PTR(-EPROBE_DEFER);
 }
 
+static int swnode_gpio_get_reference(const struct fwnode_handle *fwnode,
+				     const char *propname, unsigned int idx,
+				     struct fwnode_reference_args *args)
+{
+	/*
+	 * We expect all swnode-described GPIOs have GPIO number and
+	 * polarity arguments, hence nargs is set to 2.
+	 */
+	return fwnode_property_get_reference_args(fwnode, propname, NULL, 2, idx, args);
+}
+
 struct gpio_desc *swnode_find_gpio(struct fwnode_handle *fwnode,
 				   const char *con_id, unsigned int idx,
 				   unsigned long *flags)

@@ -75,11 +86,7 @@ struct gpio_desc *swnode_find_gpio(struct fwnode_handle *fwnode,
 
 	swnode_format_propname(con_id, propname, sizeof(propname));
 
-	/*
-	 * We expect all swnode-described GPIOs have GPIO number and
-	 * polarity arguments, hence nargs is set to 2.
-	 */
-	ret = fwnode_property_get_reference_args(fwnode, propname, NULL, 2, idx, &args);
+	ret = swnode_gpio_get_reference(fwnode, propname, idx, &args);
 	if (ret) {
 		pr_debug("%s: can't parse '%s' property of node '%pfwP[%d]'\n",
 			__func__, propname, fwnode, idx);

@@ -128,8 +135,7 @@ int swnode_gpio_count(const struct fwnode_handle *fwnode, const char *con_id)
 	 * 1 or 2 entries.
 	 */
 	count = 0;
-	while (fwnode_property_get_reference_args(fwnode, propname, NULL, 0,
-						  count, &args) == 0) {
+	while (swnode_gpio_get_reference(fwnode, propname, count, &args) == 0) {
 		fwnode_handle_put(args.fwnode);
 		count++;
 	}

Signature · 1 line

2.43.0.rc1.1336.g36b5255a03ac

Andy Shevchenko

author

2024-08-19 14:29 UTC #4 lore raw

PATCH v2 4/5 gpiolib: swnode: Make use of for_each_gpio_property_name()

In reply to Andy Shevchenko

For the sake of unification and easier maintenance replace swnode_format_propname() call with for_each_gpio_property_name() for-loop.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/gpio/gpiolib-swnode.c | 38 +++++++++++++----------------------
 1 file changed, 14 insertions(+), 24 deletions(-)

drivers/gpio/gpiolib-swnode.c +14 ~~-24~~

index d5e58a9673b5..1a6f70671816 100644
@@ -24,20 +24,6 @@
 
 #define GPIOLIB_SWNODE_UNDEFINED_NAME "swnode-gpio-undefined"
 
-static void swnode_format_propname(const char *con_id, char *propname,
-				   size_t max_size)
-{
-	/*
-	 * Note we do not need to try both -gpios and -gpio suffixes,
-	 * as, unlike OF and ACPI, we can fix software nodes to conform
-	 * to the proper binding.
-	 */
-	if (con_id)
-		snprintf(propname, max_size, "%s-gpios", con_id);
-	else
-		strscpy(propname, "gpios", max_size);
-}
-
 static struct gpio_device *swnode_get_gpio_device(struct fwnode_handle *fwnode)
 {
 	const struct software_node *gdev_node;

@@ -84,9 +70,11 @@ struct gpio_desc *swnode_find_gpio(struct fwnode_handle *fwnode,
 	if (!swnode)
 		return ERR_PTR(-EINVAL);
 
-	swnode_format_propname(con_id, propname, sizeof(propname));
-
-	ret = swnode_gpio_get_reference(fwnode, propname, idx, &args);
+	for_each_gpio_property_name(propname, con_id) {
+		ret = swnode_gpio_get_reference(fwnode, propname, idx, &args);
+		if (ret == 0)
+			break;
+	}
 	if (ret) {
 		pr_debug("%s: can't parse '%s' property of node '%pfwP[%d]'\n",
 			__func__, propname, fwnode, idx);

@@ -128,19 +116,21 @@ int swnode_gpio_count(const struct fwnode_handle *fwnode, const char *con_id)
 	char propname[32];
 	int count;
 
-	swnode_format_propname(con_id, propname, sizeof(propname));
-
 	/*
 	 * This is not very efficient, but GPIO lists usually have only
 	 * 1 or 2 entries.
 	 */
-	count = 0;
-	while (swnode_gpio_get_reference(fwnode, propname, count, &args) == 0) {
-		fwnode_handle_put(args.fwnode);
-		count++;
+	for_each_gpio_property_name(propname, con_id) {
+		count = 0;
+		while (swnode_gpio_get_reference(fwnode, propname, count, &args) == 0) {
+			fwnode_handle_put(args.fwnode);
+			count++;
+		}
+		if (count)
+			return count;
 	}
 
-	return count ?: -ENOENT;
+	return -ENOENT;
 }
 
 #if IS_ENABLED(CONFIG_GPIO_SWNODE_UNDEFINED)

Signature · 1 line

2.43.0.rc1.1336.g36b5255a03ac

Andy Shevchenko

author

2024-08-19 14:29 UTC #5 lore raw

PATCH v2 5/5 gpiolib: Replace gpio_suffix_count with NULL-terminated array

In reply to Andy Shevchenko

There is no need to have and export the count variable for the array in question. Instead, make it NULL-terminated.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/gpio/gpiolib.c |  3 +--
 drivers/gpio/gpiolib.h | 11 +++++------
 2 files changed, 6 insertions(+), 8 deletions(-)

drivers/gpio/gpiolib.c +1 -2

index 3a9668cc100d..3903d0a75304 100644
@@ -90,8 +90,7 @@ DEFINE_STATIC_SRCU(gpio_devices_srcu);
 static DEFINE_MUTEX(gpio_machine_hogs_mutex);
 static LIST_HEAD(gpio_machine_hogs);
 
-const char *const gpio_suffixes[] = { "gpios", "gpio" };
-const size_t gpio_suffix_count = ARRAY_SIZE(gpio_suffixes);
+const char *const gpio_suffixes[] = { "gpios", "gpio", NULL };
 
 static void gpiochip_free_hogs(struct gpio_chip *gc);
 static int gpiochip_add_irqchip(struct gpio_chip *gc,

drivers/gpio/gpiolib.h +5 -6

index 0271e747fb6e..067197d61d57 100644
@@ -89,14 +89,13 @@ static inline struct gpio_device *to_gpio_device(struct device *dev)
 	return container_of(dev, struct gpio_device, dev);
 }
 
-/* gpio suffixes used for ACPI and device tree lookup */
+/* GPIO suffixes used for ACPI and device tree lookup */
 extern const char *const gpio_suffixes[];
-extern const size_t gpio_suffix_count;
 
 #define for_each_gpio_property_name(propname, con_id)					\
-	for (unsigned int __i = 0;							\
-	     __i < gpio_suffix_count && ({						\
-		     const char *__gs = gpio_suffixes[__i];				\
+	for (const char * const *__suffixes = gpio_suffixes;				\
+	     *__suffixes && ({								\
+		const char *__gs = *__suffixes;						\
 											\
 		if (con_id)								\
 			snprintf(propname, sizeof(propname), "%s-%s", con_id, __gs);	\

@@ -104,7 +103,7 @@ extern const size_t gpio_suffix_count;
 			snprintf(propname, sizeof(propname), "%s", __gs);		\
 		1;									\
 	     });									\
-	     __i++)
+	     __suffixes++)
 
 /**
  * struct gpio_array - Opaque descriptor for a structure of GPIO array attributes

Signature · 1 line

2.43.0.rc1.1336.g36b5255a03ac

Bartosz Golaszewski

Applied

2024-08-20 08:33 UTC #6 lore raw

In reply to Andy Shevchenko

From: Bartosz Golaszewski <[email protected]>

Quoted 9 lines · Andy Shevchenko

There are a few of duplication of the same for-loop against GPIO suffixes. This series addresses that along with proposal to eliminate the exported gpio_suffix_count by converting the array to be NULL-terminated.
v2:
- fixed a rebase issue (LKP)
[...]

Applied, thanks!

[1/5] gpiolib: Introduce for_each_gpio_property_name() helper

      commit: ef3d4b94d2d88b160887ff9ca737a5f8ec101579
[2/5] gpiolib: swnode: Unify return code variable name
      commit: e42fce0ff99658b5b43e8dae4f7acc43d38a00ef
[3/5] gpiolib: swnode: Introduce swnode_gpio_get_reference() helper
      commit: 7fd6809888a82055fcca9d14417d5e2675f0acc5
[4/5] gpiolib: swnode: Make use of for_each_gpio_property_name()
      commit: a975a64692c39991fdde2f1d990b7bdd48d183fc
[5/5] gpiolib: Replace gpio_suffix_count with NULL-terminated array
      commit: 4b91188dced811e2d867574b672888406cb7114c

Best regards,

Signature · 1 line

Bartosz Golaszewski <[email protected]>

Linus Walleij

2024-08-24 14:43 UTC #7 lore raw

In reply to Andy Shevchenko

Quoted 4 lines · Andy Shevchenko

There are a few of duplication of the same for-loop against GPIO suffixes. This series addresses that along with proposal to eliminate the exported gpio_suffix_count by converting the array to be NULL-terminated.

Too late to the show, but excellent patches Andy!

Yours,
Linus Walleij

Rafael J. Wysocki

maintainer Acked-by on the v2 series

2024-08-29 10:45 UTC #8 lore raw

In reply to Andy Shevchenko

Quoted 8 lines · Andy Shevchenko

There are a few of duplication of the same for-loop against GPIO suffixes. This series addresses that along with proposal to eliminate the exported gpio_suffix_count by converting the array to be NULL-terminated.
v2:
- fixed a rebase issue (LKP)

All patches in this set look good to me:

Acked-by Rafael J. Wysocki <[email protected]>

Quoted 18 lines

Andy Shevchenko (5):
gpiolib: Introduce for_each_gpio_property_name() helper
gpiolib: swnode: Unify return code variable name
gpiolib: swnode: Introduce swnode_gpio_get_reference() helper
gpiolib: swnode: Make use of for_each_gpio_property_name()
gpiolib: Replace gpio_suffix_count with NULL-terminated array
 drivers/gpio/gpiolib-acpi.c   | 21 ++----------
 drivers/gpio/gpiolib-of.c     | 25 +++------------
 drivers/gpio/gpiolib-swnode.c | 60 ++++++++++++++++-------------------
 drivers/gpio/gpiolib.c        |  3 +-
 drivers/gpio/gpiolib.h        | 16 ++++++++--
 5 files changed, 49 insertions(+), 76 deletions(-)
--
2.43.0.rc1.1336.g36b5255a03ac