[PATCH 1/4] gpiolib: of: do not ignore requested index when applying quirks

[PATCH 1/4] gpiolib: of: do not ignore requested index when applying quirks

[Dmitry Torokhov]

We should not ignore index passed into of_find_gpio() when handling
quirks. While in practice this change will not have any effect, it
will allow consolidate quirk handling.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpiolib-of.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index a037b50bef33..c08564948bf9 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -372,7 +372,9 @@ EXPORT_SYMBOL_GPL(gpiod_get_from_of_node);
  * properties should be named "foo-gpios" so we have this special kludge for
  * them.
  */
-static struct gpio_desc *of_find_spi_gpio(struct device *dev, const char *con_id,
+static struct gpio_desc *of_find_spi_gpio(struct device *dev,
+					  const char *con_id,
+					  unsigned int idx,
 					  enum of_gpio_flags *of_flags)
 {
 	char prop_name[32]; /* 32 is max size of property name */
@@ -393,7 +395,7 @@ static struct gpio_desc *of_find_spi_gpio(struct device *dev, const char *con_id
 	/* Will be "gpio-sck", "gpio-mosi" or "gpio-miso" */
 	snprintf(prop_name, sizeof(prop_name), "%s-%s", "gpio", con_id);
 
-	desc = of_get_named_gpiod_flags(np, prop_name, 0, of_flags);
+	desc = of_get_named_gpiod_flags(np, prop_name, idx, of_flags);
 	return desc;
 }
 
@@ -434,7 +436,9 @@ static struct gpio_desc *of_find_spi_cs_gpio(struct device *dev,
  * properties should be named "foo-gpios" so we have this special kludge for
  * them.
  */
-static struct gpio_desc *of_find_regulator_gpio(struct device *dev, const char *con_id,
+static struct gpio_desc *of_find_regulator_gpio(struct device *dev,
+						const char *con_id,
+						unsigned int idx,
 						enum of_gpio_flags *of_flags)
 {
 	/* These are the connection IDs we accept as legacy GPIO phandles */
@@ -457,12 +461,13 @@ static struct gpio_desc *of_find_regulator_gpio(struct device *dev, const char *
 	if (i < 0)
 		return ERR_PTR(-ENOENT);
 
-	desc = of_get_named_gpiod_flags(np, con_id, 0, of_flags);
+	desc = of_get_named_gpiod_flags(np, con_id, idx, of_flags);
 	return desc;
 }
 
 static struct gpio_desc *of_find_arizona_gpio(struct device *dev,
 					      const char *con_id,
+					      unsigned int idx,
 					      enum of_gpio_flags *of_flags)
 {
 	if (!IS_ENABLED(CONFIG_MFD_ARIZONA))
@@ -471,17 +476,18 @@ static struct gpio_desc *of_find_arizona_gpio(struct device *dev,
 	if (!con_id || strcmp(con_id, "wlf,reset"))
 		return ERR_PTR(-ENOENT);
 
-	return of_get_named_gpiod_flags(dev->of_node, con_id, 0, of_flags);
+	return of_get_named_gpiod_flags(dev->of_node, con_id, idx, of_flags);
 }
 
 static struct gpio_desc *of_find_usb_gpio(struct device *dev,
 					  const char *con_id,
+					  unsigned int idx,
 					  enum of_gpio_flags *of_flags)
 {
 	/*
-	 * Currently this USB quirk is only for the Fairchild FUSB302 host which is using
-	 * an undocumented DT GPIO line named "fcs,int_n" without the compulsory "-gpios"
-	 * suffix.
+	 * Currently this USB quirk is only for the Fairchild FUSB302 host
+	 * which is using an undocumented DT GPIO line named "fcs,int_n"
+	 * without the compulsory "-gpios" suffix.
 	 */
 	if (!IS_ENABLED(CONFIG_TYPEC_FUSB302))
 		return ERR_PTR(-ENOENT);
@@ -489,7 +495,7 @@ static struct gpio_desc *of_find_usb_gpio(struct device *dev,
 	if (!con_id || strcmp(con_id, "fcs,int_n"))
 		return ERR_PTR(-ENOENT);
 
-	return of_get_named_gpiod_flags(dev->of_node, con_id, 0, of_flags);
+	return of_get_named_gpiod_flags(dev->of_node, con_id, idx, of_flags);
 }
 
 struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
@@ -518,7 +524,7 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 
 	if (gpiod_not_found(desc)) {
 		/* Special handling for SPI GPIOs if used */
-		desc = of_find_spi_gpio(dev, con_id, &of_flags);
+		desc = of_find_spi_gpio(dev, con_id, idx, &of_flags);
 	}
 
 	if (gpiod_not_found(desc)) {
@@ -530,14 +536,14 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 
 	if (gpiod_not_found(desc)) {
 		/* Special handling for regulator GPIOs if used */
-		desc = of_find_regulator_gpio(dev, con_id, &of_flags);
+		desc = of_find_regulator_gpio(dev, con_id, idx, &of_flags);
 	}
 
 	if (gpiod_not_found(desc))
-		desc = of_find_arizona_gpio(dev, con_id, &of_flags);
+		desc = of_find_arizona_gpio(dev, con_id, idx, &of_flags);
 
 	if (gpiod_not_found(desc))
-		desc = of_find_usb_gpio(dev, con_id, &of_flags);
+		desc = of_find_usb_gpio(dev, con_id, idx, &of_flags);
 
 	if (IS_ERR(desc))
 		return desc;
-- 
2.37.2.789.g6183377224-goog

[PATCH 2/4] gpiolib: of: make Freescale SPI quirk similar to all others

[Dmitry Torokhov]

There is no need for of_find_spi_cs_gpio() to be different from other
quirks: the only variant of property actually used in DTS is "gpios"
(plural) so we can use of_get_named_gpiod_flags() instead of recursing
into of_find_gpio() again.

This will allow us consolidate quirk handling down the road.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpiolib-of.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index c08564948bf9..30b89b694530 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -407,7 +407,7 @@ static struct gpio_desc *of_find_spi_gpio(struct device *dev,
 static struct gpio_desc *of_find_spi_cs_gpio(struct device *dev,
 					     const char *con_id,
 					     unsigned int idx,
-					     unsigned long *flags)
+					     enum of_gpio_flags *of_flags)
 {
 	const struct device_node *np = dev->of_node;
 
@@ -428,7 +428,7 @@ static struct gpio_desc *of_find_spi_cs_gpio(struct device *dev,
 	 * uses just "gpios" so translate to that when "cs-gpios" is
 	 * requested.
 	 */
-	return of_find_gpio(dev, NULL, idx, flags);
+	return of_get_named_gpiod_flags(dev->of_node, "gpios", idx, of_flags);
 }
 
 /*
@@ -527,12 +527,8 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 		desc = of_find_spi_gpio(dev, con_id, idx, &of_flags);
 	}
 
-	if (gpiod_not_found(desc)) {
-		/* This quirk looks up flags and all */
-		desc = of_find_spi_cs_gpio(dev, con_id, idx, flags);
-		if (!IS_ERR(desc))
-			return desc;
-	}
+	if (gpiod_not_found(desc))
+		desc = of_find_spi_cs_gpio(dev, con_id, idx, &of_flags);
 
 	if (gpiod_not_found(desc)) {
 		/* Special handling for regulator GPIOs if used */
-- 
2.37.2.789.g6183377224-goog

[PATCH 3/4] gpiolib: rework quirk handling in of_find_gpio()

[Dmitry Torokhov]

Instead of having a string of "if" statements let's put all quirks into
an array and iterate over them.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpiolib-of.c | 62 ++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 30b89b694530..097e948c1d49 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -372,14 +372,12 @@ EXPORT_SYMBOL_GPL(gpiod_get_from_of_node);
  * properties should be named "foo-gpios" so we have this special kludge for
  * them.
  */
-static struct gpio_desc *of_find_spi_gpio(struct device *dev,
+static struct gpio_desc *of_find_spi_gpio(struct device_node *np,
 					  const char *con_id,
 					  unsigned int idx,
 					  enum of_gpio_flags *of_flags)
 {
 	char prop_name[32]; /* 32 is max size of property name */
-	const struct device_node *np = dev->of_node;
-	struct gpio_desc *desc;
 
 	/*
 	 * Hopefully the compiler stubs the rest of the function if this
@@ -395,8 +393,7 @@ static struct gpio_desc *of_find_spi_gpio(struct device *dev,
 	/* Will be "gpio-sck", "gpio-mosi" or "gpio-miso" */
 	snprintf(prop_name, sizeof(prop_name), "%s-%s", "gpio", con_id);
 
-	desc = of_get_named_gpiod_flags(np, prop_name, idx, of_flags);
-	return desc;
+	return of_get_named_gpiod_flags(np, prop_name, idx, of_flags);
 }
 
 /*
@@ -404,13 +401,11 @@ static struct gpio_desc *of_find_spi_gpio(struct device *dev,
  * lines rather than "cs-gpios" like all other SPI hardware. Account for this
  * with a special quirk.
  */
-static struct gpio_desc *of_find_spi_cs_gpio(struct device *dev,
+static struct gpio_desc *of_find_spi_cs_gpio(struct device_node *np,
 					     const char *con_id,
 					     unsigned int idx,
 					     enum of_gpio_flags *of_flags)
 {
-	const struct device_node *np = dev->of_node;
-
 	if (!IS_ENABLED(CONFIG_SPI_MASTER))
 		return ERR_PTR(-ENOENT);
 
@@ -428,7 +423,7 @@ static struct gpio_desc *of_find_spi_cs_gpio(struct device *dev,
 	 * uses just "gpios" so translate to that when "cs-gpios" is
 	 * requested.
 	 */
-	return of_get_named_gpiod_flags(dev->of_node, "gpios", idx, of_flags);
+	return of_get_named_gpiod_flags(np, "gpios", idx, of_flags);
 }
 
 /*
@@ -436,7 +431,7 @@ static struct gpio_desc *of_find_spi_cs_gpio(struct device *dev,
  * properties should be named "foo-gpios" so we have this special kludge for
  * them.
  */
-static struct gpio_desc *of_find_regulator_gpio(struct device *dev,
+static struct gpio_desc *of_find_regulator_gpio(struct device_node *np,
 						const char *con_id,
 						unsigned int idx,
 						enum of_gpio_flags *of_flags)
@@ -447,8 +442,6 @@ static struct gpio_desc *of_find_regulator_gpio(struct device *dev,
 		"wlf,ldo1ena", /* WM8994 */
 		"wlf,ldo2ena", /* WM8994 */
 	};
-	const struct device_node *np = dev->of_node;
-	struct gpio_desc *desc;
 	int i;
 
 	if (!IS_ENABLED(CONFIG_REGULATOR))
@@ -461,11 +454,10 @@ static struct gpio_desc *of_find_regulator_gpio(struct device *dev,
 	if (i < 0)
 		return ERR_PTR(-ENOENT);
 
-	desc = of_get_named_gpiod_flags(np, con_id, idx, of_flags);
-	return desc;
+	return of_get_named_gpiod_flags(np, con_id, idx, of_flags);
 }
 
-static struct gpio_desc *of_find_arizona_gpio(struct device *dev,
+static struct gpio_desc *of_find_arizona_gpio(struct device_node *np,
 					      const char *con_id,
 					      unsigned int idx,
 					      enum of_gpio_flags *of_flags)
@@ -476,10 +468,10 @@ static struct gpio_desc *of_find_arizona_gpio(struct device *dev,
 	if (!con_id || strcmp(con_id, "wlf,reset"))
 		return ERR_PTR(-ENOENT);
 
-	return of_get_named_gpiod_flags(dev->of_node, con_id, idx, of_flags);
+	return of_get_named_gpiod_flags(np, con_id, idx, of_flags);
 }
 
-static struct gpio_desc *of_find_usb_gpio(struct device *dev,
+static struct gpio_desc *of_find_usb_gpio(struct device_node *np,
 					  const char *con_id,
 					  unsigned int idx,
 					  enum of_gpio_flags *of_flags)
@@ -495,14 +487,27 @@ static struct gpio_desc *of_find_usb_gpio(struct device *dev,
 	if (!con_id || strcmp(con_id, "fcs,int_n"))
 		return ERR_PTR(-ENOENT);
 
-	return of_get_named_gpiod_flags(dev->of_node, con_id, idx, of_flags);
+	return of_get_named_gpiod_flags(np, con_id, idx, of_flags);
 }
 
+typedef struct gpio_desc *(*of_find_gpio_quirk)(struct device_node *np,
+						const char *con_id,
+						unsigned int idx,
+						enum of_gpio_flags *of_flags);
+static const of_find_gpio_quirk of_find_gpio_quirks[] = {
+	of_find_spi_gpio,
+	of_find_spi_cs_gpio,
+	of_find_regulator_gpio,
+	of_find_arizona_gpio,
+	of_find_usb_gpio,
+};
+
 struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 			       unsigned int idx, unsigned long *flags)
 {
 	char prop_name[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;
 
@@ -522,24 +527,9 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 			break;
 	}
 
-	if (gpiod_not_found(desc)) {
-		/* Special handling for SPI GPIOs if used */
-		desc = of_find_spi_gpio(dev, con_id, idx, &of_flags);
-	}
-
-	if (gpiod_not_found(desc))
-		desc = of_find_spi_cs_gpio(dev, con_id, idx, &of_flags);
-
-	if (gpiod_not_found(desc)) {
-		/* Special handling for regulator GPIOs if used */
-		desc = of_find_regulator_gpio(dev, con_id, idx, &of_flags);
-	}
-
-	if (gpiod_not_found(desc))
-		desc = of_find_arizona_gpio(dev, con_id, idx, &of_flags);
-
-	if (gpiod_not_found(desc))
-		desc = of_find_usb_gpio(dev, con_id, idx, &of_flags);
+	/* Properly named GPIO was not found, try workarounds */
+	for (q = of_find_gpio_quirks; gpiod_not_found(desc) && *q; q++)
+		desc = (*q)(dev->of_node, con_id, idx, &of_flags);
 
 	if (IS_ERR(desc))
 		return desc;
-- 
2.37.2.789.g6183377224-goog

[PATCH 4/4] gpiolib: of: factor out conversion from OF flags

[Dmitry Torokhov]

There are several places where we need to convert from OF flags to
"normal" GPIO_* flags, so let's introduce a helper and use it.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/gpio/gpiolib-of.c | 103 ++++++++++++++------------------------
 1 file changed, 37 insertions(+), 66 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 097e948c1d49..95be5f0d2623 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -289,6 +289,36 @@ int of_get_named_gpio_flags(const struct device_node *np, const char *list_name,
 }
 EXPORT_SYMBOL_GPL(of_get_named_gpio_flags);
 
+/* Converts gpio_lookup_flags into bitmask of GPIO_* values */
+static unsigned long of_convert_gpio_flags(enum of_gpio_flags flags)
+{
+	unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
+
+	if (flags & OF_GPIO_ACTIVE_LOW)
+		lflags |= GPIO_ACTIVE_LOW;
+
+	if (flags & OF_GPIO_SINGLE_ENDED) {
+		if (flags & OF_GPIO_OPEN_DRAIN)
+			lflags |= GPIO_OPEN_DRAIN;
+		else
+			lflags |= GPIO_OPEN_SOURCE;
+	}
+
+	if (flags & OF_GPIO_TRANSITORY)
+		lflags |= GPIO_TRANSITORY;
+
+	if (flags & OF_GPIO_PULL_UP)
+		lflags |= GPIO_PULL_UP;
+
+	if (flags & OF_GPIO_PULL_DOWN)
+		lflags |= GPIO_PULL_DOWN;
+
+	if (flags & OF_GPIO_PULL_DISABLE)
+		lflags |= GPIO_PULL_DISABLE;
+
+	return lflags;
+}
+
 /**
  * gpiod_get_from_of_node() - obtain a GPIO from an OF node
  * @node:	handle of the OF node
@@ -308,26 +338,14 @@ struct gpio_desc *gpiod_get_from_of_node(const struct device_node *node,
 					 enum gpiod_flags dflags,
 					 const char *label)
 {
-	unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
+	unsigned long lflags;
 	struct gpio_desc *desc;
-	enum of_gpio_flags flags;
-	bool active_low = false;
-	bool single_ended = false;
-	bool open_drain = false;
-	bool transitory = false;
+	enum of_gpio_flags of_flags;
 	int ret;
 
-	desc = of_get_named_gpiod_flags(node, propname,
-					index, &flags);
-
-	if (!desc || IS_ERR(desc)) {
+	desc = of_get_named_gpiod_flags(node, propname, index, &of_flags);
+	if (!desc || IS_ERR(desc))
 		return desc;
-	}
-
-	active_low = flags & OF_GPIO_ACTIVE_LOW;
-	single_ended = flags & OF_GPIO_SINGLE_ENDED;
-	open_drain = flags & OF_GPIO_OPEN_DRAIN;
-	transitory = flags & OF_GPIO_TRANSITORY;
 
 	ret = gpiod_request(desc, label);
 	if (ret == -EBUSY && (dflags & GPIOD_FLAGS_BIT_NONEXCLUSIVE))
@@ -335,27 +353,7 @@ struct gpio_desc *gpiod_get_from_of_node(const struct device_node *node,
 	if (ret)
 		return ERR_PTR(ret);
 
-	if (active_low)
-		lflags |= GPIO_ACTIVE_LOW;
-
-	if (single_ended) {
-		if (open_drain)
-			lflags |= GPIO_OPEN_DRAIN;
-		else
-			lflags |= GPIO_OPEN_SOURCE;
-	}
-
-	if (transitory)
-		lflags |= GPIO_TRANSITORY;
-
-	if (flags & OF_GPIO_PULL_UP)
-		lflags |= GPIO_PULL_UP;
-
-	if (flags & OF_GPIO_PULL_DOWN)
-		lflags |= GPIO_PULL_DOWN;
-
-	if (flags & OF_GPIO_PULL_DISABLE)
-		lflags |= GPIO_PULL_DISABLE;
+	lflags = of_convert_gpio_flags(of_flags);
 
 	ret = gpiod_configure_flags(desc, propname, lflags, dflags);
 	if (ret < 0) {
@@ -534,25 +532,7 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
 	if (IS_ERR(desc))
 		return desc;
 
-	if (of_flags & OF_GPIO_ACTIVE_LOW)
-		*flags |= GPIO_ACTIVE_LOW;
-
-	if (of_flags & OF_GPIO_SINGLE_ENDED) {
-		if (of_flags & OF_GPIO_OPEN_DRAIN)
-			*flags |= GPIO_OPEN_DRAIN;
-		else
-			*flags |= GPIO_OPEN_SOURCE;
-	}
-
-	if (of_flags & OF_GPIO_TRANSITORY)
-		*flags |= GPIO_TRANSITORY;
-
-	if (of_flags & OF_GPIO_PULL_UP)
-		*flags |= GPIO_PULL_UP;
-	if (of_flags & OF_GPIO_PULL_DOWN)
-		*flags |= GPIO_PULL_DOWN;
-	if (of_flags & OF_GPIO_PULL_DISABLE)
-		*flags |= GPIO_PULL_DISABLE;
+	*flags = of_convert_gpio_flags(of_flags);
 
 	return desc;
 }
@@ -610,16 +590,7 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
 	if (IS_ERR(desc))
 		return desc;
 
-	if (xlate_flags & OF_GPIO_ACTIVE_LOW)
-		*lflags |= GPIO_ACTIVE_LOW;
-	if (xlate_flags & OF_GPIO_TRANSITORY)
-		*lflags |= GPIO_TRANSITORY;
-	if (xlate_flags & OF_GPIO_PULL_UP)
-		*lflags |= GPIO_PULL_UP;
-	if (xlate_flags & OF_GPIO_PULL_DOWN)
-		*lflags |= GPIO_PULL_DOWN;
-	if (xlate_flags & OF_GPIO_PULL_DISABLE)
-		*lflags |= GPIO_PULL_DISABLE;
+	*lflags = of_convert_gpio_flags(xlate_flags);
 
 	if (of_property_read_bool(np, "input"))
 		*dflags |= GPIOD_IN;
-- 
2.37.2.789.g6183377224-goog

Re: [PATCH 1/4] gpiolib: of: do not ignore requested index when applying quirks

[Linus Walleij]

On Thu, Sep 8, 2022 at 7:40 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> We should not ignore index passed into of_find_gpio() when handling
> quirks. While in practice this change will not have any effect, it
> will allow consolidate quirk handling.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 2/4] gpiolib: of: make Freescale SPI quirk similar to all others

[Linus Walleij]

On Thu, Sep 8, 2022 at 7:39 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> There is no need for of_find_spi_cs_gpio() to be different from other
> quirks: the only variant of property actually used in DTS is "gpios"
> (plural) so we can use of_get_named_gpiod_flags() instead of recursing
> into of_find_gpio() again.
>
> This will allow us consolidate quirk handling down the road.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 3/4] gpiolib: rework quirk handling in of_find_gpio()

[Linus Walleij]

On Thu, Sep 8, 2022 at 7:39 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> Instead of having a string of "if" statements let's put all quirks into
> an array and iterate over them.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

That's a change I wanted to do but didn't get around to, thanks a lot!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 4/4] gpiolib: of: factor out conversion from OF flags

[Linus Walleij]

On Thu, Sep 8, 2022 at 7:40 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> There are several places where we need to convert from OF flags to
> "normal" GPIO_* flags, so let's introduce a helper and use it.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 1/4] gpiolib: of: do not ignore requested index when applying quirks

[Bartosz Golaszewski]

On Thu, Sep 8, 2022 at 7:40 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> We should not ignore index passed into of_find_gpio() when handling
> quirks. While in practice this change will not have any effect, it
> will allow consolidate quirk handling.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---

Applied the entire series. Thanks!

Bart

Re: [PATCH 3/4] gpiolib: rework quirk handling in of_find_gpio()

[Conor.Dooley]

On 08/09/2022 06:39, Dmitry Torokhov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Instead of having a string of "if" statements let's put all quirks into
> an array and iterate over them.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Hey,
This seems to have landed in -next today as a2b5e207cade which has
unfortunately broken boot for me:

[    0.765969] Unable to handle kernel paging request at virtual address ffffffad6f697066
[    0.774948] Oops [#1]
[    0.777491] Modules linked in:
[    0.780896] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.0.0-rc1-00037-ga2b5e207cade #1
[    0.789668] Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
[    0.796512] epc : 0xffffffad6f697066
[    0.800491]  ra : of_find_gpio+0x12c/0x1fa
[    0.805066] epc : ffffffad6f697066 ra : ffffffff8042032c sp : ffffffc80400b4e0
[    0.813062]  gp : ffffffff810ef490 tp : ffffffe77fe68000 t0 : 0000000400000000
[    0.821063]  t1 : 0000001000000000 t2 : 0000000000000010 s0 : ffffffc80400b560
[    0.829058]  s1 : ffffffff80c52a88 a0 : ffffffe7bfdf37b0 a1 : ffffffff80d5320e
[    0.837053]  a2 : 0000000000000000 a3 : ffffffc80400b4e4 a4 : 6e61722d6f697067
[    0.845057]  a5 : 0000000000000000 a6 : ffffffff80c4e760 a7 : ffffffe77ffb5b73
[    0.853048]  s2 : ffffffc80400b588 s3 : 0000000000000000 s4 : ffffffe77ffad810
[    0.861052]  s5 : ffffffff810f3098 s6 : ffffffff80d5320e s7 : fffffffffffffffe
[    0.869048]  s8 : fffffffffffff000 s9 : 0000000000000003 s10: 0000000000000000
[    0.877050]  s11: ffffffff80d5320e t3 : 0000000200000000 t4 : ffffffff80c4ebe8
[    0.885045]  t5 : ffffffff80d4ce2e t6 : ffffffe77ffb5b72
[    0.890929] status: 0000000200000120 badaddr: ffffffad6f697066 cause: 000000000000000c
[    0.900316] ---[ end trace 0000000000000000 ]---
[    0.905544] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[    0.914024] SMP: stopping secondary CPUs
[    0.918394] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---

In case it is useful to you, I have gpio nodes in my devicetree
but I am not building a driver that binds to those nodes. Since I
don't have a driver, that's pretty much all of the relevant info
from the boot log. Anything else you need, lmk and I will try to
provide :)

Thanks,
Conor.


> ---
>   drivers/gpio/gpiolib-of.c | 62 ++++++++++++++++-----------------------
>   1 file changed, 26 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index 30b89b694530..097e948c1d49 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -372,14 +372,12 @@ EXPORT_SYMBOL_GPL(gpiod_get_from_of_node);
>    * properties should be named "foo-gpios" so we have this special kludge for
>    * them.
>    */
> -static struct gpio_desc *of_find_spi_gpio(struct device *dev,
> +static struct gpio_desc *of_find_spi_gpio(struct device_node *np,
>                                            const char *con_id,
>                                            unsigned int idx,
>                                            enum of_gpio_flags *of_flags)
>   {
>          char prop_name[32]; /* 32 is max size of property name */
> -       const struct device_node *np = dev->of_node;
> -       struct gpio_desc *desc;
> 
>          /*
>           * Hopefully the compiler stubs the rest of the function if this
> @@ -395,8 +393,7 @@ static struct gpio_desc *of_find_spi_gpio(struct device *dev,
>          /* Will be "gpio-sck", "gpio-mosi" or "gpio-miso" */
>          snprintf(prop_name, sizeof(prop_name), "%s-%s", "gpio", con_id);
> 
> -       desc = of_get_named_gpiod_flags(np, prop_name, idx, of_flags);
> -       return desc;
> +       return of_get_named_gpiod_flags(np, prop_name, idx, of_flags);
>   }
> 
>   /*
> @@ -404,13 +401,11 @@ static struct gpio_desc *of_find_spi_gpio(struct device *dev,
>    * lines rather than "cs-gpios" like all other SPI hardware. Account for this
>    * with a special quirk.
>    */
> -static struct gpio_desc *of_find_spi_cs_gpio(struct device *dev,
> +static struct gpio_desc *of_find_spi_cs_gpio(struct device_node *np,
>                                               const char *con_id,
>                                               unsigned int idx,
>                                               enum of_gpio_flags *of_flags)
>   {
> -       const struct device_node *np = dev->of_node;
> -
>          if (!IS_ENABLED(CONFIG_SPI_MASTER))
>                  return ERR_PTR(-ENOENT);
> 
> @@ -428,7 +423,7 @@ static struct gpio_desc *of_find_spi_cs_gpio(struct device *dev,
>           * uses just "gpios" so translate to that when "cs-gpios" is
>           * requested.
>           */
> -       return of_get_named_gpiod_flags(dev->of_node, "gpios", idx, of_flags);
> +       return of_get_named_gpiod_flags(np, "gpios", idx, of_flags);
>   }
> 
>   /*
> @@ -436,7 +431,7 @@ static struct gpio_desc *of_find_spi_cs_gpio(struct device *dev,
>    * properties should be named "foo-gpios" so we have this special kludge for
>    * them.
>    */
> -static struct gpio_desc *of_find_regulator_gpio(struct device *dev,
> +static struct gpio_desc *of_find_regulator_gpio(struct device_node *np,
>                                                  const char *con_id,
>                                                  unsigned int idx,
>                                                  enum of_gpio_flags *of_flags)
> @@ -447,8 +442,6 @@ static struct gpio_desc *of_find_regulator_gpio(struct device *dev,
>                  "wlf,ldo1ena", /* WM8994 */
>                  "wlf,ldo2ena", /* WM8994 */
>          };
> -       const struct device_node *np = dev->of_node;
> -       struct gpio_desc *desc;
>          int i;
> 
>          if (!IS_ENABLED(CONFIG_REGULATOR))
> @@ -461,11 +454,10 @@ static struct gpio_desc *of_find_regulator_gpio(struct device *dev,
>          if (i < 0)
>                  return ERR_PTR(-ENOENT);
> 
> -       desc = of_get_named_gpiod_flags(np, con_id, idx, of_flags);
> -       return desc;
> +       return of_get_named_gpiod_flags(np, con_id, idx, of_flags);
>   }
> 
> -static struct gpio_desc *of_find_arizona_gpio(struct device *dev,
> +static struct gpio_desc *of_find_arizona_gpio(struct device_node *np,
>                                                const char *con_id,
>                                                unsigned int idx,
>                                                enum of_gpio_flags *of_flags)
> @@ -476,10 +468,10 @@ static struct gpio_desc *of_find_arizona_gpio(struct device *dev,
>          if (!con_id || strcmp(con_id, "wlf,reset"))
>                  return ERR_PTR(-ENOENT);
> 
> -       return of_get_named_gpiod_flags(dev->of_node, con_id, idx, of_flags);
> +       return of_get_named_gpiod_flags(np, con_id, idx, of_flags);
>   }
> 
> -static struct gpio_desc *of_find_usb_gpio(struct device *dev,
> +static struct gpio_desc *of_find_usb_gpio(struct device_node *np,
>                                            const char *con_id,
>                                            unsigned int idx,
>                                            enum of_gpio_flags *of_flags)
> @@ -495,14 +487,27 @@ static struct gpio_desc *of_find_usb_gpio(struct device *dev,
>          if (!con_id || strcmp(con_id, "fcs,int_n"))
>                  return ERR_PTR(-ENOENT);
> 
> -       return of_get_named_gpiod_flags(dev->of_node, con_id, idx, of_flags);
> +       return of_get_named_gpiod_flags(np, con_id, idx, of_flags);
>   }
> 
> +typedef struct gpio_desc *(*of_find_gpio_quirk)(struct device_node *np,
> +                                               const char *con_id,
> +                                               unsigned int idx,
> +                                               enum of_gpio_flags *of_flags);
> +static const of_find_gpio_quirk of_find_gpio_quirks[] = {
> +       of_find_spi_gpio,
> +       of_find_spi_cs_gpio,
> +       of_find_regulator_gpio,
> +       of_find_arizona_gpio,
> +       of_find_usb_gpio,
> +};
> +
>   struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
>                                 unsigned int idx, unsigned long *flags)
>   {
>          char prop_name[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;
> 
> @@ -522,24 +527,9 @@ struct gpio_desc *of_find_gpio(struct device *dev, const char *con_id,
>                          break;
>          }
> 
> -       if (gpiod_not_found(desc)) {
> -               /* Special handling for SPI GPIOs if used */
> -               desc = of_find_spi_gpio(dev, con_id, idx, &of_flags);
> -       }
> -
> -       if (gpiod_not_found(desc))
> -               desc = of_find_spi_cs_gpio(dev, con_id, idx, &of_flags);
> -
> -       if (gpiod_not_found(desc)) {
> -               /* Special handling for regulator GPIOs if used */
> -               desc = of_find_regulator_gpio(dev, con_id, idx, &of_flags);
> -       }
> -
> -       if (gpiod_not_found(desc))
> -               desc = of_find_arizona_gpio(dev, con_id, idx, &of_flags);
> -
> -       if (gpiod_not_found(desc))
> -               desc = of_find_usb_gpio(dev, con_id, idx, &of_flags);
> +       /* Properly named GPIO was not found, try workarounds */
> +       for (q = of_find_gpio_quirks; gpiod_not_found(desc) && *q; q++)
> +               desc = (*q)(dev->of_node, con_id, idx, &of_flags);
> 
>          if (IS_ERR(desc))
>                  return desc;
> --
> 2.37.2.789.g6183377224-goog
> 

Re: [PATCH 3/4] gpiolib: rework quirk handling in of_find_gpio()

[Linus Walleij]

On Fri, Sep 16, 2022 at 1:04 PM <Conor.Dooley@microchip.com> wrote:
>
> On 08/09/2022 06:39, Dmitry Torokhov wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Instead of having a string of "if" statements let's put all quirks into
> > an array and iterate over them.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Hey,
> This seems to have landed in -next today as a2b5e207cade which has
> unfortunately broken boot for me:

Michael Walle has sent a patch fixing it:
https://lore.kernel.org/linux-gpio/20220916110118.446132-1-michael@walle.cc/

Yours,
Linus Walleij

Re: [PATCH 3/4] gpiolib: rework quirk handling in of_find_gpio()

[Conor.Dooley]

On 16/09/2022 12:20, Linus Walleij wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Sep 16, 2022 at 1:04 PM <Conor.Dooley@microchip.com> wrote:
>>
>> On 08/09/2022 06:39, Dmitry Torokhov wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Instead of having a string of "if" statements let's put all quirks into
>>> an array and iterate over them.
>>>
>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>
>> Hey,
>> This seems to have landed in -next today as a2b5e207cade which has
>> unfortunately broken boot for me:
> 
> Michael Walle has sent a patch fixing it:
> https://lore.kernel.org/linux-gpio/20220916110118.446132-1-michael@walle.cc/

Ohh great, I checked before I started writing my mail but didn't
see anything, which makes sense given the times..