[PATCH v4 10/16] dm: gpio: Add a way to update flags
Simon Glass
sjg at chromium.org
Fri Feb 5 05:22:03 CET 2021
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.
Also update the STM32 drivers to let the uclass handle the active low/high
logic.
Drop the GPIOD_FLAGS_OUTPUT() macro which is no-longer used.
Signed-off-by: Simon Glass <sjg at chromium.org>
---
Changes in v4:
- Update dm_gpio_set_dir_flags() to clear direction flags first
Changes in v3:
- Incorporate GPIOD_FLAGS_OUTPUT() changes from Patrick Delaunay
drivers/gpio/gpio-uclass.c | 75 ++++++++++++++----
drivers/gpio/stm32_gpio.c | 3 +-
drivers/pinctrl/pinctrl-stmfx.c | 5 +-
include/asm-generic/gpio.h | 31 ++++++--
test/dm/gpio.c | 132 ++++++++++++++++++++++++++++----
5 files changed, 203 insertions(+), 43 deletions(-)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 8931c420845..daaa1401087 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,38 +661,52 @@ 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);
} else {
if (flags & GPIOD_IS_OUT) {
- ret = ops->direction_output(dev, desc->offset,
- GPIOD_FLAGS_OUTPUT(flags));
+ bool 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);
}
}
- /* 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, GPIOD_MASK_DIR, flags);
}
int dm_gpio_get_flags(struct gpio_desc *desc, ulong *flagsp)
@@ -999,7 +1037,10 @@ static int gpio_request_tail(int ret, const char *nodename,
debug("%s: dm_gpio_requestf failed\n", __func__);
goto err;
}
- ret = dm_gpio_set_dir_flags(desc, flags);
+
+ /* Keep any direction flags provided by the devicetree */
+ ret = dm_gpio_set_dir_flags(desc,
+ flags | (desc->flags & GPIOD_MASK_DIR));
if (ret) {
debug("%s: dm_gpio_set_dir failed\n", __func__);
goto err;
diff --git a/drivers/gpio/stm32_gpio.c b/drivers/gpio/stm32_gpio.c
index c2d7046c0dd..125c237551c 100644
--- a/drivers/gpio/stm32_gpio.c
+++ b/drivers/gpio/stm32_gpio.c
@@ -203,12 +203,13 @@ static int stm32_gpio_set_flags(struct udevice *dev, unsigned int offset,
return idx;
if (flags & GPIOD_IS_OUT) {
- int value = GPIOD_FLAGS_OUTPUT(flags);
+ bool 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), ®s->bsrr);
diff --git a/drivers/pinctrl/pinctrl-stmfx.c b/drivers/pinctrl/pinctrl-stmfx.c
index 8ddbc3dc281..711b1a5d8c4 100644
--- a/drivers/pinctrl/pinctrl-stmfx.c
+++ b/drivers/pinctrl/pinctrl-stmfx.c
@@ -169,6 +169,8 @@ static int stmfx_gpio_set_flags(struct udevice *dev, unsigned int offset,
int ret = -ENOTSUPP;
if (flags & GPIOD_IS_OUT) {
+ bool 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_set_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)
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index f7f10c469c0..af3ce21a7cf 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
@@ -135,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
*
@@ -657,6 +657,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..be5da953045 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,91 @@ 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(GPIOD_IS_OUT, desc.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(GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE, desc.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_asserteq(GPIOD_IS_IN, desc.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);
+ ut_asserteq(GPIOD_IS_IN | GPIOD_PULL_UP, desc.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_asserteq(GPIOD_IS_OUT | GPIOD_ACTIVE_LOW | GPIOD_IS_OUT_ACTIVE,
+ desc.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);
+ ut_asserteq(GPIOD_IS_OUT | GPIOD_ACTIVE_LOW, desc.flags);
+
+ return 0;
+}
+DM_TEST(dm_test_clrset_flags_invert, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
--
2.30.0.478.g8a0d178c01-goog
More information about the U-Boot
mailing list