[PATCH 15/15] gpio: Add a way to read 3-way strapping pins
Patrick DELAUNAY
patrick.delaunay at foss.st.com
Thu Jan 21 11:52:37 CET 2021
Hi Simon,
On 1/15/21 3:05 PM, Simon Glass wrote:
> Using the internal vs. external pull resistors it is possible to get
> 27 different combinations from 3 strapping pins. Add an implementation
> of this.
>
> This involves updating the sandbox GPIO driver to model external and
> (weaker) internal pull resistors. The get_value() method now takes account
> of what is driving a pin:
>
> sandbox: GPIOD_EXT_DRIVEN - in which case GPIO_EXT_HIGH provides the
> value
> outside source - in which case GPIO_EXT_PULL_UP/DOWN indicates the
> external state and we work the final state using those flags and
> the internal GPIOD_PULL_UP/DOWN flags
>
> Of course the outside source does not really exist in sandbox. We are just
> modelling it for test purpose.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> arch/sandbox/include/asm/gpio.h | 5 +-
> drivers/gpio/gpio-uclass.c | 78 ++++++++++++++++++++++++++
> drivers/gpio/sandbox.c | 13 +++--
> include/asm-generic/gpio.h | 37 +++++++++++++
> test/dm/gpio.c | 98 +++++++++++++++++++++++++++++++++
> 5 files changed, 226 insertions(+), 5 deletions(-)
>
> diff --git a/arch/sandbox/include/asm/gpio.h b/arch/sandbox/include/asm/gpio.h
> index edf78cb4131..097abfb299c 100644
> --- a/arch/sandbox/include/asm/gpio.h
> +++ b/arch/sandbox/include/asm/gpio.h
> @@ -26,8 +26,11 @@
> /* 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_EXT_DRIVEN BIT(21) /* external source is driven */
> +#define GPIOD_EXT_PULL_UP BIT(22) /* GPIO has external pull-up */
> +#define GPIOD_EXT_PULL_DOWN BIT(23) /* GPIO has external pull-down */
>
> -#define GPIOD_SANDBOX_MASK GENMASK(21, 20)
> +#define GPIOD_EXT_PULL (BIT(21) | BIT(22))
> +#define GPIOD_SANDBOX_MASK GENMASK(23, 20)
>
> /**
> * Return the simulated value of a GPIO (used only in sandbox test code)
> diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> index 77b40263bbd..984c07d1dfa 100644
> --- a/drivers/gpio/gpio-uclass.c
> +++ b/drivers/gpio/gpio-uclass.c
> @@ -3,6 +3,8 @@
> * Copyright (c) 2013 Google, Inc
> */
>
> +#define LOG_CATEGORY UCLASS_GPIO
> +
define for LOG feature could be a separate patch ?
to support CONFIG_LOG with existing debug() macro
> #include <common.h>
> #include <dm.h>
> #include <log.h>
> @@ -20,6 +22,7 @@
> #include <dm/device_compat.h>
> #include <linux/bug.h>
> #include <linux/ctype.h>
> +#include <linux/delay.h>
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -708,6 +711,21 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
> return dm_gpio_clrset_flags(desc, 0, flags);
> }
>
> +int dm_gpios_clrset_flags(struct gpio_desc *desc, int count, ulong clr,
> + ulong set)
> +{
> + int ret;
> + int i;
> +
> + for (i = 0; i < count; i++) {
> + ret = dm_gpio_clrset_flags(&desc[i], clr, set);
> + if (ret)
> + return log_ret(ret);
> + }
> +
> + return 0;
> +}
> +
> int dm_gpio_get_flags(struct gpio_desc *desc, ulong *flagsp)
> {
> struct udevice *dev = desc->dev;
> @@ -974,6 +992,66 @@ int dm_gpio_get_values_as_int(const struct gpio_desc *desc_list, int count)
> return vector;
> }
>
> +int dm_gpio_get_values_as_int_base3(struct gpio_desc *desc_list,
> + int count)
> +{
> + static const char tristate[] = "01z";
> + enum {
> + PULLUP,
> + PULLDOWN,
> +
> + NUM_OPTIONS,
> + };
> + int vals[NUM_OPTIONS];
> + uint mask;
> + uint vector = 0;
> + int ret, i;
> +
return error if overflow for the the request count ?
when size_of(int) / 3 < count.
> + for (i = 0; i < NUM_OPTIONS; i++) {
> + uint flags = GPIOD_IS_IN;
> +
> + flags |= (i == PULLDOWN) ? GPIOD_PULL_DOWN : GPIOD_PULL_UP;
> + ret = dm_gpios_clrset_flags(desc_list, count, GPIOD_MASK_PULL,
> + flags);
> + if (ret)
> + return log_msg_ret("pu", ret);
> +
> + /* Give the lines time to settle */
> + udelay(10);
> +
> + ret = dm_gpio_get_values_as_int(desc_list, count);
> + if (ret < 0)
> + return log_msg_ret("get1", ret);
> + vals[i] = ret;
> + }
> +
> + log_debug("values: %x %x, count = %d\n", vals[0], vals[1], count);
> + for (i = count - 1, mask = 1 << i; i >= 0; i--, mask >>= 1) {
> + uint pd = vals[PULLDOWN] & mask ? 1 : 0;
> + uint pu = vals[PULLUP] & mask ? 1 : 0;
> + uint digit;
> +
> + /*
> + * Get value with internal pulldown active. If this is 1 then
> + * there is a stronger external pullup, which we call 1. If not
> + * then call it 0.
> + */
> + digit = pd;
> +
> + /*
> + * If the values differ then the pin is floating so we call
> + * this a 2.
> + */
> + if (pu != pd)
> + digit |= 2;
/*
* If the values differ then the pin is floating so we call
* this a 2.
* else Get value with internal pulldown active. If this is 1 then
* there is a stronger external pullup, which we call 1. If not
* then call it 0.
*/
if (pu != pd)
digit = 2;
else
digit = pd;
I known that it is the same result (as in this case digit = pd = 0 and
pu = 1)
but perhaps more clear that "digit |= 2;" with OR operator
> + log_debug("%c ", tristate[digit]);
> + vector = 3 * vector + digit;
> + }
> + log_debug("vector=%d\n", vector);
> +
> + return vector;
> +}
> +
> /**
> * gpio_request_tail: common work for requesting a gpio.
> *
> diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
> index b32f7ac529b..56b339f5e3c 100644
> --- a/drivers/gpio/sandbox.c
> +++ b/drivers/gpio/sandbox.c
> @@ -19,7 +19,6 @@
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/gpio/sandbox-gpio.h>
>
> -
> struct gpio_state {
> const char *label; /* label given by requester */
> ulong flags; /* flags (GPIOD_...) */
> @@ -81,10 +80,16 @@ int sandbox_gpio_get_value(struct udevice *dev, unsigned offset)
> if (get_gpio_flag(dev, offset, GPIOD_IS_OUT))
> debug("sandbox_gpio: get_value on output gpio %u\n", offset);
>
> - if (state->flags & GPIOD_EXT_DRIVEN)
> + if (state->flags & GPIOD_EXT_DRIVEN) {
> val = state->flags & GPIOD_EXT_HIGH;
> - else
> - val = false;
> + } else {
> + if (state->flags & GPIOD_EXT_PULL_UP)
> + val = true;
> + else if (state->flags & GPIOD_EXT_PULL_DOWN)
> + val = false;
> + else
> + val = state->flags & GPIOD_PULL_UP;
> + }
>
> return val;
> }
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index 4628d937259..1cf81a3fc7a 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -505,6 +505,28 @@ int gpio_get_values_as_int(const int *gpio_list);
> */
> int dm_gpio_get_values_as_int(const struct gpio_desc *desc_list, int count);
>
> +/**
> + * dm_gpio_get_values_as_int_base3() - Create a base-3 int from a list of GPIOs
> + *
> + * This uses pull-ups/pull-downs to figure out whether a GPIO line is externally
> + * pulled down, pulled up or floating. This allows three different strap values
> + * for each pin.
Add base 3 associated value description in function description
* 0 : external pull down
* 1 : externall pull up
* 2 : floating
> + *
> + * With this it is possible to obtain more combinations from the same number of
> + * strapping pins, when compared to dm_gpio_get_values_as_int(). The external
> + * pull resistors should be made stronger that the internal SoC pull resistors,
> + * for this to work.
> + *
> + * With 2 pins, 6 combinations are possible, compare with 4
> + * With 3 pins, 27 are possible, compared with 8
> + *
> + * @desc_list: List of GPIOs to collect
> + * @count: Number of GPIOs
> + * @return resulting integer value, or -ve on error
> + */
> +int dm_gpio_get_values_as_int_base3(struct gpio_desc *desc_list,
> + int count);
> +
> /**
> * gpio_claim_vector() - claim a number of GPIOs for input
> *
(...)
For information, I am testing the feature on STM32 platform.
I hope have a result before the end of the week.
Regards
Patrick
More information about the U-Boot
mailing list