mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib
@ 2023-06-14 10:07 Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 01/10] gpiolib: fix gpio-hog functionality Ahmad Fatoum
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 10:07 UTC (permalink / raw)
  To: barebox; +Cc: mfe, Jules Maselbas

Quoting v1:

  the purpose of this series is to fix the gpio-hogs mechanism since this
  is broken since commit 3641d381e6 ("gpiolib: Add of_xlate support").
  
  Patch1: Revert the above mentioned commit to make the gpio-hogs working
          again.
  
  Patch2-10: Add the of_xlate support required for the upcoming sunxi
             pinctrl driver. The mechanism is now more in sync with kernel
             gpiolib. This allows to sync easier with the kernel gpiolib
             in case of new features or fixes.

I've incorporated feedback and tested this still works on a i.mx8mn.
Patch 1 should be ok to go into master and the rest to next IMO.

Changelog in each patch.

Marco Felsch (10):
  gpiolib: fix gpio-hog functionality
  gpiolib: simplify for loop break condition
  gpiolib: rename local gpio-line-names variable
  gpiolib: fix gpio name memory leak
  gpiolib: fix missing error check while query gpio-line-names
  gpiolib: refactor gpio-line-names parsing
  gpiolib: introduce of_gpiochip_add to bundle all of functions
  OF: gpio: sync of_get_named_gpio_flags variable with kernel
  OF: gpio: call of_node_put in of_get_named_gpio_flags
  gpiolib: add of_xlate support

 drivers/gpio/gpiolib.c | 207 ++++++++++++++++++++++++++++++-----------
 drivers/of/of_gpio.c   |  69 ++++++++++----
 include/gpio.h         |  30 +++++-
 3 files changed, 231 insertions(+), 75 deletions(-)

-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 01/10] gpiolib: fix gpio-hog functionality
  2023-06-14 10:07 [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Ahmad Fatoum
@ 2023-06-14 10:07 ` Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 02/10] gpiolib: simplify for loop break condition Ahmad Fatoum
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 10:07 UTC (permalink / raw)
  To: barebox; +Cc: mfe, Jules Maselbas, Ahmad Fatoum

From: Marco Felsch <m.felsch@pengutronix.de>

This reverts commit 3641d381e63321016e3bf09504852a6b2a2f879b.

Since the of_xlate support the gpio-hog support is broken because the
'gpio' property used to specify the gpio-hog pin does not contain any
phandle. Due to the fact that of_xlate was never implemented the easiest
way to fix the gpio-hog functionality is to revert the commit.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Fixes gpio-hog regression in master. sunxi problem is fixed in the
rest of series, but that's not upstream yet, so can go through next.

v1 -> v2:
  no change
---
 drivers/gpio/gpiolib.c | 64 ++++++++++++++++++++----------------------
 drivers/of/of_gpio.c   |  5 ++--
 include/gpio.h         |  4 +--
 3 files changed, 33 insertions(+), 40 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 6ec63b1119b5..b4a3a4e5504c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -444,28 +444,45 @@ static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
 		       unsigned int idx)
 {
 	struct device_node *chip_np = chip->dev->of_node;
-	struct of_phandle_args gpiospec;
 	unsigned long flags = 0;
-	u32 gpio_flags;
+	u32 gpio_cells, gpio_num, gpio_flags;
 	int ret, gpio;
 	const char *name = NULL;
 
-	ret = of_parse_phandle_with_args(chip_np, "gpios", "#gpio-cells", idx,
-					&gpiospec);
+	ret = of_property_read_u32(chip_np, "#gpio-cells", &gpio_cells);
 	if (ret)
 		return ret;
 
-	gpio = gpio_of_xlate(chip->dev, &gpiospec, &gpio_flags);
-	if (gpio == -EPROBE_DEFER)
-		return gpio;
-	if (gpio < 0) {
-		dev_err(chip->dev, "unable to get gpio: %d\n", gpio);
-		return gpio;
-	}
+	/*
+	 * Support for GPIOs that don't have #gpio-cells set to 2 is
+	 * not implemented
+	 */
+	if (WARN_ON(gpio_cells != 2))
+		return -ENOTSUPP;
+
+	ret = of_property_read_u32_index(np, "gpios", idx * gpio_cells,
+					 &gpio_num);
+	if (ret)
+		return ret;
+
+	ret = of_property_read_u32_index(np, "gpios", idx * gpio_cells + 1,
+					 &gpio_flags);
+	if (ret)
+		return ret;
 
 	if (gpio_flags & OF_GPIO_ACTIVE_LOW)
 		flags |= GPIOF_ACTIVE_LOW;
 
+	gpio = gpio_get_num(chip->dev, gpio_num);
+	if (gpio == -EPROBE_DEFER)
+		return gpio;
+
+	if (gpio < 0) {
+		dev_err(chip->dev, "unable to get gpio %u\n", gpio_num);
+		return gpio;
+	}
+
+
 	/*
 	 * Note that, in order to be compatible with Linux, the code
 	 * below interprets 'output-high' as to mean 'output-active'.
@@ -625,23 +642,6 @@ void gpiochip_remove(struct gpio_chip *chip)
 	list_del(&chip->list);
 }
 
-static int of_gpio_simple_xlate(struct gpio_chip *chip,
-				const struct of_phandle_args *gpiospec,
-				u32 *flags)
-{
-	/*
-	 * Support for GPIOs that don't have #gpio-cells set to 2 is
-	 * not implemented
-	 */
-	if (WARN_ON(gpiospec->args_count != 2))
-		return -ENOTSUPP;
-
-	if (flags)
-		*flags = gpiospec->args[1];
-
-	return chip->base + gpiospec->args[0];
-}
-
 struct gpio_chip *gpio_get_chip_by_dev(struct device *dev)
 {
 	struct gpio_chip *chip;
@@ -654,8 +654,7 @@ struct gpio_chip *gpio_get_chip_by_dev(struct device *dev)
 	return NULL;
 }
 
-int gpio_of_xlate(struct device *dev, struct of_phandle_args *gpiospec,
-		  int *flags)
+int gpio_get_num(struct device *dev, int gpio)
 {
 	struct gpio_chip *chip;
 
@@ -666,10 +665,7 @@ int gpio_of_xlate(struct device *dev, struct of_phandle_args *gpiospec,
 	if (!chip)
 		return -EPROBE_DEFER;
 
-	if (chip->ops->of_xlate)
-		return chip->ops->of_xlate(chip, gpiospec, flags);
-	else
-		return of_gpio_simple_xlate(chip, gpiospec, flags);
+	return chip->base + gpio;
 }
 
 struct gpio_chip *gpio_get_chip(int gpio)
diff --git a/drivers/of/of_gpio.c b/drivers/of/of_gpio.c
index b6625637421b..5da80e249386 100644
--- a/drivers/of/of_gpio.c
+++ b/drivers/of/of_gpio.c
@@ -62,7 +62,6 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
 {
 	struct of_phandle_args out_args;
 	struct device *dev;
-	int of_flags;
 	int ret;
 
 	ret = of_parse_phandle_with_args(np, propname, "#gpio-cells",
@@ -80,7 +79,7 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
 		return -EPROBE_DEFER;
 	}
 
-	ret = gpio_of_xlate(dev, &out_args, &of_flags);
+	ret = gpio_get_num(dev, out_args.args[0]);
 	if (ret == -EPROBE_DEFER)
 		return ret;
 	if (ret < 0) {
@@ -90,7 +89,7 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
 	}
 
 	if (flags) {
-		*flags = of_flags;
+		*flags = out_args.args[1];
 		of_gpio_flags_quirks(np, propname, flags, index);
 	}
 
diff --git a/include/gpio.h b/include/gpio.h
index 10ac7fd7c4cb..5f2c16584c3b 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -177,7 +177,6 @@ struct gpio_ops {
 	int (*get_direction)(struct gpio_chip *chip, unsigned offset);
 	int (*get)(struct gpio_chip *chip, unsigned offset);
 	void (*set)(struct gpio_chip *chip, unsigned offset, int value);
-	int (*of_xlate)(struct gpio_chip *chip, const struct of_phandle_args *gpiospec, u32 *flags);
 };
 
 struct gpio_chip {
@@ -194,8 +193,7 @@ struct gpio_chip {
 int gpiochip_add(struct gpio_chip *chip);
 void gpiochip_remove(struct gpio_chip *chip);
 
-int gpio_of_xlate(struct device *dev, struct of_phandle_args *gpiospec,
-		  int *flags);
+int gpio_get_num(struct device *dev, int gpio);
 struct gpio_chip *gpio_get_chip(int gpio);
 
 #endif /* __GPIO_H */
-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 02/10] gpiolib: simplify for loop break condition
  2023-06-14 10:07 [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 01/10] gpiolib: fix gpio-hog functionality Ahmad Fatoum
@ 2023-06-14 10:07 ` Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 03/10] gpiolib: rename local gpio-line-names variable Ahmad Fatoum
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 10:07 UTC (permalink / raw)
  To: barebox; +Cc: mfe, Jules Maselbas, Ahmad Fatoum

