[PATCH v4 10/16] dm: gpio: Add a way to update flags
Patrick DELAUNAY
patrick.delaunay at foss.st.com
Mon Feb 8 18:33:03 CET 2021
Hi Simon,
2 minor remarks,
On 2/5/21 5:22 AM, 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.
>
> 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/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;
here the bool variable valeu can be 0 or GPIOD_IS_OUT_ACTIVE = BIT(4),
so it should have
+ bool value = !!(flags & GPIOD_IS_OUT_ACTIVE);
or
+ int value = flags & GPIOD_IS_OUT_ACTIVE;
PS: not functional impact as
#define BSRR_BIT(gpio_pin, value) BIT((gpio_pin) + (value ? 0 : 16))
>
> 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;
> +
same here
+ int value = flags & GPIOD_IS_OUT_ACTIVE;
or
+ bool value = !!(flags & GPIOD_IS_OUT_ACTIVE);
But no impact
> 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)
(...)
With the 2 remarks
Reviewed-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
Tested-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
Regards,
Patrick
More information about the U-Boot
mailing list