Re: [PATCH 15/15] gpio: Add a way to read 3-way strapping pins

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@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