From: Marco Felsch <m.felsch@pengutronix.de>

No functional change just a simplification to align it with the Linux
kernel. Also add the kernel comment to make the reason clear.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - no change
---
 drivers/gpio/gpiolib.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b4a3a4e5504c..0df43c9f8f7e 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -529,7 +529,15 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
 		of_property_read_string_array(chip->dev->of_node,
 					      "gpio-line-names", arr, count);
 
-		for (i = 0; i < chip->ngpio && i < count; i++)
+		/*
+		 * Since property 'gpio-line-names' cannot contains gaps, we
+		 * have to be sure we only assign those pins that really exists
+		 * since chip->ngpio can be less.
+		 */
+		if (count > chip->ngpio)
+			count = chip->ngpio;
+
+		for (i = 0; i < count; i++)
 			gpio_desc[chip->base + i].name = xstrdup(arr[i]);
 
 		free(arr);
-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 03/10] gpiolib: rename local gpio-line-names variable
  2023-06-14 10:07 [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 01/10] gpiolib: fix gpio-hog functionality Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 02/10] gpiolib: simplify for loop break condition Ahmad Fatoum
@ 2023-06-14 10:07 ` Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 04/10] gpiolib: fix gpio name memory leak Ahmad Fatoum
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 10:07 UTC (permalink / raw)
  To: barebox; +Cc: mfe, Jules Maselbas, Ahmad Fatoum

From: Marco Felsch <m.felsch@pengutronix.de>

This is just a cosmetic commit to align the code more with the Linux
kernel.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - no change
---
 drivers/gpio/gpiolib.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 0df43c9f8f7e..eb2bba042e49 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -524,10 +524,10 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
 					  "gpio-line-names");
 
 	if (count > 0) {
-		const char **arr = xzalloc(count * sizeof(char *));
+		const char **names = xzalloc(count * sizeof(char *));
 
 		of_property_read_string_array(chip->dev->of_node,
-					      "gpio-line-names", arr, count);
+					      "gpio-line-names", names, count);
 
 		/*
 		 * Since property 'gpio-line-names' cannot contains gaps, we
@@ -538,9 +538,9 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
 			count = chip->ngpio;
 
 		for (i = 0; i < count; i++)
-			gpio_desc[chip->base + i].name = xstrdup(arr[i]);
+			gpio_desc[chip->base + i].name = xstrdup(names[i]);
 
-		free(arr);
+		free(names);
 	}
 
 	for_each_available_child_of_node(chip->dev->of_node, np) {
-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 04/10] gpiolib: fix gpio name memory leak
  2023-06-14 10:07 [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2023-06-14 10:07 ` [PATCH v2 03/10] gpiolib: rename local gpio-line-names variable Ahmad Fatoum
@ 2023-06-14 10:07 ` Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 05/10] gpiolib: fix missing error check while query gpio-line-names Ahmad Fatoum
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 10:07 UTC (permalink / raw)
  To: barebox; +Cc: mfe, Jules Maselbas, Ahmad Fatoum

From: Marco Felsch <m.felsch@pengutronix.de>

We never freed the name allocated by xstrdup(). Fix this by use the
const devicetree property value. While on it check if the name is valid
and add the comment to align the code with the kernel.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - no change
---
 drivers/gpio/gpiolib.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index eb2bba042e49..f05e2ac7356a 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -18,7 +18,7 @@ struct gpio_info {
 	bool requested;
 	bool active_low;
 	char *label;
-	char *name;
+	const char *name;
 };
 
 static struct gpio_info *gpio_desc;
@@ -537,8 +537,16 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
 		if (count > chip->ngpio)
 			count = chip->ngpio;
 
-		for (i = 0; i < count; i++)
-			gpio_desc[chip->base + i].name = xstrdup(names[i]);
+		for (i = 0; i < count; i++) {
+			/*
+			 * Allow overriding "fixed" names provided by the GPIO
+			 * provider. The "fixed" names are more often than not
+			 * generic and less informative than the names given in
+			 * device properties.
+			 */
+			if (names[i] && names[i][0])
+				gpio_desc[chip->base + i].name = names[i];
+		}
 
 		free(names);
 	}
-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 05/10] gpiolib: fix missing error check while query gpio-line-names
  2023-06-14 10:07 [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Ahmad Fatoum
                   ` (3 preceding siblings ...)
  2023-06-14 10:07 ` [PATCH v2 04/10] gpiolib: fix gpio name memory leak Ahmad Fatoum
@ 2023-06-14 10:07 ` Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 06/10] gpiolib: refactor gpio-line-names parsing Ahmad Fatoum
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 10:07 UTC (permalink / raw)
  To: barebox; +Cc: mfe, Jules Maselbas, Ahmad Fatoum

