diff mbox series

[10/15] dm: gpio: Add a way to update flags

Message ID 20210115140500.846307-11-sjg@chromium.org
State Superseded
Delegated to: Simon Glass
Headers show
Series gpio: Update and simplify the uclass API | expand

Commit Message

Simon Glass Jan. 15, 2021, 2:04 p.m. UTC
It is convenient to be able to adjust some of the flags for a GPIO while
leaving others alone. Add a function for this.

Update dm_gpio_set_dir_flags() to make use of this.

Also update dm_gpio_set_value() to use this also, since this allows the
open-drain / open-source features to be implemented directly in the
driver, rather than using the uclass workaround.

Update the sandbox tests accordingly. This involves a lot of changes to
dm_test_gpio_opendrain_opensource() since we no-longer have the direciion
being reported differently depending on the open drain/open source flags.

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

 drivers/gpio/gpio-uclass.c |  65 ++++++++++++++-----
 include/asm-generic/gpio.h |  25 ++++++++
 test/dm/gpio.c             | 125 ++++++++++++++++++++++++++++++++-----
 3 files changed, 184 insertions(+), 31 deletions(-)

Comments

Patrick DELAUNAY Jan. 21, 2021, 10:07 a.m. UTC | #1
Hi Simon,

On 1/15/21 3:04 PM, Simon Glass wrote:
> It is convenient to be able to adjust some of the flags for a GPIO while
> leaving others alone. Add a function for this.
>
> Update dm_gpio_set_dir_flags() to make use of this.
>
> Also update dm_gpio_set_value() to use this also, since this allows the
> open-drain / open-source features to be implemented directly in the
> driver, rather than using the uclass workaround.
>
> Update the sandbox tests accordingly. This involves a lot of changes to
> dm_test_gpio_opendrain_opensource() since we no-longer have the direciion
> being reported differently depending on the open drain/open source flags.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   drivers/gpio/gpio-uclass.c |  65 ++++++++++++++-----
>   include/asm-generic/gpio.h |  25 ++++++++
>   test/dm/gpio.c             | 125 ++++++++++++++++++++++++++++++++-----
>   3 files changed, 184 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> index aa0e3852122..05627ecdc30 100644
> --- a/drivers/gpio/gpio-uclass.c
> +++ b/drivers/gpio/gpio-uclass.c
> @@ -568,6 +568,7 @@ int dm_gpio_get_value(const struct gpio_desc *desc)
>   
>   int dm_gpio_set_value(const struct gpio_desc *desc, int value)
>   {
> +	const struct dm_gpio_ops *ops;
>   	int ret;
>   
>   	ret = check_reserved(desc, "set_value");
> @@ -577,21 +578,33 @@ int dm_gpio_set_value(const struct gpio_desc *desc, int value)
>   	if (desc->flags & GPIOD_ACTIVE_LOW)
>   		value = !value;
>   
> +	/* GPIOD_ are directly managed by driver in update_flags */
> +	ops = gpio_get_ops(desc->dev);
> +	if (ops->update_flags) {
> +		ulong flags = desc->flags;
> +
> +		if (value)
> +			flags |= GPIOD_IS_OUT_ACTIVE;
> +		else
> +			flags &= ~GPIOD_IS_OUT_ACTIVE;
> +		return ops->update_flags(desc->dev, desc->offset, flags);
> +	}
> +
>   	/*
>   	 * Emulate open drain by not actively driving the line high or
>   	 * Emulate open source by not actively driving the line low
>   	 */
>   	if ((desc->flags & GPIOD_OPEN_DRAIN && value) ||
>   	    (desc->flags & GPIOD_OPEN_SOURCE && !value))
> -		return gpio_get_ops(desc->dev)->direction_input(desc->dev,
> -								desc->offset);
> +		return ops->direction_input(desc->dev, desc->offset);
>   	else if (desc->flags & GPIOD_OPEN_DRAIN ||
>   		 desc->flags & GPIOD_OPEN_SOURCE)
> -		return gpio_get_ops(desc->dev)->direction_output(desc->dev,
> -								desc->offset,
> -								value);
> +		return ops->direction_output(desc->dev, desc->offset, value);
> +
> +	ret = ops->set_value(desc->dev, desc->offset, value);
> +	if (ret)
> +		return ret;
>   
> -	gpio_get_ops(desc->dev)->set_value(desc->dev, desc->offset, value);
>   	return 0;
>   }
>   
> @@ -619,6 +632,17 @@ static int check_dir_flags(ulong flags)
>   	return 0;
>   }
>   
> +/**
> + * _dm_gpio_update_flags() - Send flags to the driver
> + *
> + * This uses the best available method to send the given flags to the driver.
> + * Note that if flags & GPIOD_ACTIVE_LOW, the driver sees the opposite value
> + * of GPIOD_IS_OUT_ACTIVE.
> + *
> + * @desc:	GPIO description
> + * @flags:	flags value to set
> + * @return 0 if OK, -ve on error
> + */
>   static int _dm_gpio_update_flags(struct gpio_desc *desc, ulong flags)
>   {
>   	struct udevice *dev = desc->dev;
> @@ -637,6 +661,11 @@ static int _dm_gpio_update_flags(struct gpio_desc *desc, ulong flags)
>   		return ret;
>   	}
>   
> +	/* If active low, invert the output state */
> +	if ((flags & (GPIOD_IS_OUT | GPIOD_ACTIVE_LOW)) ==
> +		(GPIOD_IS_OUT | GPIOD_ACTIVE_LOW))
> +		flags ^= GPIOD_IS_OUT_ACTIVE;
> +


