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 <s...@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;
+
        /* 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);
-- 
2.30.0.284.gd98b1dd5eaa7-goog

Reply via email to