[PATCH 10/15] dm: gpio: Add a way to update flags
Patrick DELAUNAY
patrick.delaunay at foss.st.com
Thu Jan 21 11:07:47 CET 2021
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 at 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), ®s->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
More information about the U-Boot
mailing list