diff mbox series

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

Message ID 20200522090858.2569904-4-hs@denx.de
State Accepted
Commit 2bd261dd1712561b8363fbf6f9f630176fe99caa
Headers show
Series gpio: add possibility to search for gpio label name | expand

Commit Message

Heiko Schocher May 22, 2020, 9:08 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 at denx.de>
Reviewed-by: Simon Glass <sjg at chromium.org>
---

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 v6:
- add Reviewed-by tag from Simon
- add additional comments from Simon:
  - remove "default n" in Kconfig for SPL_DM_GPIO_LOOKUP_LABEL
  - type of function in same line as the function
  - add negativ test, where label is not found

Changes in v5: None
Changes in v4:
- rebased to current master ac14bc4169

Changes in v3:
- add comment from Simon Glass
  make this new function configurable through Kconfig
  option DM_GPIO_LOOKUP_LABEL

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/Kconfig       | 21 +++++++++++++++++
 drivers/gpio/gpio-uclass.c | 46 ++++++++++++++++++++++++++++++++++++++
 test/dm/gpio.c             |  9 ++++++++
 3 files changed, 76 insertions(+)

Comments

Michal Simek July 21, 2020, 1:18 p.m. UTC | #1
On 21. 07. 20 8:40, Heiko Schocher wrote:
> Hi Michal,

> 

> Am 20.07.2020 um 10:30 schrieb Michal Simek:

>> Hi Heiko,

>>

>> pá 22. 5. 2020 v 11:10 odesílatel Heiko Schocher <hs@denx.de> napsal:

>>>

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

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

>>> ---

>>>

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

>>> =>

>>

>> I spotted this patch and I am curious if you have thought about also

>> supporting gpio-line-names.

> 

> Yes, that would be a good extension!

> 

> searching for "gpio-line-names" in u-boot source shows only an entry in

> 

> ./doc/device-tree-bindings/gpio/gpio.txt

> 

> and some DTS files in arch/arm

> 

> but no driver code, which handles the property, so there is some

> work to do.

> 

> Currently I have no project (and time) to give this a chance...

> 

> But if the code, which handle "gpio-line-names" saves the name in

> "struct gpio_dev_priv name", DM_GPIO_LOOKUP_LABEL based code should

> work with it.


I have sent a patch for it. Please take a look.

Thanks,
Michal
diff mbox series

Patch

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d87f6cc105..829c68eda1 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -46,6 +46,27 @@  config GPIO_HOG
 	  is a mechanism providing automatic GPIO request and config-
 	  uration as part of the gpio-controller's driver probe function.
 
+config DM_GPIO_LOOKUP_LABEL
+	bool "Enable searching for gpio labelnames"
+	depends on DM_GPIO
+	default y
+	help
+	  This option enables searching for gpio names in
+	  the defined gpio labels, if the search for the
+	  gpio bank name failed. This makes sense if you use
+	  different gpios on different hardware versions
+	  for the same functionality in board code.
+
+config SPL_DM_GPIO_LOOKUP_LABEL
+	bool "Enable searching for gpio labelnames"
+	depends on DM_GPIO && SPL_DM && SPL_GPIO_SUPPORT
+	help
+	  This option enables searching for gpio names in
+	  the defined gpio labels, if the search for the
+	  gpio bank name failed. This makes sense if you use
+	  different gpios on different hardware versions
+	  for the same functionality in board code.
+
 config ALTERA_PIO
 	bool "Altera PIO driver"
 	depends on DM_GPIO
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index f016532354..ab17fa8a5d 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -68,6 +68,45 @@  static int gpio_to_device(unsigned int gpio, struct gpio_desc *desc)
 	return ret ? ret : -ENOENT;
 }
 
+#if CONFIG_IS_ENABLED(DM_GPIO_LOOKUP_LABEL)
+/**
+ * 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;
+}
+#else
+static int
+dm_gpio_lookup_label(const char *name, struct gpio_dev_priv *uc_priv,
+		     ulong *offset)
+{
+	return -ENOENT;
+}
+#endif
+
 int dm_gpio_lookup_name(const char *name, struct gpio_desc *desc)
 {
 	struct gpio_dev_priv *uc_priv = NULL;
@@ -96,6 +135,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 ecba566983..fcee1fe598 100644
--- a/test/dm/gpio.c
+++ b/test/dm/gpio.c
@@ -132,6 +132,15 @@  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);
+	ut_assert(gpio_lookup_name("hog_not_exist", &dev, &offset,
+				   &gpio));
+
 	return 0;
 }
 DM_TEST(dm_test_gpio, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);