[PATCH 0/3] gpio: uclass enhancements for DM_GPIO drivers

[PATCH 0/3] gpio: uclass enhancements for DM_GPIO drivers

[Samuel Holland]

This series makes a couple of enhancements to the generic GPIO code that
simplified updating some DM_GPIO drivers.

Patches 1-2 add bounds checking when looking up GPIOs by name and from
the device tree. After this, all functions that fill out a gpio_desc
ensure the offset field is in bounds, so each driver doesn't need
separate bounds checking in its ops functions.

Patch 3 allows the GPIO flag translation code to be shared by drivers
that use the same flags, but cannot use gpio_xlate_offs_flags directly.
For example, the sunxi GPIO binding has 3 cells because it separates the
bank and pin numbers.


Samuel Holland (3):
  gpio: Verify validity of pin offsets when looking up names
  gpio: Verify validity of pin offsets from device trees
  gpio: Factor out DT flag translation

 drivers/gpio/gpio-uclass.c | 55 +++++++++++++++++++++++---------------
 include/asm-generic/gpio.h |  8 ++++++
 2 files changed, 42 insertions(+), 21 deletions(-)

-- 
2.31.1

[PATCH 1/3] gpio: Verify validity of pin offsets when looking up names

[Samuel Holland]

Translation of a pin name to a device+offset should fail if the offset
is larger than the number of pins in the GPIO bank.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpio/gpio-uclass.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 8c77777dbe3..57e87960ee4 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -140,7 +140,8 @@ int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc)
 
 		if (!strncasecmp(name, uc_priv->bank_name, len)) {
 			if (!strict_strtoul(name + len, 10, &offset))
-				break;
+				if (offset < uc_priv->gpio_count)
+					break;
 		}
 
 		/*
-- 
2.31.1

Re: [PATCH 1/3] gpio: Verify validity of pin offsets when looking up names

[Simon Glass]

On Sat, 11 Sept 2021 at 16:05, Samuel Holland <samuel@sholland.org> wrote:
>
> Translation of a pin name to a device+offset should fail if the offset
> is larger than the number of pins in the GPIO bank.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>
>  drivers/gpio/gpio-uclass.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

test?

Re: [PATCH 1/3] gpio: Verify validity of pin offsets when looking up names

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 351 bytes --]

On Sat, Sep 11, 2021 at 05:05:51PM -0500, Samuel Holland wrote:

> Translation of a pin name to a device+offset should fail if the offset
> is larger than the number of pins in the GPIO bank.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 2/3] gpio: Verify validity of pin offsets from device trees

[Samuel Holland]

Translation of an OF GPIO specifier should fail if the pin offset is
larger than the number of pins in the GPIO bank.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpio/gpio-uclass.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 57e87960ee4..6f2b1adfdec 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -188,10 +188,14 @@ int gpio_lookup_name(const char *name, struct udevice **devp,
 int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc,
 			  struct ofnode_phandle_args *args)
 {
+	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+
 	if (args->args_count < 1)
 		return -EINVAL;
 
 	desc->offset = args->args[0];
+	if (desc->offset >= uc_priv->gpio_count)
+		return -EINVAL;
 
 	if (args->args_count < 2)
 		return 0;
-- 
2.31.1

Re: [PATCH 2/3] gpio: Verify validity of pin offsets from device trees

[Simon Glass]

On Sat, 11 Sept 2021 at 16:06, Samuel Holland <samuel@sholland.org> wrote:
>
> Translation of an OF GPIO specifier should fail if the pin offset is
> larger than the number of pins in the GPIO bank.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>
>  drivers/gpio/gpio-uclass.c | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

update gpio test?

Re: [PATCH 2/3] gpio: Verify validity of pin offsets from device trees

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

On Sat, Sep 11, 2021 at 05:05:52PM -0500, Samuel Holland wrote:

> Translation of an OF GPIO specifier should fail if the pin offset is
> larger than the number of pins in the GPIO bank.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 2/3] gpio: Verify validity of pin offsets from device trees

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

On Sat, Sep 11, 2021 at 05:05:52PM -0500, Samuel Holland wrote:

> Translation of an OF GPIO specifier should fail if the pin offset is
> larger than the number of pins in the GPIO bank.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH 3/3] gpio: Factor out DT flag translation

[Samuel Holland]

The generic GPIO flags binding is shared across many drivers, some of
which need their own xlate function. Factor out the flag translation
code from gpio_xlate_offs_flags so it does not need to be duplicated.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 drivers/gpio/gpio-uclass.c | 50 ++++++++++++++++++++++----------------
 include/asm-generic/gpio.h |  8 ++++++
 2 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 6f2b1adfdec..b88b4290a45 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -185,6 +185,34 @@ int gpio_lookup_name(const char *name, struct udevice **devp,
 	return 0;
 }
 
+unsigned long gpio_flags_xlate(uint32_t arg)
+{
+	unsigned long flags = 0;
+
+	if (arg & GPIO_ACTIVE_LOW)
+		flags |= GPIOD_ACTIVE_LOW;
+
+	/*
+	 * need to test 2 bits for gpio output binding:
+	 * OPEN_DRAIN (0x6) = SINGLE_ENDED (0x2) | LINE_OPEN_DRAIN (0x4)
+	 * OPEN_SOURCE (0x2) = SINGLE_ENDED (0x2) | LINE_OPEN_SOURCE (0x0)
+	 */
+	if (arg & GPIO_SINGLE_ENDED) {
+		if (arg & GPIO_LINE_OPEN_DRAIN)
+			flags |= GPIOD_OPEN_DRAIN;
+		else
+			flags |= GPIOD_OPEN_SOURCE;
+	}
+
+	if (arg & GPIO_PULL_UP)
+		flags |= GPIOD_PULL_UP;
+
+	if (arg & GPIO_PULL_DOWN)
+		flags |= GPIOD_PULL_DOWN;
+
+	return flags;
+}
+
 int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc,
 			  struct ofnode_phandle_args *args)
 {
@@ -200,27 +228,7 @@ int gpio_xlate_offs_flags(struct udevice *dev, struct gpio_desc *desc,
 	if (args->args_count < 2)
 		return 0;
 
-	desc->flags = 0;
-	if (args->args[1] & GPIO_ACTIVE_LOW)
-		desc->flags |= GPIOD_ACTIVE_LOW;
-
-	/*
-	 * need to test 2 bits for gpio output binding:
-	 * OPEN_DRAIN (0x6) = SINGLE_ENDED (0x2) | LINE_OPEN_DRAIN (0x4)
-	 * OPEN_SOURCE (0x2) = SINGLE_ENDED (0x2) | LINE_OPEN_SOURCE (0x0)
-	 */
-	if (args->args[1] & GPIO_SINGLE_ENDED) {
-		if (args->args[1] & GPIO_LINE_OPEN_DRAIN)
-			desc->flags |= GPIOD_OPEN_DRAIN;
-		else
-			desc->flags |= GPIOD_OPEN_SOURCE;
-	}
-
-	if (args->args[1] & GPIO_PULL_UP)
-		desc->flags |= GPIOD_PULL_UP;
-
-	if (args->args[1] & GPIO_PULL_DOWN)
-		desc->flags |= GPIOD_PULL_DOWN;
+	desc->flags = gpio_flags_xlate(args->args[1]);
 
 	return 0;
 }
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index e33cde7abdd..911b11bc389 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -221,6 +221,14 @@ int gpio_requestf(unsigned gpio, const char *fmt, ...)
 
 struct fdtdec_phandle_args;
 
+/**
+ * gpio_flags_xlate() - convert DT flags to internal flags
+ *
+ * This routine converts the GPIO_* flags from the generic DT binding to the
+ * GPIOD_* flags used internally. It can be called from driver xlate functions.
+ */
+unsigned long gpio_flags_xlate(uint32_t arg);
+
 /**
  * gpio_xlate_offs_flags() - implementation for common use of dm_gpio_ops.xlate
  *
-- 
2.31.1

Re: [PATCH 3/3] gpio: Factor out DT flag translation

[Simon Glass]

On Sat, 11 Sept 2021 at 16:06, Samuel Holland <samuel@sholland.org> wrote:
>
> The generic GPIO flags binding is shared across many drivers, some of
> which need their own xlate function. Factor out the flag translation
> code from gpio_xlate_offs_flags so it does not need to be duplicated.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>
>  drivers/gpio/gpio-uclass.c | 50 ++++++++++++++++++++++----------------
>  include/asm-generic/gpio.h |  8 ++++++
>  2 files changed, 37 insertions(+), 21 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 3/3] gpio: Factor out DT flag translation

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 440 bytes --]

On Sat, Sep 11, 2021 at 05:05:53PM -0500, Samuel Holland wrote:

> The generic GPIO flags binding is shared across many drivers, some of
> which need their own xlate function. Factor out the flag translation
> code from gpio_xlate_offs_flags so it does not need to be duplicated.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]