From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick DELAUNAY Date: Wed, 10 Feb 2021 09:38:49 +0100 Subject: [PATCH v4 10/16] dm: gpio: Add a way to update flags In-Reply-To: References: <20210205042210.2949365-1-sjg@chromium.org> <20210205042210.2949365-11-sjg@chromium.org> Message-ID: <9eb742a5-37b3-82d6-f472-c2861083b66d@foss.st.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 2/9/21 5:28 AM, Simon Glass wrote: > Hi Patrick, > > On Mon, 8 Feb 2021 at 10:33, Patrick DELAUNAY > wrote: >> 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 >>> --- >>> >>> 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); > The bool type should do this automatically. I tested this and it seems > to do the right thing. > I confirmed that it is working, forget my remarks. for information: I tested it in stm32MP157C-DK2 board (with gcc 9.2).... After check, it is my old habit / coding rule, when the bool type don't exist in C (it was a typedef to int) but, since C++ introducing a proper bool type, the cast to 'bool' is enough with current compilers . > > Regards, > SImon Reviewed-by: Patrick Delaunay Tested-by: Patrick Delaunay Regards