This modifciation imply that GPIOD_ACTIVE_LOW must no more managed in 
ops update_flags

(as indicated previously in the serie in ops description)



>   	/* GPIOD_ are directly managed by driver in update_flags */
>   	if (ops->update_flags) {
>   		ret = ops->update_flags(dev, desc->offset, flags);
> @@ -649,26 +678,34 @@ static int _dm_gpio_update_flags(struct gpio_desc *desc, ulong flags)
>   		}
>   	}
>   
> -	/* save the flags also in descriptor */
> -	if (!ret)
> -		desc->flags = flags;
> -
>   	return ret;
>   }
>   
(...)

In current STM32 implementation, the set_dir_flags could be called with 
GPIOD_ACTIVE_LOW, so it was managed in ops set_dir_flags()

it was hndle by using the macro GPIOD_FLAGS_OUTPUT.


As after the serie the GPIOD_ACTIVE_LOW must no more mamaged in driver: 
I agree that is is more clean / simple.


But this patch need also modification in  driver using GPIOD_FLAGS_OUTPUT

drivers/gpio/stm32_gpio.c:208:        int value = GPIOD_FLAGS_OUTPUT(flags);

drivers/gpio/gpio-uclass.c:680: GPIOD_FLAGS_OUTPUT(flags));


I think GPIOD_FLAGS_OUTPUT could be remove, as it is only used in

drivers/gpio/stm32_gpio.c::stm32_gpio_update_flags()


Something as:


-------------------------- drivers/gpio/gpio-uclass.c 
--------------------------
index b240ddd3d9..45fe791431 100644
@@ -654,6 +654,7 @@ static int _dm_gpio_update_flags(struct gpio_desc 
*desc, ulong flags)
      const struct dm_gpio_ops *ops = gpio_get_ops(dev);
      struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
      int ret = 0;
