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 ---
(no changes since v1)
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 8931c420845..3492086725e 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 set_flags */ + ops = gpio_get_ops(desc->dev); + if (ops->set_flags) { + ulong flags = desc->flags; + + if (value) + flags |= GPIOD_IS_OUT_ACTIVE; + else + flags &= ~GPIOD_IS_OUT_ACTIVE; + return ops->set_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_set_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_set_flags(struct gpio_desc *desc, ulong flags) { struct udevice *dev = desc->dev; @@ -637,6 +661,11 @@ static int _dm_gpio_set_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 set_flags */ if (ops->set_flags) { ret = ops->set_flags(dev, desc->offset, flags); @@ -649,26 +678,34 @@ static int _dm_gpio_set_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_set_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 f7f10c469c0..947b65d9d0d 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 @@ -657,6 +663,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 dfbb634bf71..7b12277b32d 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);