[PATCH v2 0/2] gpio: add possibility to search for gpio label name

[PATCH v2 0/2] gpio: add possibility to search for gpio label name

[Heiko Schocher]

search for gpio label if gpio name from bankname is not found.

This makes sense on boards with different hardware verions. You
can now search for the gpio label name, and can give the gpio
a unique name. The real gpio pin number is not needed in board
code anymore.

while at it add basic gpio hog test functions in seperate patch.

Travis build:
https://travis-ci.org/hsdenx/u-boot-test/builds/644219338

Changes in v2:
- add basic gpio hog test functions
- add comment from Simon Glass
  move code into seperate function dm_gpio_lookup_label()
  add test if dm_gpio_lookup_label() works

Heiko Schocher (2):
  sandbox, test: add test for GPIO_HOG function
  gpio: search for gpio label if gpio is not found through bank name

 arch/sandbox/dts/test.dts          | 24 +++++++++++++++++++
 configs/sandbox64_defconfig        |  1 +
 configs/sandbox_defconfig          |  1 +
 configs/sandbox_flattree_defconfig |  1 +
 configs/sandbox_spl_defconfig      |  1 +
 drivers/gpio/gpio-uclass.c         | 38 ++++++++++++++++++++++++++++++
 test/dm/gpio.c                     | 30 +++++++++++++++++++++++
 7 files changed, 96 insertions(+)

-- 
2.24.1

[PATCH v2 1/2] sandbox, test: add test for GPIO_HOG function

[Heiko Schocher]

currently gpio hog function is not tested with "ut dm gpio"
so add some basic tests for gpio hog functionality.

For this enable GPIO_HOG in sandbox_defconfig, add
in DTS some gpio hog entries, and add testcase in
"ut dm gpio" command.

Signed-off-by: Heiko Schocher <hs@denx.de>

---

Changes in v2:
- add basic gpio hog test functions

 arch/sandbox/dts/test.dts          | 24 ++++++++++++++++++++++++
 configs/sandbox64_defconfig        |  1 +
 configs/sandbox_defconfig          |  1 +
 configs/sandbox_flattree_defconfig |  1 +
 configs/sandbox_spl_defconfig      |  1 +
 test/dm/gpio.c                     | 23 +++++++++++++++++++++++
 6 files changed, 51 insertions(+)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index e529c54d8d..cff05dcf7c 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -1,5 +1,7 @@
 /dts-v1/;
 