+    int value;

      ret = check_dir_flags(flags);
      if (ret) {
@@ -676,8 +677,8 @@ static int _dm_gpio_update_flags(struct gpio_desc 
*desc, ulong flags)
          ret = ops->update_flags(dev, desc->offset, flags);
      } else {
          if (flags & GPIOD_IS_OUT) {
-            ret = ops->direction_output(dev, desc->offset,
-                            GPIOD_FLAGS_OUTPUT(flags));
+            value = !!(flags & GPIOD_IS_OUT_ACTIVE);
+            ret = ops->direction_output(dev, desc->offset, value);
          } else if (flags & GPIOD_IS_IN) {
              ret = ops->direction_input(dev, desc->offset);
          }

-------------------------- drivers/gpio/stm32_gpio.c 
--------------------------
index da9a40ebf8..d9ad50f3c9 100644
@@ -205,12 +205,13 @@ static int stm32_gpio_update_flags(struct udevice 
*dev, unsigned int offset,
      printf("%s: %s %d flags = %lx\n", __func__, dev->name, idx, flags);

      if (flags & GPIOD_IS_OUT) {
-        int value = GPIOD_FLAGS_OUTPUT(flags);
+        int value = !!(flags & GPIOD_IS_OUT_ACTIVE);

          if (flags & GPIOD_OPEN_DRAIN)
              stm32_gpio_set_otype(regs, idx, STM32_GPIO_OTYPE_OD);
          else
              stm32_gpio_set_otype(regs, idx, STM32_GPIO_OTYPE_PP);
+
          stm32_gpio_set_moder(regs, idx, STM32_GPIO_MODE_OUT);
          writel(BSRR_BIT(idx, value), &regs->bsrr);
          printf("%s: %s %d moder = %x value = %d\n",
@@ -304,7 +305,7 @@ static int gpio_stm32_probe(struct udevice *dev)
      ret = dev_read_phandle_with_args(dev, "gpio-ranges",
                       NULL, 3, i, &args);

-    dev_dbg(dev, "dev_read_phandle_with_args = %d %d %d\n", ret, 
-ENOENT, dev_read_u32_default(dev, "ngpios", 0));
+    dev_dbg(dev, "dev_read_phandle_with_args(%d) = %d %d %d\n", i, ret, 
-ENOENT, dev_read_u32_default(dev, "ngpios", 0));

      if (!ret && args.args_count < 3)
          return -EINVAL;
@@ -322,6 +323,8 @@ static int gpio_stm32_probe(struct udevice *dev)

          ret = dev_read_phandle_with_args(dev, "gpio-ranges", NULL, 3,
                           ++i, &args);
+        dev_dbg(dev, "dev_read_phandle_with_args(%d) = %d %d %d\n", 
i-1, ret, -ENOENT, dev_read_u32_default(dev, "ngpios", 0));
+
          if (!ret && args.args_count < 3)
              return -EINVAL;
      }

----------------------- drivers/pinctrl/pinctrl-stmfx.c 
-----------------------
index 6477febbaa..e2c95d8d42 100644
@@ -169,6 +169,8 @@ static int stmfx_gpio_update_flags(struct udevice 
*dev, unsigned int offset,
      int ret = -ENOTSUPP;

      if (flags & GPIOD_IS_OUT) {
+        int value = !!(flags & GPIOD_IS_OUT_ACTIVE);
+
          if (flags & GPIOD_OPEN_SOURCE)
              return -ENOTSUPP;
          if (flags & GPIOD_OPEN_DRAIN)
@@ -177,8 +179,7 @@ static int stmfx_gpio_update_flags(struct udevice 
*dev, unsigned int offset,
              ret = stmfx_conf_set_type(dev, offset, 1);
          if (ret)
              return ret;
-        ret = stmfx_gpio_direction_output(dev, offset,
-                          GPIOD_FLAGS_OUTPUT(flags));
+        ret = stmfx_gpio_direction_output(dev, offset, value);
      } else if (flags & GPIOD_IS_IN) {
          ret = stmfx_gpio_direction_input(dev, offset);
          if (ret)

-------------------------- include/asm-generic/gpio.h 
--------------------------
index 1cf81a3fc7..62514db5be 100644
@@ -141,12 +141,6 @@ struct gpio_desc {
       */
  };

-/* helper to compute the value of the gpio output */
-#define GPIOD_FLAGS_OUTPUT_MASK (GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE)
-#define GPIOD_FLAGS_OUTPUT(flags) \
-    (((((flags) & GPIOD_FLAGS_OUTPUT_MASK) == GPIOD_IS_OUT_ACTIVE) || \
-      (((flags) & GPIOD_FLAGS_OUTPUT_MASK) == GPIOD_ACTIVE_LOW)))
-
  /**
   * dm_gpio_is_valid() - Check if a GPIO is valid
   *



Regards

Patrick
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index aa0e3852122..05627ecdc30 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -568,6 +568,7 @@  int dm_gpio_get_value(const struct gpio_desc *desc)
 
 int dm_gpio_set_value(const struct gpio_desc *desc, int value)
 {
+	const struct dm_gpio_ops *ops;
 	int ret;
 
 	ret = check_reserved(desc, "set_value");
@@ -577,21 +578,33 @@  int dm_gpio_set_value(const struct gpio_desc *desc, int value)
 	if (desc->flags & GPIOD_ACTIVE_LOW)
 		value = !value;
 
+	/* GPIOD_ are directly managed by driver in update_flags */
+	ops = gpio_get_ops(desc->dev);
+	if (ops->update_flags) {
+		ulong flags = desc->flags;
+
+		if (value)
+			flags |= GPIOD_IS_OUT_ACTIVE;
+		else
+			flags &= ~GPIOD_IS_OUT_ACTIVE;
+		return ops->update_flags(desc->dev, desc->offset, flags);
+	}
+
 	/*
 	 * Emulate open drain by not actively driving the line high or
 	 * Emulate open source by not actively driving the line low
 	 */
 	if ((desc->flags & GPIOD_OPEN_DRAIN && value) ||
 	    (desc->flags & GPIOD_OPEN_SOURCE && !value))
-		return gpio_get_ops(desc->dev)->direction_input(desc->dev,
-								desc->offset);
+		return ops->direction_input(desc->dev, desc->offset);
 	else if (desc->flags & GPIOD_OPEN_DRAIN ||
 		 desc->flags & GPIOD_OPEN_SOURCE)
-		return gpio_get_ops(desc->dev)->direction_output(desc->dev,
-								desc->offset,
-								value);
+		return ops->direction_output(desc->dev, desc->offset, value);
+
+	ret = ops->set_value(desc->dev, desc->offset, value);
+	if (ret)
+		return ret;
 
-	gpio_get_ops(desc->dev)->set_value(desc->dev, desc->offset, value);
 	return 0;
 }
 
@@ -619,6 +632,17 @@  static int check_dir_flags(ulong flags)
 	return 0;
 }
 
+/**
+ * _dm_gpio_update_flags() - Send flags to the driver
+ *
+ * This uses the best available method to send the given flags to the driver.
+ * Note that if flags & GPIOD_ACTIVE_LOW, the driver sees the opposite value
+ * of GPIOD_IS_OUT_ACTIVE.
+ *
+ * @desc:	GPIO description
+ * @flags:	flags value to set
+ * @return 0 if OK, -ve on error
+ */
 static int _dm_gpio_update_flags(struct gpio_desc *desc, ulong flags)
 {
 	struct udevice *dev = desc->dev;
@@ -637,6 +661,11 @@  static int _dm_gpio_update_flags(struct gpio_desc *desc, ulong flags)
 		return ret;
 	}
 
+	/* If active low, invert the output state */
+	if ((flags & (GPIOD_IS_OUT | GPIOD_ACTIVE_LOW)) ==
+		(GPIOD_IS_OUT | GPIOD_ACTIVE_LOW))
+		flags ^= GPIOD_IS_OUT_ACTIVE;
+
 	/* GPIOD_ are directly managed by driver in update_flags */
 	if (ops->update_flags) {
 		ret = ops->update_flags(dev, desc->offset, flags);
@@ -649,26 +678,34 @@  static int _dm_gpio_update_flags(struct gpio_desc *desc, ulong flags)
 		}
 	}
 
-	/* save the flags also in descriptor */
-	if (!ret)
-		desc->flags = flags;
-
 	return ret;
 }
 
-int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
+int dm_gpio_clrset_flags(struct gpio_desc *desc, ulong clr, ulong set)
 {
+	ulong flags;
 	int ret;
 
 	ret = check_reserved(desc, "set_dir_flags");
 	if (ret)
 		return ret;
 
-	/* combine the requested flags (for IN/OUT) and the descriptor flags */
-	flags |= desc->flags;
+	flags = (desc->flags & ~clr) | set;
+
 	ret = _dm_gpio_update_flags(desc, flags);
+	if (ret)
+		return ret;
 
-	return ret;
+	/* save the flags also in descriptor */
+	desc->flags = flags;
+
+	return 0;
+}
+
+int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
+{
+	/* combine the requested flags (for IN/OUT) and the descriptor flags */
+	return dm_gpio_clrset_flags(desc, 0, flags);
 }
 
 int dm_gpio_get_flags(struct gpio_desc *desc, ulong *flagsp)
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 30ff5feb160..4a657b1bd2b 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -128,6 +128,12 @@  struct gpio_desc {
 #define GPIOD_PULL_UP		BIT(7)	/* GPIO has pull-up enabled */
 #define GPIOD_PULL_DOWN		BIT(8)	/* GPIO has pull-down enabled */
 
+/* Flags for updating the above */
+#define GPIOD_MASK_DIR		(GPIOD_IS_OUT | GPIOD_IS_IN | \
+					GPIOD_IS_OUT_ACTIVE)
+#define GPIOD_MASK_DSTYPE	(GPIOD_OPEN_DRAIN | GPIOD_OPEN_SOURCE)
+#define GPIOD_MASK_PULL		(GPIOD_PULL_UP | GPIOD_PULL_DOWN)
+
 	uint offset;		/* GPIO offset within the device */
 	/*
 	 * We could consider adding the GPIO label in here. Possibly we could
@@ -659,6 +665,25 @@  int dm_gpio_get_value(const struct gpio_desc *desc);
 
 int dm_gpio_set_value(const struct gpio_desc *desc, int value);
 
+/**
+ * dm_gpio_clrset_flags() - Update flags
+ *
+ * This updates the flags as directled. Note that desc->flags is updated by this
+ * function on success. If any changes cannot be made, best efforts are made.
+ *
+ * By use of @clr and @set any of flags can be individually updated, or left
+ * alone
+ *
+ * @desc:	GPIO description containing device, offset and flags,
+ *		previously returned by gpio_request_by_name()
+ * @clr:	Flags to clear (GPIOD_...)
+ * @set:	Flags to set (GPIOD_...)
+ * @return 0 if OK, -EINVAL if the flags had obvious conflicts,
+ * -ERECALLCONFLICT if there was a non-obvious hardware conflict when attempting
+ * to set the flags
+ */
+int dm_gpio_clrset_flags(struct gpio_desc *desc, ulong clr, ulong set);
+
 /**
  * dm_gpio_set_dir_flags() - Set direction using description and added flags
  *
diff --git a/test/dm/gpio.c b/test/dm/gpio.c
index 43e868cd4ea..eaf78e9aed8 100644
--- a/test/dm/gpio.c
+++ b/test/dm/gpio.c
@@ -178,15 +178,15 @@  static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts)
 	ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN,
 		    sandbox_gpio_get_flags(gpio_c, 0));
 
-	/* Set it as output high, should become an input */
+	/* Set it as output high */
 	ut_assertok(dm_gpio_set_value(&desc_list[0], 1));
-	ut_assertok(gpio_get_status(gpio_c, 0, buf, sizeof(buf)));
-	ut_asserteq_str("c0: input: 0 [x] a-test.test3-gpios0", buf);
+	ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN | GPIOD_IS_OUT_ACTIVE,
+		    sandbox_gpio_get_flags(gpio_c, 0));
 
-	/* Set it as output low, should become output low */
+	/* Set it as output low */
 	ut_assertok(dm_gpio_set_value(&desc_list[0], 0));
-	ut_assertok(gpio_get_status(gpio_c, 0, buf, sizeof(buf)));
-	ut_asserteq_str("c0: output: 0 [x] a-test.test3-gpios0", buf);
+	ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_DRAIN,
+		    sandbox_gpio_get_flags(gpio_c, 0));
 
 	/* GPIO 1 is (GPIO_OUT|GPIO_OPEN_SOURCE) */
 	ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_SOURCE,
