diff mbox series

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

Message ID 20200201080215.596439-3-hs@denx.de
State Superseded
Headers show
Series gpio: add possibility to search for gpio label name | expand

Commit Message

Heiko Schocher Feb. 1, 2020, 8:02 a.m. UTC
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(+)

Comments

Simon Glass Feb. 3, 2020, 12:04 a.m. UTC | #1
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
Heiko Schocher Feb. 3, 2020, 5:25 a.m. UTC | #2
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
Simon Glass Feb. 3, 2020, 5:15 p.m. UTC | #3
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
diff mbox series

Patch

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);