| Message ID | 20200515140100.3306517-4-hs@denx.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | gpio: add possibility to search for gpio label name | expand |
Hi Heiko, On Fri, 15 May 2020 at 08:02, Heiko Schocher <hs at 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 at 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 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 | 22 ++++++++++++++++++ > drivers/gpio/gpio-uclass.c | 47 ++++++++++++++++++++++++++++++++++++++ > test/dm/gpio.c | 7 ++++++ > 3 files changed, 76 insertions(+) Reviewed-by: Simon Glass <sjg at chromium.org> Some optional thoughts below > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 2081520f42..4a635b905c 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -46,6 +46,28 @@ 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 Perhaps the first term could be DM_GPIO_LOOKUP_LABEL? > + default n Not needed > + 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 d241263970..317b486982 100644 > --- a/drivers/gpio/gpio-uclass.c > +++ b/drivers/gpio/gpio-uclass.c > @@ -67,6 +67,46 @@ 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 I prefer to have the type on the same line as the function > +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; > @@ -95,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 1443a2db79..e2f147e776 100644 > --- a/test/dm/gpio.c > +++ b/test/dm/gpio.c > @@ -131,6 +131,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); Could add a negative test too (where you don't find it). > + > return 0; > } > DM_TEST(dm_test_gpio, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); > -- > 2.24.1 > Regards, SImon
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 2081520f42..4a635b905c 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -46,6 +46,28 @@ 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 + default n + 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 d241263970..317b486982 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -67,6 +67,46 @@ 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; @@ -95,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 1443a2db79..e2f147e776 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -131,6 +131,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);
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> --- 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 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 | 22 ++++++++++++++++++ drivers/gpio/gpio-uclass.c | 47 ++++++++++++++++++++++++++++++++++++++ test/dm/gpio.c | 7 ++++++ 3 files changed, 76 insertions(+)