@@ -197,13 +197,21 @@  static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts)
 	ut_assertok(gpio_get_status(gpio_c, 1, buf, sizeof(buf)));
 	ut_asserteq_str("c1: output: 1 [x] a-test.test3-gpios1", buf);
 
-	/* Set it as output low, should become an input */
+	/* Set it as output low */
 	ut_assertok(dm_gpio_set_value(&desc_list[1], 0));
+	ut_asserteq(GPIOD_IS_OUT | GPIOD_OPEN_SOURCE,
+		    sandbox_gpio_get_flags(gpio_c, 1));
+
 	ut_assertok(gpio_get_status(gpio_c, 1, buf, sizeof(buf)));
-	ut_asserteq_str("c1: input: 1 [x] a-test.test3-gpios1", buf);
+	ut_asserteq_str("c1: output: 0 [x] a-test.test3-gpios1", buf);
 
-	/* GPIO 6 is (GPIO_ACTIVE_LOW|GPIO_OUT|GPIO_OPEN_DRAIN) */
-	ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_DRAIN,
+	/*
+	 * GPIO 6 is (GPIO_ACTIVE_LOW|GPIO_OUT|GPIO_OPEN_DRAIN). Looking at it
+	 * directlt from the driver, we get GPIOD_IS_OUT_ACTIVE also, since it
+	 * is active low
+	 */
+	ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_DRAIN |
+		    GPIOD_IS_OUT_ACTIVE,
 		    sandbox_gpio_get_flags(gpio_c, 6));
 
 	/* Set it as output high, should become output low */