+#include <dt-bindings/gpio/gpio.h>
+
 / {
 	model = "sandbox";
 	compatible = "sandbox";
@@ -284,6 +286,28 @@
 		#gpio-cells = <1>;
 		gpio-bank-name = "a";
 		sandbox,gpio-count = <20>;
+
+		hog_input_active_low {
+			gpio-hog;
+			input;
+			gpios = <0 GPIO_ACTIVE_LOW>;
+		};
+		hog_input_active_high {
+			gpio-hog;
+			input;
+			gpios = <1 GPIO_ACTIVE_HIGH>;
+		};
+		hog_output_low {
+			gpio-hog;
+			output-low;
+			gpios = <2 GPIO_ACTIVE_HIGH>;
+		};
+		hog_output_high {
+			gpio-hog;
+			output-high;
+			gpios = <3 GPIO_ACTIVE_HIGH>;
+		};
+
 	};
 
 	gpio_b: extra-gpios {
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig
index 7b80033c3b..f614b23666 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -92,6 +92,7 @@ CONFIG_DM_DEMO_SIMPLE=y
 CONFIG_DM_DEMO_SHAPE=y
 CONFIG_BOARD=y
 CONFIG_BOARD_SANDBOX=y
+CONFIG_GPIO_HOG=y
 CONFIG_PM8916_GPIO=y
 CONFIG_SANDBOX_GPIO=y
 CONFIG_I2C_CROS_EC_TUNNEL=y
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index fc018bdd2c..afce8050b4 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -108,6 +108,7 @@ CONFIG_BOARD_SANDBOX=y
 CONFIG_DMA=y
 CONFIG_DMA_CHANNELS=y
 CONFIG_SANDBOX_DMA=y
+CONFIG_GPIO_HOG=y
 CONFIG_PM8916_GPIO=y
 CONFIG_SANDBOX_GPIO=y
 CONFIG_DM_HWSPINLOCK=y
diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig
index 2c90639ecb..0518b56fd9 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -78,6 +78,7 @@ CONFIG_DM_DEMO_SIMPLE=y
 CONFIG_DM_DEMO_SHAPE=y
 CONFIG_BOARD=y
 CONFIG_BOARD_SANDBOX=y
+CONFIG_GPIO_HOG=y
 CONFIG_PM8916_GPIO=y
 CONFIG_SANDBOX_GPIO=y
 CONFIG_I2C_CROS_EC_TUNNEL=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index b78115af61..e70f2b4aa8 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -98,6 +98,7 @@ CONFIG_DM_DEMO_SHAPE=y
 CONFIG_BOARD=y
 CONFIG_BOARD_SANDBOX=y
 CONFIG_SPL_FIRMWARE=y
+CONFIG_GPIO_HOG=y
 CONFIG_PM8916_GPIO=y
 CONFIG_SANDBOX_GPIO=y
 CONFIG_I2C_CROS_EC_TUNNEL=y
diff --git a/test/dm/gpio.c b/test/dm/gpio.c
index bb4b20cea9..9003ea82c7 100644
--- a/test/dm/gpio.c
+++ b/test/dm/gpio.c
@@ -18,6 +18,7 @@ static int dm_test_gpio(struct unit_test_state *uts)
 	unsigned int offset, gpio;
 	struct dm_gpio_ops *ops;
 	struct udevice *dev;
+	struct gpio_desc *desc;
 	const char *name;
 	int offset_count;
 	char buf[80];
@@ -102,6 +103,28 @@ static int dm_test_gpio(struct unit_test_state *uts)
 	ut_asserteq_str("a", name);
 	ut_asserteq(20, offset_count);
 
+	/* add gpio hog tests */
+	ut_assertok(gpio_hog_lookup_name("hog_input_active_low", &desc));
+	ut_asserteq(GPIOD_IS_IN | GPIOD_ACTIVE_LOW, desc->flags);
+	ut_asserteq(0, desc->offset);
+	ut_asserteq(1, dm_gpio_get_value(desc));
+	ut_assertok(gpio_hog_lookup_name("hog_input_active_high", &desc));
+	ut_asserteq(GPIOD_IS_IN, desc->flags);
+	ut_asserteq(1, desc->offset);
+	ut_asserteq(0, dm_gpio_get_value(desc));
+	ut_assertok(gpio_hog_lookup_name("hog_output_low", &desc));
+	ut_asserteq(GPIOD_IS_OUT, desc->flags);
+	ut_asserteq(2, desc->offset);
+	ut_asserteq(0, dm_gpio_get_value(desc));
+	ut_assertok(dm_gpio_set_value(desc, 1));
+	ut_asserteq(1, dm_gpio_get_value(desc));
+	ut_assertok(gpio_hog_lookup_name("hog_output_high", &desc));
+	ut_asserteq(GPIOD_IS_OUT, desc->flags);
+	ut_asserteq(3, desc->offset);
+	ut_asserteq(1, dm_gpio_get_value(desc));
+	ut_assertok(dm_gpio_set_value(desc, 0));
+	ut_asserteq(0, dm_gpio_get_value(desc));
+
 	return 0;
 }
 DM_TEST(dm_test_gpio, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
-- 
2.24.1

[PATCH v2 2/2] gpio: search for gpio label if gpio is not found through bank name

[Heiko Schocher]

dm_gpio_lookup_name() searches for a gpio through
the bank name. But we have also gpio labels, and it
makes sense to search for a gpio also in the labels
we have defined, if no gpio is found through the
bank name definition.

This is useful for example if you have a wp pin on
different gpios on different board versions.

If dm_gpio_lookup_name() searches also for the gpio labels,
you can give the gpio an unique label name and search
for this label, and do not need to differ between
board revisions.

Signed-off-by: Heiko Schocher <hs@denx.de>
---

Example on the aristainetos board:

=> gpio clear wp_spi_nor.gpio-hog
gpio: pin wp_spi_nor.gpio-hog (gpio 47) value is 0
=>

before this patch, you need to know where your
pin is:

=> gpio clear GPIO2_15
gpio: pin GPIO2_15 (gpio 47) value is 0
=>

travis build:

Changes in v2:
- add comment from Simon Glass
  move code into seperate function dm_gpio_lookup_label()
  add test if dm_gpio_lookup_label() works

 drivers/gpio/gpio-uclass.c | 38 ++++++++++++++++++++++++++++++++++++++
 test/dm/gpio.c             |  7 +++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 90fbed455b..fb87ef9810 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -52,6 +52,37 @@ static int gpio_to_device(unsigned int gpio, struct gpio_desc *desc)
 	return ret ? ret : -ENOENT;
 }
 