From: Marco Felsch <m.felsch@pengutronix.de>

Don't ignore the possible error return code of
of_property_read_string_array().

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - drop error message as we shouldn't hit this issue
    anyway when of_property_count_strings called before
    succeeds.
---
 drivers/gpio/gpiolib.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f05e2ac7356a..275fb7324331 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -526,8 +526,13 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
 	if (count > 0) {
 		const char **names = xzalloc(count * sizeof(char *));
 
-		of_property_read_string_array(chip->dev->of_node,
-					      "gpio-line-names", names, count);
+		ret = of_property_read_string_array(chip->dev->of_node,
+						    "gpio-line-names", names,
+						    count);
+		if (ret < 0) {
+			kfree(names);
+			return ret;
+		}
 
 		/*
 		 * Since property 'gpio-line-names' cannot contains gaps, we
-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 06/10] gpiolib: refactor gpio-line-names parsing
  2023-06-14 10:07 [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Ahmad Fatoum
                   ` (4 preceding siblings ...)
  2023-06-14 10:07 ` [PATCH v2 05/10] gpiolib: fix missing error check while query gpio-line-names Ahmad Fatoum
@ 2023-06-14 10:07 ` Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 07/10] gpiolib: introduce of_gpiochip_add to bundle all of functions Ahmad Fatoum
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 10:07 UTC (permalink / raw)
  To: barebox; +Cc: mfe, Jules Maselbas, Ahmad Fatoum

From: Marco Felsch <m.felsch@pengutronix.de>

Move the gpio-line-names parsing out of of_gpiochip_scan_hogs() since
this has nothing to do with gpio-hogs. The new function is very similar
with the kernel function devprop_gpiochip_set_names().

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - no change
---
 drivers/gpio/gpiolib.c | 102 ++++++++++++++++++++++++++---------------
 1 file changed, 65 insertions(+), 37 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 275fb7324331..97d1a6d6687f 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -515,47 +515,11 @@ static int of_hog_gpio(struct device_node *np, struct gpio_chip *chip,
 static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
 {
 	struct device_node *np;
-	int ret, i, count;
+	int ret, i;
 
 	if (!IS_ENABLED(CONFIG_OFDEVICE) || !chip->dev->of_node)
 		return 0;
 
-	count = of_property_count_strings(chip->dev->of_node,
-					  "gpio-line-names");
-
-	if (count > 0) {
-		const char **names = xzalloc(count * sizeof(char *));
-
-		ret = of_property_read_string_array(chip->dev->of_node,
-						    "gpio-line-names", names,
-						    count);
-		if (ret < 0) {
-			kfree(names);
-			return ret;
-		}
-
-		/*
-		 * Since property 'gpio-line-names' cannot contains gaps, we
-		 * have to be sure we only assign those pins that really exists
-		 * since chip->ngpio can be less.
-		 */
-		if (count > chip->ngpio)
-			count = chip->ngpio;
-
-		for (i = 0; i < count; i++) {
-			/*
-			 * Allow overriding "fixed" names provided by the GPIO
-			 * provider. The "fixed" names are more often than not
-			 * generic and less informative than the names given in
-			 * device properties.
-			 */
-			if (names[i] && names[i][0])
-				gpio_desc[chip->base + i].name = names[i];
-		}
-
-		free(names);
-	}
-
 	for_each_available_child_of_node(chip->dev->of_node, np) {
 		if (!of_property_read_bool(np, "gpio-hog"))
 			continue;
@@ -575,6 +539,65 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
 	return 0;
 }
 