@@ -211,19 +219,21 @@  static int dm_test_gpio_opendrain_opensource(struct unit_test_state *uts)
 	ut_assertok(gpio_get_status(gpio_c, 6, buf, sizeof(buf)));
 	ut_asserteq_str("c6: output: 0 [x] a-test.test3-gpios6", buf);
 
-	/* Set it as output low, should become an input */
+	/* Set it as output low */
 	ut_assertok(dm_gpio_set_value(&desc_list[6], 0));
-	ut_assertok(gpio_get_status(gpio_c, 6, buf, sizeof(buf)));
-	ut_asserteq_str("c6: input: 0 [x] a-test.test3-gpios6", buf);
+	ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_DRAIN |
+		    GPIOD_IS_OUT_ACTIVE,
+		    sandbox_gpio_get_flags(gpio_c, 6));
 
 	/* GPIO 7 is (GPIO_ACTIVE_LOW|GPIO_OUT|GPIO_OPEN_SOURCE) */
-	ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_SOURCE,
+	ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_SOURCE |
+		    GPIOD_IS_OUT_ACTIVE,
 		    sandbox_gpio_get_flags(gpio_c, 7));
 
-	/* Set it as output high, should become an input */
+	/* Set it as output high */
 	ut_assertok(dm_gpio_set_value(&desc_list[7], 1));
