Hi Patrick, On Wed, 10 Feb 2021 at 01:38, Patrick DELAUNAY <patrick.delau...@foss.st.com> wrote: > > > On 2/9/21 5:28 AM, Simon Glass wrote: > > Hi Patrick, > > > > On Mon, 8 Feb 2021 at 10:33, Patrick DELAUNAY > > <patrick.delau...@foss.st.com> 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 <s...@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); > > 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 .
Yes I am still getting used to it myself! > > Reviewed-by: Patrick Delaunay <patrick.delau...@foss.st.com> > > Tested-by: Patrick Delaunay <patrick.delau...@foss.st.com> Regards, Simon