From mboxrd@z Thu Jan 1 00:00:00 1970 From: Patrick DELAUNAY Date: Thu, 21 Jan 2021 10:33:17 +0100 Subject: [PATCH 07/15] gpio: sandbox: Use a separate flag for the value In-Reply-To: <20210115140500.846307-8-sjg@chromium.org> References: <20210115140500.846307-1-sjg@chromium.org> <20210115140500.846307-8-sjg@chromium.org> Message-ID: <1f79052d-f470-6d8b-c22f-b8a1c3e10364@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 Hi Simon, On 1/15/21 3:04 PM, Simon Glass wrote: > At present with the sandbox GPIO driver it is not possible to change the > value of GPIOD_IS_OUT_ACTIVE unless the GPIO is an output. This makes it > hard to test changing the flags since we need to be aware of the internal > workings of the driver. > > The feature is designed to aid testing. > > Split this feature out into a separate sandbox-specific flag, so that the > flags can change unimpeded. This will make it easier to allow updating the > flags in a future patch. > > Signed-off-by: Simon Glass > --- > > arch/sandbox/include/asm/gpio.h | 5 ++++ > drivers/gpio/sandbox.c | 43 +++++++++++++++------------------ > 2 files changed, 25 insertions(+), 23 deletions(-) > > diff --git a/arch/sandbox/include/asm/gpio.h b/arch/sandbox/include/asm/gpio.h > index 20d78296551..3f267797644 100644 > --- a/arch/sandbox/include/asm/gpio.h > +++ b/arch/sandbox/include/asm/gpio.h > @@ -23,6 +23,11 @@ > */ > #include > > +/* Our own private GPIO flags, which musn't conflict with GPIOD_... */ > +#define GPIOD_EXT_HIGH BIT(20) /* external source is high (else low) */ > + > +#define GPIOD_SANDBOX_MASK BIT(20) > + This internal SANDBOX is starting at 20 to avoid possible conflict with? GPIOD_ It should start at BIT(32) and decreasing order to allow more possible GPIOD for SANDOX or in include/asm/gpio.h > /** > * Return the simulated value of a GPIO (used only in sandbox test code) > * > diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c > index 9ce5d823505..52e73e2300a 100644 > --- a/drivers/gpio/sandbox.c > +++ b/drivers/gpio/sandbox.c > @@ -59,12 +59,12 @@ static int get_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag) > static int set_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag, > int value) > { > - ulong *gpio = get_gpio_flags(dev, offset); > + struct gpio_state *state = get_gpio_state(dev, offset); > > if (value) > - *gpio |= flag; > + state->flags |= flag; > else > - *gpio &= ~flag; > + state->flags &= ~flag; > > return 0; > } > @@ -75,14 +75,19 @@ static int set_gpio_flag(struct udevice *dev, unsigned int offset, ulong flag, > > int sandbox_gpio_get_value(struct udevice *dev, unsigned offset) > { > + struct gpio_state *state = get_gpio_state(dev, offset); > + > if (get_gpio_flag(dev, offset, GPIOD_IS_OUT)) > debug("sandbox_gpio: get_value on output gpio %u\n", offset); > - return get_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE); > + > + return state->flags & GPIOD_EXT_HIGH ? true : false; > } > > int sandbox_gpio_set_value(struct udevice *dev, unsigned offset, int value) > { > - return set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE, value); > + set_gpio_flag(dev, offset, GPIOD_IS_OUT_ACTIVE | GPIOD_EXT_HIGH, value); > + > + return 0; > } > > int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset) > @@ -93,19 +98,25 @@ int sandbox_gpio_get_direction(struct udevice *dev, unsigned offset) > int sandbox_gpio_set_direction(struct udevice *dev, unsigned offset, int output) > { > set_gpio_flag(dev, offset, GPIOD_IS_OUT, output); > - set_gpio_flag(dev, offset, GPIOD_IS_IN, !(output)); > + set_gpio_flag(dev, offset, GPIOD_IS_IN, !output); > > return 0; > } > > ulong sandbox_gpio_get_flags(struct udevice *dev, uint offset) > { > - return *get_gpio_flags(dev, offset); > + ulong flags = *get_gpio_flags(dev, offset); > + > + return flags & ~GPIOD_SANDBOX_MASK; > } > > int sandbox_gpio_set_flags(struct udevice *dev, uint offset, ulong flags) > { > - *get_gpio_flags(dev, offset) = flags; > + struct gpio_state *state = get_gpio_state(dev, offset); > + > + if (flags & GPIOD_IS_OUT_ACTIVE) > + flags |= GPIOD_EXT_HIGH; when you set GPIO out active, you force the EXT_HIGH for sandbox but if the the test request LOW output it is not managed ? I just ask if it is normal... here the EXT_HIGH bit could be cleared with: ? else flags &= ~GPIOD_EXT_HIGH; > + state->flags = (state->flags & GPIOD_SANDBOX_MASK) | flags; > > return 0; > } > @@ -188,23 +199,9 @@ static int sb_gpio_xlate(struct udevice *dev, struct gpio_desc *desc, > > static int sb_gpio_update_flags(struct udevice *dev, uint offset, ulong newf) > { > - ulong *flags; > - > debug("%s: offset:%u, flags = %lx\n", __func__, offset, newf); > > - flags = get_gpio_flags(dev, offset); > - > - /* > - * For testing purposes keep the output value when switching to input. > - * This allows us to manipulate the input value via the gpio command. > - */ > - if (newf & GPIOD_IS_IN) > - *flags = (newf & ~GPIOD_IS_OUT_ACTIVE) | > - (*flags & GPIOD_IS_OUT_ACTIVE); > - else > - *flags = newf; > - > - return 0; > + return sandbox_gpio_set_flags(dev, offset, newf); > } > > static int sb_gpio_get_flags(struct udevice *dev, uint offset, ulong *flagsp) Patrick