-	ut_assertok(gpio_get_status(gpio_c, 7, buf, sizeof(buf)));
-	ut_asserteq_str("c7: input: 0 [x] a-test.test3-gpios7", buf);
+	ut_asserteq(GPIOD_ACTIVE_LOW | GPIOD_IS_OUT | GPIOD_OPEN_SOURCE,
+		    sandbox_gpio_get_flags(gpio_c, 7));
 
 	/* Set it as output low, should become output high */
 	ut_assertok(dm_gpio_set_value(&desc_list[7], 0));
@@ -582,3 +592,84 @@  static int dm_test_gpio_devm(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_gpio_devm, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+static int dm_test_clrset_flags(struct unit_test_state *uts)
+{
+	struct gpio_desc desc;
+	struct udevice *dev;
+	ulong flags;
+
+	ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));
+	ut_asserteq_str("a-test", dev->name);
+	ut_assertok(gpio_request_by_name(dev, "test-gpios", 1, &desc, 0));
+
+	ut_assertok(dm_gpio_clrset_flags(&desc, GPIOD_MASK_DIR, GPIOD_IS_OUT));
+	ut_assertok(dm_gpio_get_flags(&desc, &flags));
+	ut_asserteq(GPIOD_IS_OUT, flags);
+	ut_asserteq(0, sandbox_gpio_get_value(desc.dev, desc.offset));
+
+	ut_assertok(dm_gpio_clrset_flags(&desc, 0, GPIOD_IS_OUT_ACTIVE));
+	ut_assertok(dm_gpio_get_flags(&desc, &flags));
+	ut_asserteq(GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE, flags);
+	ut_asserteq(1, sandbox_gpio_get_value(desc.dev, desc.offset));
+	ut_asserteq(1, dm_gpio_get_value(&desc));
+
+	ut_assertok(dm_gpio_clrset_flags(&desc, GPIOD_MASK_DIR, GPIOD_IS_IN));
+	ut_assertok(dm_gpio_get_flags(&desc, &flags));
+	ut_asserteq(GPIOD_IS_IN, flags & GPIOD_MASK_DIR);
+
+	ut_assertok(dm_gpio_clrset_flags(&desc, GPIOD_MASK_PULL,
+					 GPIOD_PULL_UP));
+	ut_assertok(dm_gpio_get_flags(&desc, &flags));
+	ut_asserteq(GPIOD_IS_IN | GPIOD_PULL_UP, flags);
+
+	/* Check we cannot set both PULL_UP and PULL_DOWN */
+	ut_asserteq(-EINVAL, dm_gpio_clrset_flags(&desc, 0, GPIOD_PULL_DOWN));
+
+	return 0;
+}
+DM_TEST(dm_test_clrset_flags, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+/* Check that an active-low GPIO works as expected */
+static int dm_test_clrset_flags_invert(struct unit_test_state *uts)
+{
+	struct gpio_desc desc;
+	struct udevice *dev;
+	ulong flags;
+
+	ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));
+	ut_asserteq_str("a-test", dev->name);
+	ut_assertok(gpio_request_by_name(dev, "test-gpios", 1, &desc,
+					 GPIOD_IS_OUT | GPIOD_ACTIVE_LOW));
+
+	/*
+	 * From this size we see it as 0 (active low), but the sandbox driver
+	 * sees the pin value high
+	 */
+	ut_asserteq(0, dm_gpio_get_value(&desc));
+	ut_asserteq(1, sandbox_gpio_get_value(desc.dev, desc.offset));
+
+	ut_assertok(dm_gpio_set_value(&desc, 1));
+	ut_asserteq(1, dm_gpio_get_value(&desc));
+	ut_asserteq(0, sandbox_gpio_get_value(desc.dev, desc.offset));
+
+	/* Do the same with dm_gpio_clrset_flags() */
+	ut_assertok(dm_gpio_clrset_flags(&desc, GPIOD_IS_OUT_ACTIVE, 0));
+	ut_asserteq(0, dm_gpio_get_value(&desc));
+	ut_asserteq(1, sandbox_gpio_get_value(desc.dev, desc.offset));
+
+	ut_assertok(dm_gpio_clrset_flags(&desc, 0, GPIOD_IS_OUT_ACTIVE));
+	ut_asserteq(1, dm_gpio_get_value(&desc));
+	ut_asserteq(0, sandbox_gpio_get_value(desc.dev, desc.offset));
+
+	ut_assertok(dm_gpio_get_flags(&desc, &flags));
+	ut_asserteq(GPIOD_IS_OUT | GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE,
+		    flags);
+
+	ut_assertok(dm_gpio_clrset_flags(&desc, GPIOD_IS_OUT_ACTIVE, 0));
+	ut_assertok(dm_gpio_get_flags(&desc, &flags));
+	ut_asserteq(GPIOD_IS_OUT | GPIOD_ACTIVE_LOW, flags);
+
+	return 0;
+}
+DM_TEST(dm_test_clrset_flags_invert, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);