+/**
+ * dm_gpio_lookup_label() - look for name in gpio device
+ *
+ * search in uc_priv, if there is a gpio with labelname same
+ * as name.
+ *
+ * @name:	name which is searched
+ * @uc_priv:	gpio_dev_priv pointer.
+ * @offset:	gpio offset within the device
+ * @return:	0 if found, -ENOENT if not.
+ */
+static int
+dm_gpio_lookup_label(const char *name, struct gpio_dev_priv *uc_priv,
+		     ulong *offset)
+{
+	int len;
+	int i;
+
+	*offset = -1;
+	len = strlen(name);
+	for (i = 0; i < uc_priv->gpio_count; i++) {
+		if (!uc_priv->name[i])
+			continue;
+		if (!strncmp(name, uc_priv->name[i], len)) {
+			*offset = i;
+			return 0;
+		}
+	}
+	return -ENOENT;
+}
+
 int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc)
 {
 	struct gpio_dev_priv *uc_priv = NULL;
@@ -80,6 +111,13 @@ int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc)
 			if (!strict_strtoul(name + len, 10, &offset))
 				break;
 		}
+
+		/*
+		 * if we did not found a gpio through its bank
+		 * name, we search for a valid gpio label.
+		 */
+		if (!dm_gpio_lookup_label(name, uc_priv, &offset))
+			break;
 	}
 
 	if (!dev)
diff --git a/test/dm/gpio.c b/test/dm/gpio.c
index 9003ea82c7..1f1bf680c5 100644
--- a/test/dm/gpio.c
+++ b/test/dm/gpio.c
@@ -125,6 +125,13 @@ static int dm_test_gpio(struct unit_test_state *uts)
 	ut_assertok(dm_gpio_set_value(desc, 0));
 	ut_asserteq(0, dm_gpio_get_value(desc));
 