+/*
+ * of_gpiochip_set_names - Set GPIO line names using OF properties
+ * @chip: GPIO chip whose lines should be named, if possible
+ *
+ * Looks for device property "gpio-line-names" and if it exists assigns
+ * GPIO line names for the chip. The memory allocated for the assigned
+ * names belong to the underlying firmware node and should not be released
+ * by the caller.
+ */
+static int of_gpiochip_set_names(struct gpio_chip *chip)
+{
+	struct device *dev = chip->dev;
+	struct device_node *np;
+	const char **names;
+	int ret, i, count;
+
+	np = dev_of_node(dev);
+	if (!np)
+		return 0;
+
+	count = of_property_count_strings(np, "gpio-line-names");
+	if (count < 0)
+		return 0;
+
+	names = kcalloc(count, sizeof(*names), GFP_KERNEL);
+	if (!names)
+		return -ENOMEM;
+
+	ret = of_property_read_string_array(np, "gpio-line-names",
+					    names, count);
+	if (ret < 0) {
+		kfree(names);
+		return ret;
+	}
+
+	/*
+	 * Since property 'gpio-line-names' cannot contains gaps, we
+	 * have to be sure we only assign those pins that really exists
+	 * since chip->ngpio can be less.
+	 */
+	if (count > chip->ngpio)
+		count = chip->ngpio;
+
+	for (i = 0; i < count; i++) {
+		/*
+		 * Allow overriding "fixed" names provided by the GPIO
+		 * provider. The "fixed" names are more often than not
+		 * generic and less informative than the names given in
+		 * device properties.
+		 */
+		if (names[i] && names[i][0])
+			gpio_desc[chip->base + i].name = names[i];
+	}
+
+	free(names);
+
+	return 0;
+}
+
 #ifdef CONFIG_OFDEVICE
 static const char *gpio_suffixes[] = {
 	"gpios",
@@ -636,6 +659,7 @@ int dev_gpiod_get_index(struct device *dev,
 
 int gpiochip_add(struct gpio_chip *chip)
 {
+	int ret;
 	int i;
 
 	if (chip->base >= 0) {
@@ -655,6 +679,10 @@ int gpiochip_add(struct gpio_chip *chip)
 	for (i = chip->base; i < chip->base + chip->ngpio; i++)
 		gpio_desc[i].chip = chip;
 
+	ret = of_gpiochip_set_names(chip);
+	if (ret)
+		return ret;
+
 	return of_gpiochip_scan_hogs(chip);
 }
 
-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 07/10] gpiolib: introduce of_gpiochip_add to bundle all of functions
  2023-06-14 10:07 [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Ahmad Fatoum
                   ` (5 preceding siblings ...)
  2023-06-14 10:07 ` [PATCH v2 06/10] gpiolib: refactor gpio-line-names parsing Ahmad Fatoum
@ 2023-06-14 10:07 ` Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 08/10] OF: gpio: sync of_get_named_gpio_flags variable with kernel Ahmad Fatoum
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 10:07 UTC (permalink / raw)
  To: barebox; +Cc: mfe, Jules Maselbas, Ahmad Fatoum

From: Marco Felsch <m.felsch@pengutronix.de>

Introduce a common OF related function which does all the OF related
part. This allow us to drop the santity checks and harmonizes the code
with the linux kernel.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - no change
---
 drivers/gpio/gpiolib.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 97d1a6d6687f..5e753d1c7300 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -517,9 +517,6 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
 	struct device_node *np;
 	int ret, i;
 
-	if (!IS_ENABLED(CONFIG_OFDEVICE) || !chip->dev->of_node)
-		return 0;
-
 	for_each_available_child_of_node(chip->dev->of_node, np) {
 		if (!of_property_read_bool(np, "gpio-hog"))
 			continue;
@@ -550,15 +547,10 @@ static int of_gpiochip_scan_hogs(struct gpio_chip *chip)
  */
 static int of_gpiochip_set_names(struct gpio_chip *chip)
 {
-	struct device *dev = chip->dev;
-	struct device_node *np;
+	struct device_node *np = dev_of_node(chip->dev);
 	const char **names;
 	int ret, i, count;
 
-	np = dev_of_node(dev);
-	if (!np)
-		return 0;
-
 	count = of_property_count_strings(np, "gpio-line-names");
 	if (count < 0)
 		return 0;
@@ -598,6 +590,22 @@ static int of_gpiochip_set_names(struct gpio_chip *chip)
 	return 0;
 }
 
+static int of_gpiochip_add(struct gpio_chip *chip)
+{
+	struct device_node *np;
+	int ret;
+
+	np = dev_of_node(chip->dev);
+	if (!np)
+		return 0;
+
+	ret = of_gpiochip_set_names(chip);
+	if (ret)
+		return ret;
+
+	return of_gpiochip_scan_hogs(chip);
+}
+
 #ifdef CONFIG_OFDEVICE
 static const char *gpio_suffixes[] = {
 	"gpios",
@@ -659,7 +667,6 @@ int dev_gpiod_get_index(struct device *dev,
 
 int gpiochip_add(struct gpio_chip *chip)
 {
-	int ret;
 	int i;
 
 	if (chip->base >= 0) {
@@ -679,11 +686,7 @@ int gpiochip_add(struct gpio_chip *chip)
 	for (i = chip->base; i < chip->base + chip->ngpio; i++)
 		gpio_desc[i].chip = chip;
 
-	ret = of_gpiochip_set_names(chip);
-	if (ret)
-		return ret;
-
-	return of_gpiochip_scan_hogs(chip);
+	return of_gpiochip_add(chip);
 }
 
 void gpiochip_remove(struct gpio_chip *chip)
-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 08/10] OF: gpio: sync of_get_named_gpio_flags variable with kernel
  2023-06-14 10:07 [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Ahmad Fatoum
                   ` (6 preceding siblings ...)
  2023-06-14 10:07 ` [PATCH v2 07/10] gpiolib: introduce of_gpiochip_add to bundle all of functions Ahmad Fatoum
@ 2023-06-14 10:07 ` Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 09/10] OF: gpio: call of_node_put in of_get_named_gpio_flags Ahmad Fatoum
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 10:07 UTC (permalink / raw)
  To: barebox; +Cc: mfe, Jules Maselbas, Ahmad Fatoum

From: Marco Felsch <m.felsch@pengutronix.de>

Sync local of_phandle_args variable with the kernel to make it easier to
see the diff between both versions. No functional change.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - fix typo (Jules)
---
 drivers/of/of_gpio.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/of/of_gpio.c b/drivers/of/of_gpio.c
index 5da80e249386..c20133bbfd6f 100644
--- a/drivers/of/of_gpio.c
+++ b/drivers/of/of_gpio.c
@@ -60,26 +60,26 @@ static void of_gpio_flags_quirks(struct device_node *np,
 int of_get_named_gpio_flags(struct device_node *np, const char *propname,
 			   int index, enum of_gpio_flags *flags)
 {
-	struct of_phandle_args out_args;
+	struct of_phandle_args gpiospec;
 	struct device *dev;
 	int ret;
 
 	ret = of_parse_phandle_with_args(np, propname, "#gpio-cells",
-					index, &out_args);
+					index, &gpiospec);
 	if (ret) {
 		pr_debug("%s: cannot parse %s property: %d\n",
 			__func__, propname, ret);
 		return ret;
 	}
 
-	dev = of_find_device_by_node(out_args.np);
+	dev = of_find_device_by_node(gpiospec.np);
 	if (!dev) {
 		pr_debug("%s: unable to find device of node %s\n",
-			 __func__, out_args.np->full_name);
+			 __func__, gpiospec.np->full_name);
 		return -EPROBE_DEFER;
 	}
 
-	ret = gpio_get_num(dev, out_args.args[0]);
+	ret = gpio_get_num(dev, gpiospec.args[0]);
 	if (ret == -EPROBE_DEFER)
 		return ret;
 	if (ret < 0) {
@@ -89,7 +89,7 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
 	}
 
 	if (flags) {
-		*flags = out_args.args[1];
+		*flags = gpiospec.args[1];
 		of_gpio_flags_quirks(np, propname, flags, index);
 	}
 
-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 09/10] OF: gpio: call of_node_put in of_get_named_gpio_flags
  2023-06-14 10:07 [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Ahmad Fatoum
                   ` (7 preceding siblings ...)
  2023-06-14 10:07 ` [PATCH v2 08/10] OF: gpio: sync of_get_named_gpio_flags variable with kernel Ahmad Fatoum
@ 2023-06-14 10:07 ` Ahmad Fatoum
  2023-06-14 10:07 ` [PATCH v2 10/10] gpiolib: add of_xlate support Ahmad Fatoum
  2023-06-14 14:19 ` [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Sascha Hauer
  10 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 10:07 UTC (permalink / raw)
  To: barebox; +Cc: mfe, Jules Maselbas, Ahmad Fatoum

From: Marco Felsch <m.felsch@pengutronix.de>

This aligns us with the Linux implementation. There's no functional
change though, because of_node_put is a stub in barebox (same as when
compiling kernel without CONFIG_OF_DYNAMIC enabled).

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - reword commit message to make it clearer that patch is a no-op
    in barebox
---
 drivers/of/of_gpio.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/of/of_gpio.c b/drivers/of/of_gpio.c
index c20133bbfd6f..76398f75422f 100644
--- a/drivers/of/of_gpio.c
+++ b/drivers/of/of_gpio.c
@@ -76,16 +76,17 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
 	if (!dev) {
 		pr_debug("%s: unable to find device of node %s\n",
 			 __func__, gpiospec.np->full_name);
-		return -EPROBE_DEFER;
+		ret = -EPROBE_DEFER;
+		goto out;
 	}
 
 	ret = gpio_get_num(dev, gpiospec.args[0]);
 	if (ret == -EPROBE_DEFER)
-		return ret;
+		goto out;
 	if (ret < 0) {
 		pr_err("%s: unable to get gpio num of device %s: %d\n",
 			__func__, dev_name(dev), ret);
-		return ret;
+		goto out;
 	}
 
 	if (flags) {
@@ -93,6 +94,9 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
 		of_gpio_flags_quirks(np, propname, flags, index);
 	}
 
+out:
+	of_node_put(gpiospec.np);
+
 	return ret;
 }
 EXPORT_SYMBOL(of_get_named_gpio_flags);
-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 10/10] gpiolib: add of_xlate support
  2023-06-14 10:07 [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Ahmad Fatoum
                   ` (8 preceding siblings ...)
  2023-06-14 10:07 ` [PATCH v2 09/10] OF: gpio: call of_node_put in of_get_named_gpio_flags Ahmad Fatoum
@ 2023-06-14 10:07 ` Ahmad Fatoum
  2023-06-16  8:14   ` Sascha Hauer
  2023-06-14 14:19 ` [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Sascha Hauer
  10 siblings, 1 reply; 14+ messages in thread
From: Ahmad Fatoum @ 2023-06-14 10:07 UTC (permalink / raw)
  To: barebox; +Cc: mfe, Jules Maselbas, Ahmad Fatoum

From: Marco Felsch <m.felsch@pengutronix.de>

The of_xlate function can be used to by drivers which need a other gpio
encoding than:

  gpio = <&phandle gpio-nr gpio-flags>;

The of_xlate code is as close as possible to the kernel counter part
which makes it easier to add 'struct gpio_desc' support later on.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - fix typos (Jules)
  - forward-declare struct of_phandle_args in header
  - use WARN_ON in if condition instead of WARN_ON(1) in body
  - use early exits in of_find_gpiochip_by_xlate
---
 drivers/gpio/gpiolib.c | 49 ++++++++++++++++++++++++++++++++++++
 drivers/of/of_gpio.c   | 56 +++++++++++++++++++++++++++++++++---------
 include/gpio.h         | 26 ++++++++++++++++++++
 3 files changed, 119 insertions(+), 12 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5e753d1c7300..f353ce50196c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -590,6 +590,41 @@ static int of_gpiochip_set_names(struct gpio_chip *chip)
 	return 0;
 }
 
+/**
+ * of_gpio_simple_xlate - translate gpiospec to the GPIO number and flags
+ * @gc:		pointer to the gpio_chip structure
+ * @gpiospec:	GPIO specifier as found in the device tree
+ * @flags:	a flags pointer to fill in
+ *
+ * This is simple translation function, suitable for the most 1:1 mapped
+ * GPIO chips. This function performs only one sanity check: whether GPIO
+ * is less than ngpios (that is specified in the gpio_chip).
+ */
+static int of_gpio_simple_xlate(struct gpio_chip *gc,
+				const struct of_phandle_args *gpiospec,
+				u32 *flags)
+{
+	/*
+	 * We're discouraging gpio_cells < 2, since that way you'll have to
+	 * write your own xlate function (that will have to retrieve the GPIO
+	 * number and the flags from a single gpio cell -- this is possible,
+	 * but not recommended).
+	 */
+	if (WARN_ON(gc->of_gpio_n_cells < 2))
+		return -EINVAL;
+
+	if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
+		return -EINVAL;
+
+	if (gpiospec->args[0] >= gc->ngpio)
+		return -EINVAL;
+
+	if (flags)
+		*flags = gpiospec->args[1];
+
+	return gc->base + gpiospec->args[0];
+}
+
 static int of_gpiochip_add(struct gpio_chip *chip)
 {
 	struct device_node *np;
@@ -599,6 +634,20 @@ static int of_gpiochip_add(struct gpio_chip *chip)
 	if (!np)
 		return 0;
 
+	if (!chip->ops->of_xlate)
+		chip->ops->of_xlate = of_gpio_simple_xlate;
+
+	/*
+	 * Separate check since the 'struct gpio_ops' is always the same for
+	 * every 'struct gpio_chip' of the same instance (e.g. 'struct
+	 * imx_gpio_chip').
+	 */
+	if (chip->ops->of_xlate == of_gpio_simple_xlate)
+		chip->of_gpio_n_cells = 2;
+
+	if (chip->of_gpio_n_cells > MAX_PHANDLE_ARGS)
+		return -EINVAL;
+
 	ret = of_gpiochip_set_names(chip);
 	if (ret)
 		return ret;
diff --git a/drivers/of/of_gpio.c b/drivers/of/of_gpio.c
index 76398f75422f..7a6435e2296a 100644
--- a/drivers/of/of_gpio.c
+++ b/drivers/of/of_gpio.c
@@ -46,6 +46,44 @@ static void of_gpio_flags_quirks(struct device_node *np,
 
 }
 
+static struct gpio_chip *of_find_gpiochip_by_xlate(
+					struct of_phandle_args *gpiospec)
+{
+	struct gpio_chip *chip;
+	struct device *dev;
+
+	dev = of_find_device_by_node(gpiospec->np);
+	if (!dev) {
+		pr_debug("%s: unable to find device of node %pOF\n",
+			 __func__, gpiospec->np);
+		return NULL;
+	}
+
+	chip = gpio_get_chip_by_dev(dev);
+	if (!chip) {
+		pr_debug("%s: unable to find gpiochip\n", __func__);
+		return NULL;
+	}
+
+	if (!chip->ops->of_xlate ||
+	    chip->ops->of_xlate(chip, gpiospec, NULL) < 0) {
+		pr_err("%s: failed to execute of_xlate\n", __func__);
+		return NULL;
+	}
+
+	return chip;
+}
+
+static int of_xlate_and_get_gpiod_flags(struct gpio_chip *chip,
+					struct of_phandle_args *gpiospec,
+					enum of_gpio_flags *flags)
+{
+	if (chip->of_gpio_n_cells != gpiospec->args_count)
+		return -EINVAL;
+
+	return chip->ops->of_xlate(chip, gpiospec, flags);
+}
+
 /**
  * of_get_named_gpio_flags() - Get a GPIO number and flags to use with GPIO API
  * @np:		device node to get GPIO from
@@ -61,7 +99,7 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
 			   int index, enum of_gpio_flags *flags)
 {
 	struct of_phandle_args gpiospec;
-	struct device *dev;
+	struct gpio_chip *chip;
 	int ret;
 
 	ret = of_parse_phandle_with_args(np, propname, "#gpio-cells",
@@ -72,27 +110,21 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
 		return ret;
 	}
 
-	dev = of_find_device_by_node(gpiospec.np);
-	if (!dev) {
-		pr_debug("%s: unable to find device of node %s\n",
-			 __func__, gpiospec.np->full_name);
+	chip = of_find_gpiochip_by_xlate(&gpiospec);
+	if (!chip) {
 		ret = -EPROBE_DEFER;
 		goto out;
 	}
 
-	ret = gpio_get_num(dev, gpiospec.args[0]);
-	if (ret == -EPROBE_DEFER)
-		goto out;
+	ret = of_xlate_and_get_gpiod_flags(chip, &gpiospec, flags);
 	if (ret < 0) {
 		pr_err("%s: unable to get gpio num of device %s: %d\n",
-			__func__, dev_name(dev), ret);
+			__func__, dev_name(chip->dev), ret);
 		goto out;
 	}
 
-	if (flags) {
-		*flags = gpiospec.args[1];
+	if (flags)
 		of_gpio_flags_quirks(np, propname, flags, index);
-	}
 
 out:
 	of_node_put(gpiospec.np);
diff --git a/include/gpio.h b/include/gpio.h
index 5f2c16584c3b..9951532084ac 100644
--- a/include/gpio.h
+++ b/include/gpio.h
@@ -168,6 +168,7 @@ int gpio_array_to_id(const struct gpio *array, size_t num, u32 *val);
 #endif
 
 struct gpio_chip;
+struct of_phandle_args;
 
 struct gpio_ops {
 	int (*request)(struct gpio_chip *chip, unsigned offset);
@@ -177,6 +178,22 @@ struct gpio_ops {
 	int (*get_direction)(struct gpio_chip *chip, unsigned offset);
 	int (*get)(struct gpio_chip *chip, unsigned offset);
 	void (*set)(struct gpio_chip *chip, unsigned offset, int value);
+
+#if defined(CONFIG_OF_GPIO)
+	/*
+	 * If CONFIG_OF_GPIO is enabled, then all GPIO controllers described in
+	 * the device tree automatically may have an OF translation
+	 */
+
+	/**
+	 * @of_xlate:
+	 *
+	 * Callback to translate a device tree GPIO specifier into a chip-
+	 * relative GPIO number and flags.
+	 */
+	int (*of_xlate)(struct gpio_chip *gc,
+			const struct of_phandle_args *gpiospec, u32 *flags);
+#endif
 };
 
 struct gpio_chip {
@@ -185,6 +202,15 @@ struct gpio_chip {
 	int base;
 	int ngpio;
 
+#if defined(CONFIG_OF_GPIO)
+	/**
+	 * @of_gpio_n_cells:
+	 *
+	 * Number of cells used to form the GPIO specifier.
+	 */
+	unsigned int of_gpio_n_cells;
+#endif
+
 	struct gpio_ops *ops;
 
 	struct list_head list;
-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib
  2023-06-14 10:07 [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Ahmad Fatoum
                   ` (9 preceding siblings ...)
  2023-06-14 10:07 ` [PATCH v2 10/10] gpiolib: add of_xlate support Ahmad Fatoum
@ 2023-06-14 14:19 ` Sascha Hauer
  10 siblings, 0 replies; 14+ messages in thread
From: Sascha Hauer @ 2023-06-14 14:19 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox, mfe, Jules Maselbas

On Wed, Jun 14, 2023 at 12:07:37PM +0200, Ahmad Fatoum wrote:
> Quoting v1:
> 
>   the purpose of this series is to fix the gpio-hogs mechanism since this
>   is broken since commit 3641d381e6 ("gpiolib: Add of_xlate support").
>   
>   Patch1: Revert the above mentioned commit to make the gpio-hogs working
>           again.
>   
>   Patch2-10: Add the of_xlate support required for the upcoming sunxi
>              pinctrl driver. The mechanism is now more in sync with kernel
>              gpiolib. This allows to sync easier with the kernel gpiolib
>              in case of new features or fixes.
> 
> I've incorporated feedback and tested this still works on a i.mx8mn.
> Patch 1 should be ok to go into master and the rest to next IMO.
> 
> Changelog in each patch.
> 
> Marco Felsch (10):
>   gpiolib: fix gpio-hog functionality
>   gpiolib: simplify for loop break condition
>   gpiolib: rename local gpio-line-names variable
>   gpiolib: fix gpio name memory leak
>   gpiolib: fix missing error check while query gpio-line-names
>   gpiolib: refactor gpio-line-names parsing
>   gpiolib: introduce of_gpiochip_add to bundle all of functions
>   OF: gpio: sync of_get_named_gpio_flags variable with kernel
>   OF: gpio: call of_node_put in of_get_named_gpio_flags
>   gpiolib: add of_xlate support
> 
>  drivers/gpio/gpiolib.c | 207 ++++++++++++++++++++++++++++++-----------
>  drivers/of/of_gpio.c   |  69 ++++++++++----
>  include/gpio.h         |  30 +++++-
>  3 files changed, 231 insertions(+), 75 deletions(-)

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 10/10] gpiolib: add of_xlate support
  2023-06-14 10:07 ` [PATCH v2 10/10] gpiolib: add of_xlate support Ahmad Fatoum
@ 2023-06-16  8:14   ` Sascha Hauer
  2023-06-16 12:13     ` Marco Felsch
  0 siblings, 1 reply; 14+ messages in thread
From: Sascha Hauer @ 2023-06-16  8:14 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox, mfe, Jules Maselbas

On Wed, Jun 14, 2023 at 12:07:47PM +0200, Ahmad Fatoum wrote:
> From: Marco Felsch <m.felsch@pengutronix.de>
> 
> The of_xlate function can be used to by drivers which need a other gpio
> encoding than:
> 
>   gpio = <&phandle gpio-nr gpio-flags>;
> 
> The of_xlate code is as close as possible to the kernel counter part
> which makes it easier to add 'struct gpio_desc' support later on.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> v1 -> v2:
>   - fix typos (Jules)
>   - forward-declare struct of_phandle_args in header
>   - use WARN_ON in if condition instead of WARN_ON(1) in body
>   - use early exits in of_find_gpiochip_by_xlate
> ---
>  drivers/gpio/gpiolib.c | 49 ++++++++++++++++++++++++++++++++++++
>  drivers/of/of_gpio.c   | 56 +++++++++++++++++++++++++++++++++---------
>  include/gpio.h         | 26 ++++++++++++++++++++
>  3 files changed, 119 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 5e753d1c7300..f353ce50196c 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -590,6 +590,41 @@ static int of_gpiochip_set_names(struct gpio_chip *chip)
>  	return 0;
>  }
>  
> +/**
> + * of_gpio_simple_xlate - translate gpiospec to the GPIO number and flags
> + * @gc:		pointer to the gpio_chip structure
> + * @gpiospec:	GPIO specifier as found in the device tree
> + * @flags:	a flags pointer to fill in
> + *
> + * This is simple translation function, suitable for the most 1:1 mapped
> + * GPIO chips. This function performs only one sanity check: whether GPIO
> + * is less than ngpios (that is specified in the gpio_chip).
> + */
> +static int of_gpio_simple_xlate(struct gpio_chip *gc,
> +				const struct of_phandle_args *gpiospec,
> +				u32 *flags)
> +{
> +	/*
> +	 * We're discouraging gpio_cells < 2, since that way you'll have to
> +	 * write your own xlate function (that will have to retrieve the GPIO
> +	 * number and the flags from a single gpio cell -- this is possible,
> +	 * but not recommended).
> +	 */
> +	if (WARN_ON(gc->of_gpio_n_cells < 2))
> +		return -EINVAL;
> +
> +	if (WARN_ON(gpiospec->args_count < gc->of_gpio_n_cells))
> +		return -EINVAL;
> +
> +	if (gpiospec->args[0] >= gc->ngpio)
> +		return -EINVAL;
> +
> +	if (flags)
> +		*flags = gpiospec->args[1];
> +
> +	return gc->base + gpiospec->args[0];
> +}
> +
>  static int of_gpiochip_add(struct gpio_chip *chip)
>  {
>  	struct device_node *np;
> @@ -599,6 +634,20 @@ static int of_gpiochip_add(struct gpio_chip *chip)
>  	if (!np)
>  		return 0;
>  
> +	if (!chip->ops->of_xlate)
> +		chip->ops->of_xlate = of_gpio_simple_xlate;
> +
> +	/*
> +	 * Separate check since the 'struct gpio_ops' is always the same for
> +	 * every 'struct gpio_chip' of the same instance (e.g. 'struct
> +	 * imx_gpio_chip').
> +	 */
> +	if (chip->ops->of_xlate == of_gpio_simple_xlate)
> +		chip->of_gpio_n_cells = 2;
> +
> +	if (chip->of_gpio_n_cells > MAX_PHANDLE_ARGS)
> +		return -EINVAL;
> +
>  	ret = of_gpiochip_set_names(chip);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/of/of_gpio.c b/drivers/of/of_gpio.c
> index 76398f75422f..7a6435e2296a 100644
> --- a/drivers/of/of_gpio.c
> +++ b/drivers/of/of_gpio.c
> @@ -46,6 +46,44 @@ static void of_gpio_flags_quirks(struct device_node *np,
>  
>  }
>  
> +static struct gpio_chip *of_find_gpiochip_by_xlate(
> +					struct of_phandle_args *gpiospec)
> +{
> +	struct gpio_chip *chip;
> +	struct device *dev;
> +
> +	dev = of_find_device_by_node(gpiospec->np);
> +	if (!dev) {
> +		pr_debug("%s: unable to find device of node %pOF\n",
> +			 __func__, gpiospec->np);
> +		return NULL;
> +	}
> +
> +	chip = gpio_get_chip_by_dev(dev);
> +	if (!chip) {
> +		pr_debug("%s: unable to find gpiochip\n", __func__);
> +		return NULL;
> +	}
> +
> +	if (!chip->ops->of_xlate ||
> +	    chip->ops->of_xlate(chip, gpiospec, NULL) < 0) {
> +		pr_err("%s: failed to execute of_xlate\n", __func__);
> +		return NULL;
> +	}
> +
> +	return chip;
> +}
> +
> +static int of_xlate_and_get_gpiod_flags(struct gpio_chip *chip,
> +					struct of_phandle_args *gpiospec,
> +					enum of_gpio_flags *flags)
> +{
> +	if (chip->of_gpio_n_cells != gpiospec->args_count)
> +		return -EINVAL;
> +
> +	return chip->ops->of_xlate(chip, gpiospec, flags);
> +}
> +
>  /**
>   * of_get_named_gpio_flags() - Get a GPIO number and flags to use with GPIO API
>   * @np:		device node to get GPIO from
> @@ -61,7 +99,7 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
>  			   int index, enum of_gpio_flags *flags)
>  {
>  	struct of_phandle_args gpiospec;
> -	struct device *dev;
> +	struct gpio_chip *chip;
>  	int ret;
>  
>  	ret = of_parse_phandle_with_args(np, propname, "#gpio-cells",
> @@ -72,27 +110,21 @@ int of_get_named_gpio_flags(struct device_node *np, const char *propname,
>  		return ret;
>  	}
>  
> -	dev = of_find_device_by_node(gpiospec.np);
> -	if (!dev) {
> -		pr_debug("%s: unable to find device of node %s\n",
> -			 __func__, gpiospec.np->full_name);
> +	chip = of_find_gpiochip_by_xlate(&gpiospec);
> +	if (!chip) {
>  		ret = -EPROBE_DEFER;
>  		goto out;
>  	}
>  
> -	ret = gpio_get_num(dev, gpiospec.args[0]);
> -	if (ret == -EPROBE_DEFER)
> -		goto out;
> +	ret = of_xlate_and_get_gpiod_flags(chip, &gpiospec, flags);
>  	if (ret < 0) {
>  		pr_err("%s: unable to get gpio num of device %s: %d\n",
> -			__func__, dev_name(dev), ret);
> +			__func__, dev_name(chip->dev), ret);
>  		goto out;
>  	}
>  
> -	if (flags) {
> -		*flags = gpiospec.args[1];
> +	if (flags)
>  		of_gpio_flags_quirks(np, propname, flags, index);
> -	}
>  
>  out:
>  	of_node_put(gpiospec.np);
> diff --git a/include/gpio.h b/include/gpio.h
> index 5f2c16584c3b..9951532084ac 100644
> --- a/include/gpio.h
> +++ b/include/gpio.h
> @@ -168,6 +168,7 @@ int gpio_array_to_id(const struct gpio *array, size_t num, u32 *val);
>  #endif
>  
>  struct gpio_chip;
> +struct of_phandle_args;
>  
>  struct gpio_ops {
>  	int (*request)(struct gpio_chip *chip, unsigned offset);
> @@ -177,6 +178,22 @@ struct gpio_ops {
>  	int (*get_direction)(struct gpio_chip *chip, unsigned offset);
>  	int (*get)(struct gpio_chip *chip, unsigned offset);
>  	void (*set)(struct gpio_chip *chip, unsigned offset, int value);
> +
> +#if defined(CONFIG_OF_GPIO)
> +	/*
> +	 * If CONFIG_OF_GPIO is enabled, then all GPIO controllers described in
> +	 * the device tree automatically may have an OF translation
> +	 */
> +
> +	/**
> +	 * @of_xlate:
> +	 *
> +	 * Callback to translate a device tree GPIO specifier into a chip-
> +	 * relative GPIO number and flags.
> +	 */
> +	int (*of_xlate)(struct gpio_chip *gc,
> +			const struct of_phandle_args *gpiospec, u32 *flags);
> +#endif
>  };
>  
>  struct gpio_chip {
> @@ -185,6 +202,15 @@ struct gpio_chip {
>  	int base;
>  	int ngpio;
>  
> +#if defined(CONFIG_OF_GPIO)
> +	/**
> +	 * @of_gpio_n_cells:
> +	 *
> +	 * Number of cells used to form the GPIO specifier.
> +	 */
> +	unsigned int of_gpio_n_cells;
> +#endif

This patch adds new fields to struct gpio_chip and struct gpio_ops
#ifdeffed with CONFIG_OF_GPIO, but the code accessing these fields is
not guarded with that #ifdef resulting in compilation failures in
several defconfigs like for example animeo_ip_defconfig.

I fixed this by adding the missing #ifdefs in the code.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 10/10] gpiolib: add of_xlate support
  2023-06-16  8:14   ` Sascha Hauer
@ 2023-06-16 12:13     ` Marco Felsch
  0 siblings, 0 replies; 14+ messages in thread
From: Marco Felsch @ 2023-06-16 12:13 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Ahmad Fatoum, barebox, Jules Maselbas

On 23-06-16, Sascha Hauer wrote:
> On Wed, Jun 14, 2023 at 12:07:47PM +0200, Ahmad Fatoum wrote:

...

> > diff --git a/include/gpio.h b/include/gpio.h
> > index 5f2c16584c3b..9951532084ac 100644
> > --- a/include/gpio.h
> > +++ b/include/gpio.h
> > @@ -168,6 +168,7 @@ int gpio_array_to_id(const struct gpio *array, size_t num, u32 *val);
> >  #endif
> >  
> >  struct gpio_chip;
> > +struct of_phandle_args;
> >  
> >  struct gpio_ops {
> >  	int (*request)(struct gpio_chip *chip, unsigned offset);
> > @@ -177,6 +178,22 @@ struct gpio_ops {
> >  	int (*get_direction)(struct gpio_chip *chip, unsigned offset);
> >  	int (*get)(struct gpio_chip *chip, unsigned offset);
> >  	void (*set)(struct gpio_chip *chip, unsigned offset, int value);
> > +
> > +#if defined(CONFIG_OF_GPIO)
> > +	/*
> > +	 * If CONFIG_OF_GPIO is enabled, then all GPIO controllers described in
> > +	 * the device tree automatically may have an OF translation
> > +	 */
> > +
> > +	/**
> > +	 * @of_xlate:
> > +	 *
> > +	 * Callback to translate a device tree GPIO specifier into a chip-
> > +	 * relative GPIO number and flags.
> > +	 */
> > +	int (*of_xlate)(struct gpio_chip *gc,
> > +			const struct of_phandle_args *gpiospec, u32 *flags);
> > +#endif
> >  };
> >  
> >  struct gpio_chip {
> > @@ -185,6 +202,15 @@ struct gpio_chip {
> >  	int base;
> >  	int ngpio;
> >  
> > +#if defined(CONFIG_OF_GPIO)
> > +	/**
> > +	 * @of_gpio_n_cells:
> > +	 *
> > +	 * Number of cells used to form the GPIO specifier.
> > +	 */
> > +	unsigned int of_gpio_n_cells;
> > +#endif
> 
> This patch adds new fields to struct gpio_chip and struct gpio_ops
> #ifdeffed with CONFIG_OF_GPIO, but the code accessing these fields is
> not guarded with that #ifdef resulting in compilation failures in
> several defconfigs like for example animeo_ip_defconfig.

Argh..

> I fixed this by adding the missing #ifdefs in the code.

Thanks a lot.

Regards,
  Marco



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-06-16 12:15 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-14 10:07 [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Ahmad Fatoum
2023-06-14 10:07 ` [PATCH v2 01/10] gpiolib: fix gpio-hog functionality Ahmad Fatoum
2023-06-14 10:07 ` [PATCH v2 02/10] gpiolib: simplify for loop break condition Ahmad Fatoum
2023-06-14 10:07 ` [PATCH v2 03/10] gpiolib: rename local gpio-line-names variable Ahmad Fatoum
2023-06-14 10:07 ` [PATCH v2 04/10] gpiolib: fix gpio name memory leak Ahmad Fatoum
2023-06-14 10:07 ` [PATCH v2 05/10] gpiolib: fix missing error check while query gpio-line-names Ahmad Fatoum
2023-06-14 10:07 ` [PATCH v2 06/10] gpiolib: refactor gpio-line-names parsing Ahmad Fatoum
2023-06-14 10:07 ` [PATCH v2 07/10] gpiolib: introduce of_gpiochip_add to bundle all of functions Ahmad Fatoum
2023-06-14 10:07 ` [PATCH v2 08/10] OF: gpio: sync of_get_named_gpio_flags variable with kernel Ahmad Fatoum
2023-06-14 10:07 ` [PATCH v2 09/10] OF: gpio: call of_node_put in of_get_named_gpio_flags Ahmad Fatoum
2023-06-14 10:07 ` [PATCH v2 10/10] gpiolib: add of_xlate support Ahmad Fatoum
2023-06-16  8:14   ` Sascha Hauer
2023-06-16 12:13     ` Marco Felsch
2023-06-14 14:19 ` [PATCH v2 00/10] Fix gpio-hogs and sync with Linux gpiolib Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox