* [PATCH v5 1/3] gpio-uclass.c: save the GPIOD flags also in the gpio descriptor
2020-05-15 14:00 [PATCH v5 0/3] gpio: add possibility to search for gpio label name Heiko Schocher
@ 2020-05-15 14:00 ` Heiko Schocher
2020-05-15 19:36 ` Patrick DELAUNAY
` (2 more replies)
2020-05-15 14:00 ` [PATCH v5 2/3] sandbox, test: add test for GPIO_HOG function Heiko Schocher
2020-05-15 14:00 ` [PATCH v5 3/3] gpio: search for gpio label if gpio is not found through bank name Heiko Schocher
2 siblings, 3 replies; 10+ messages in thread
From: Heiko Schocher @ 2020-05-15 14:00 UTC (permalink / raw)
To: u-boot
save the GPIOD_ flags also in the gpio descriptor.
Signed-off-by: Heiko Schocher <hs@denx.de>
---
Changes in v5:
- add comment from patrick, update the descriptor flags
in _dm_gpio_set_dir_flags() if setting direction was OK.
Changes in v4:
- new in version 4
Changes in v3: None
Changes in v2: None
drivers/gpio/gpio-uclass.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index d3cea11f76..d241263970 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -599,6 +599,10 @@ static int _dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
}
}
+ /* save the flags also in descriptor */
+ if (!ret)
+ desc->flags = flags;
+
return ret;
}
@@ -614,10 +618,6 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
flags |= desc->flags;
ret = _dm_gpio_set_dir_flags(desc, flags);
- /* update the descriptor flags */
- if (ret)
- desc->flags = flags;
-
return ret;
}
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v5 1/3] gpio-uclass.c: save the GPIOD flags also in the gpio descriptor
2020-05-15 14:00 ` [PATCH v5 1/3] gpio-uclass.c: save the GPIOD flags also in the gpio descriptor Heiko Schocher
@ 2020-05-15 19:36 ` Patrick DELAUNAY
2020-05-20 3:06 ` Simon Glass
2020-07-02 19:05 ` Baruch Siach
2 siblings, 0 replies; 10+ messages in thread
From: Patrick DELAUNAY @ 2020-05-15 19:36 UTC (permalink / raw)
To: u-boot
Hi Heiko
> From: Heiko Schocher <hs@denx.de>
> Sent: vendredi 15 mai 2020 16:01
>
> save the GPIOD_ flags also in the gpio descriptor.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
>
>
> ---
>
> Changes in v5:
> - add comment from patrick, update the descriptor flags
> in _dm_gpio_set_dir_flags() if setting direction was OK.
>
> Changes in v4:
> - new in version 4
>
> Changes in v3: None
> Changes in v2: None
>
> drivers/gpio/gpio-uclass.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com>
Fixes: 788ea834124b ("gpio: add function _dm_gpio_set_dir_flags")
Thanks
Patrick
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v5 1/3] gpio-uclass.c: save the GPIOD flags also in the gpio descriptor
2020-05-15 14:00 ` [PATCH v5 1/3] gpio-uclass.c: save the GPIOD flags also in the gpio descriptor Heiko Schocher
2020-05-15 19:36 ` Patrick DELAUNAY
@ 2020-05-20 3:06 ` Simon Glass
2020-07-02 19:05 ` Baruch Siach
2 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2020-05-20 3:06 UTC (permalink / raw)
To: u-boot
On Fri, 15 May 2020 at 08:01, Heiko Schocher <hs@denx.de> wrote:
>
> save the GPIOD_ flags also in the gpio descriptor.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
>
>
> ---
>
> Changes in v5:
> - add comment from patrick, update the descriptor flags
> in _dm_gpio_set_dir_flags() if setting direction was OK.
>
> Changes in v4:
> - new in version 4
>
> Changes in v3: None
> Changes in v2: None
>
> drivers/gpio/gpio-uclass.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
Would be good mention 'why' in patches like this.
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 1/3] gpio-uclass.c: save the GPIOD flags also in the gpio descriptor
2020-05-15 14:00 ` [PATCH v5 1/3] gpio-uclass.c: save the GPIOD flags also in the gpio descriptor Heiko Schocher
2020-05-15 19:36 ` Patrick DELAUNAY
2020-05-20 3:06 ` Simon Glass
@ 2020-07-02 19:05 ` Baruch Siach
2020-07-03 5:10 ` Heiko Schocher
2 siblings, 1 reply; 10+ messages in thread
From: Baruch Siach @ 2020-07-02 19:05 UTC (permalink / raw)
To: u-boot
Hi Heiko,
On Fri, May 15 2020, Heiko Schocher wrote:
> save the GPIOD_ flags also in the gpio descriptor.
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
This fixes SD card access on Hummingboard2. Current master uses the
wrong out polarity to control the SD card power regulator.
Tested-by: Baruch Siach <baruch@tkos.co.il>
Should go to v2020.07 I believe.
One more comment below.
> @@ -614,10 +618,6 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
> flags |= desc->flags;
> ret = _dm_gpio_set_dir_flags(desc, flags);
>
> - /* update the descriptor flags */
> - if (ret)
> - desc->flags = flags;
> -
> return ret;
You can just do
return _dm_gpio_set_dir_flags(desc, flags);
here instead.
Thanks,
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v5 1/3] gpio-uclass.c: save the GPIOD flags also in the gpio descriptor
2020-07-02 19:05 ` Baruch Siach
@ 2020-07-03 5:10 ` Heiko Schocher
0 siblings, 0 replies; 10+ messages in thread
From: Heiko Schocher @ 2020-07-03 5:10 UTC (permalink / raw)
To: u-boot
Hello Baruch,
Am 02.07.2020 um 21:05 schrieb Baruch Siach:
> Hi Heiko,
>
> On Fri, May 15 2020, Heiko Schocher wrote:
>> save the GPIOD_ flags also in the gpio descriptor.
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>> Reviewed-by: Patrick Delaunay <patrick.delaunay@st.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> This fixes SD card access on Hummingboard2. Current master uses the
> wrong out polarity to control the SD card power regulator.
>
> Tested-by: Baruch Siach <baruch@tkos.co.il>
>
> Should go to v2020.07 I believe.
Toms decision... but it seems this bug pops up on more and more
boards, as also some days ago Walter reported that this patch fixes
a problem on iMX6 Hummingboard with mmc...
There is also a v6 version of this patchset ... see:
http://patchwork.ozlabs.org/project/uboot/list/?series=178637
> One more comment below.
>
>> @@ -614,10 +618,6 @@ int dm_gpio_set_dir_flags(struct gpio_desc *desc, ulong flags)
>> flags |= desc->flags;
>> ret = _dm_gpio_set_dir_flags(desc, flags);
>>
>> - /* update the descriptor flags */
>> - if (ret)
>> - desc->flags = flags;
>> -
>> return ret;
>
> You can just do
>
> return _dm_gpio_set_dir_flags(desc, flags);
>
> here instead.
Yes!
I would let the patch as it is and (I or you?) send a fix on top?
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 2/3] sandbox, test: add test for GPIO_HOG function
2020-05-15 14:00 [PATCH v5 0/3] gpio: add possibility to search for gpio label name Heiko Schocher
2020-05-15 14:00 ` [PATCH v5 1/3] gpio-uclass.c: save the GPIOD flags also in the gpio descriptor Heiko Schocher
@ 2020-05-15 14:00 ` Heiko Schocher
2020-05-20 3:06 ` Simon Glass
2020-05-15 14:00 ` [PATCH v5 3/3] gpio: search for gpio label if gpio is not found through bank name Heiko Schocher
2 siblings, 1 reply; 10+ messages in thread
From: Heiko Schocher @ 2020-05-15 14:00 UTC (permalink / raw)
To: u-boot
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 v5: None
Changes in v4:
- rebased to current master ac14bc4169
Changes in v3: None
Changes in v2:
- add basic gpio hog test functions
arch/sandbox/dts/test.dts | 20 ++++++++++++++++++++
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, 47 insertions(+)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 5ce5e28476..24bb3ce36f 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -343,6 +343,26 @@
#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 c1237ea296..39dc3dc687 100644
--- a/configs/sandbox64_defconfig
+++ b/configs/sandbox64_defconfig
@@ -98,6 +98,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 9445d78118..9015458070 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -115,6 +115,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 a4a7ae8379..b5a85e1278 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -83,6 +83,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 945fe54d20..7341fa3221 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -102,6 +102,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 7c18e5c411..1443a2db79 100644
--- a/test/dm/gpio.c
+++ b/test/dm/gpio.c
@@ -19,6 +19,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];
@@ -108,6 +109,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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v5 2/3] sandbox, test: add test for GPIO_HOG function
2020-05-15 14:00 ` [PATCH v5 2/3] sandbox, test: add test for GPIO_HOG function Heiko Schocher
@ 2020-05-20 3:06 ` Simon Glass
0 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2020-05-20 3:06 UTC (permalink / raw)
To: u-boot
Hi Heiko,
On Fri, 15 May 2020 at 08: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 v5: None
> Changes in v4:
> - rebased to current master ac14bc4169
>
> Changes in v3: None
> Changes in v2:
> - add basic gpio hog test functions
>
> arch/sandbox/dts/test.dts | 20 ++++++++++++++++++++
> 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, 47 insertions(+)
>
Reviewed-by: Simon Glass <sjg@chromium.org>
It is OK to just enable on sandbox if you like, since we don't
normally run the bulk of the tests on the others.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 3/3] gpio: search for gpio label if gpio is not found through bank name
2020-05-15 14:00 [PATCH v5 0/3] gpio: add possibility to search for gpio label name Heiko Schocher
2020-05-15 14:00 ` [PATCH v5 1/3] gpio-uclass.c: save the GPIOD flags also in the gpio descriptor Heiko Schocher
2020-05-15 14:00 ` [PATCH v5 2/3] sandbox, test: add test for GPIO_HOG function Heiko Schocher
@ 2020-05-15 14:00 ` Heiko Schocher
2020-05-20 3:06 ` Simon Glass
2 siblings, 1 reply; 10+ messages in thread
From: Heiko Schocher @ 2020-05-15 14:00 UTC (permalink / raw)
To: u-boot
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 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(+)
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);
--
2.24.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v5 3/3] gpio: search for gpio label if gpio is not found through bank name
2020-05-15 14:00 ` [PATCH v5 3/3] gpio: search for gpio label if gpio is not found through bank name Heiko Schocher
@ 2020-05-20 3:06 ` Simon Glass
0 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2020-05-20 3:06 UTC (permalink / raw)
To: u-boot
Hi Heiko,
On Fri, 15 May 2020 at 08:02, 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 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@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
^ permalink raw reply [flat|nested] 10+ messages in thread