+	/* Check if lookup for labels work */
+	ut_assertok(gpio_lookup_name("hog_input_active_low", &dev, &offset,
+				     &gpio));
+	ut_asserteq_str(dev->name, "base-gpios");
+	ut_asserteq(0, offset);
+	ut_asserteq(CONFIG_SANDBOX_GPIO_COUNT + 0, gpio);
+
 	return 0;
 }
 DM_TEST(dm_test_gpio, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
-- 
2.24.1

[PATCH v2 1/2] sandbox, test: add test for GPIO_HOG function

[Simon Glass]

On Sat, 1 Feb 2020 at 01:02, Heiko Schocher <hs@denx.de> wrote:
>
> currently gpio hog function is not tested with "ut dm gpio"
> so add some basic tests for gpio hog functionality.
>
> For this enable GPIO_HOG in sandbox_defconfig, add
> in DTS some gpio hog entries, and add testcase in
> "ut dm gpio" command.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
>
> ---
>
> Changes in v2:
> - add basic gpio hog test functions
>
>  arch/sandbox/dts/test.dts          | 24 ++++++++++++++++++++++++
>  configs/sandbox64_defconfig        |  1 +
>  configs/sandbox_defconfig          |  1 +
>  configs/sandbox_flattree_defconfig |  1 +
>  configs/sandbox_spl_defconfig      |  1 +
>  test/dm/gpio.c                     | 23 +++++++++++++++++++++++
>  6 files changed, 51 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[PATCH v2 2/2] gpio: search for gpio label if gpio is not found through bank name

[Simon Glass]

On Sat, 1 Feb 2020 at 01:03, Heiko Schocher <hs@denx.de> wrote:
>
> dm_gpio_lookup_name() searches for a gpio through
> the bank name. But we have also gpio labels, and it
> makes sense to search for a gpio also in the labels
> we have defined, if no gpio is found through the
> bank name definition.
>
> This is useful for example if you have a wp pin on
> different gpios on different board versions.
>
> If dm_gpio_lookup_name() searches also for the gpio labels,
> you can give the gpio an unique label name and search
> for this label, and do not need to differ between
> board revisions.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
>
> Example on the aristainetos board:
>
> => gpio clear wp_spi_nor.gpio-hog
> gpio: pin wp_spi_nor.gpio-hog (gpio 47) value is 0
> =>
>
> before this patch, you need to know where your
> pin is:
>
> => gpio clear GPIO2_15
> gpio: pin GPIO2_15 (gpio 47) value is 0
> =>
>
> travis build:
>
> Changes in v2:
> - add comment from Simon Glass
>   move code into seperate function dm_gpio_lookup_label()
>   add test if dm_gpio_lookup_label() works
>
>  drivers/gpio/gpio-uclass.c | 38 ++++++++++++++++++++++++++++++++++++++
>  test/dm/gpio.c             |  7 +++++++
>  2 files changed, 45 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

I wonder if this should be a Kconfig so we can disable it by default in SPL?


- Simon

[PATCH v2 2/2] gpio: search for gpio label if gpio is not found through bank name

[Heiko Schocher]

Hello Simon,

Am 03.02.2020 um 01:04 schrieb Simon Glass:
> On Sat, 1 Feb 2020 at 01:03, Heiko Schocher <hs@denx.de> wrote:
>>
>> dm_gpio_lookup_name() searches for a gpio through
>> the bank name. But we have also gpio labels, and it
>> makes sense to search for a gpio also in the labels
>> we have defined, if no gpio is found through the
>> bank name definition.
>>
>> This is useful for example if you have a wp pin on
>> different gpios on different board versions.
>>
>> If dm_gpio_lookup_name() searches also for the gpio labels,
>> you can give the gpio an unique label name and search
>> for this label, and do not need to differ between
>> board revisions.
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>> ---
>>
>> Example on the aristainetos board:
>>
>> => gpio clear wp_spi_nor.gpio-hog
>> gpio: pin wp_spi_nor.gpio-hog (gpio 47) value is 0
>> =>
>>
>> before this patch, you need to know where your
>> pin is:
>>
>> => gpio clear GPIO2_15
>> gpio: pin GPIO2_15 (gpio 47) value is 0
>> =>
>>
>> travis build:
>>
>> Changes in v2:
>> - add comment from Simon Glass
>>    move code into seperate function dm_gpio_lookup_label()
>>    add test if dm_gpio_lookup_label() works
>>
>>   drivers/gpio/gpio-uclass.c | 38 ++++++++++++++++++++++++++++++++++++++
>>   test/dm/gpio.c             |  7 +++++++
>>   2 files changed, 45 insertions(+)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> I wonder if this should be a Kconfig so we can disable it by default in SPL?

Hmm.. maybe a good idea for boards which have code size restrictions.
On the other hand, on such boards DM/DTS is most likely no option?

But it should be easy to add this into a Kconfig option, proposal

DM_GPIO_LOOKUP_LABEL ?

default: n for SPL and U-Boot ?

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de

[PATCH v2 2/2] gpio: search for gpio label if gpio is not found through bank name

[Simon Glass]

Hi Heiko,

On Sun, 2 Feb 2020 at 22:26, Heiko Schocher <hs@denx.de> wrote:
>
> Hello Simon,
>
> Am 03.02.2020 um 01:04 schrieb Simon Glass:
> > On Sat, 1 Feb 2020 at 01:03, Heiko Schocher <hs@denx.de> wrote:
> >>
> >> dm_gpio_lookup_name() searches for a gpio through
> >> the bank name. But we have also gpio labels, and it
> >> makes sense to search for a gpio also in the labels
> >> we have defined, if no gpio is found through the
> >> bank name definition.
> >>
> >> This is useful for example if you have a wp pin on
> >> different gpios on different board versions.
> >>
> >> If dm_gpio_lookup_name() searches also for the gpio labels,
> >> you can give the gpio an unique label name and search
> >> for this label, and do not need to differ between
> >> board revisions.
> >>
> >> Signed-off-by: Heiko Schocher <hs@denx.de>
> >> ---
> >>
> >> Example on the aristainetos board:
> >>
> >> => gpio clear wp_spi_nor.gpio-hog
> >> gpio: pin wp_spi_nor.gpio-hog (gpio 47) value is 0
> >> =>
> >>
> >> before this patch, you need to know where your
> >> pin is:
> >>
> >> => gpio clear GPIO2_15
> >> gpio: pin GPIO2_15 (gpio 47) value is 0
> >> =>
> >>
> >> travis build:
> >>
> >> Changes in v2:
> >> - add comment from Simon Glass
> >>    move code into seperate function dm_gpio_lookup_label()
> >>    add test if dm_gpio_lookup_label() works
> >>
> >>   drivers/gpio/gpio-uclass.c | 38 ++++++++++++++++++++++++++++++++++++++
> >>   test/dm/gpio.c             |  7 +++++++
> >>   2 files changed, 45 insertions(+)
> >
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> >
> > I wonder if this should be a Kconfig so we can disable it by default in SPL?
>
> Hmm.. maybe a good idea for boards which have code size restrictions.
> On the other hand, on such boards DM/DTS is most likely no option?

The overhead of DM in SPL is pretty small, particularly if you use of-platdata.

>
> But it should be easy to add this into a Kconfig option, proposal
>
> DM_GPIO_LOOKUP_LABEL ?
>
> default: n for SPL and U-Boot ?

I'd suggest y for U-Boot and n for SPL

Regards,
Simon