| Message ID | 2ea0889ef7947c7feda0c9327ceed087c0521fed.1595337294.git.michal.simek@xilinx.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | gpio: Add support for gpio-line-names reading | expand |
Hello Michal, Am 21.07.2020 um 15:14 schrieb Michal Simek: > The commit 2bd261dd1712 ("gpio: search for gpio label if gpio is not found > through bank name") introduced the option to search gpio via labels (gpio > hog). This patch is just follow up on this and fills pin name based on > gpio-line-names DT property. > > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > > arch/sandbox/dts/test.dts | 2 ++ > drivers/gpio/gpio-uclass.c | 31 +++++++++++++++++++++++++++++++ > test/dm/gpio.c | 6 ++++++ > 3 files changed, 39 insertions(+) Looks good to me, thanks! Reviewed-by: Heiko Schocher <hs@denx.de> bye, Heiko
On Tue, 21 Jul 2020 at 07:15, Michal Simek <michal.simek@xilinx.com> wrote: > > The commit 2bd261dd1712 ("gpio: search for gpio label if gpio is not found > through bank name") introduced the option to search gpio via labels (gpio > hog). This patch is just follow up on this and fills pin name based on > gpio-line-names DT property. > > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > > arch/sandbox/dts/test.dts | 2 ++ > drivers/gpio/gpio-uclass.c | 31 +++++++++++++++++++++++++++++++ > test/dm/gpio.c | 6 ++++++ > 3 files changed, 39 insertions(+) Reviewed-by: Simon Glass <sjg@chromium.org>
Hi, On 28. 07. 20 20:58, Simon Glass wrote: > On Tue, 21 Jul 2020 at 07:15, Michal Simek <michal.simek@xilinx.com> wrote: >> >> The commit 2bd261dd1712 ("gpio: search for gpio label if gpio is not found >> through bank name") introduced the option to search gpio via labels (gpio >> hog). This patch is just follow up on this and fills pin name based on >> gpio-line-names DT property. >> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >> --- >> >> arch/sandbox/dts/test.dts | 2 ++ >> drivers/gpio/gpio-uclass.c | 31 +++++++++++++++++++++++++++++++ >> test/dm/gpio.c | 6 ++++++ >> 3 files changed, 39 insertions(+) > > Reviewed-by: Simon Glass <sjg@chromium.org> > I have run CI loop with this patch added and I see one issue regarding this. Issue is visible when dm_test_gpio_get_dir_flags() is called. Specifically ut_asserteq(6, gpio_request_list_by_name(dev, "test3-gpios", desc_list, ARRAY_SIZE(desc_list), 0)); This function requests gpios and when request passes it assigns name to gpio_dev_priv->name[] array. This is done in dm_gpio_request() Before assignment there is this code which check if name is already assigned or not. uc_priv = dev_get_uclass_priv(dev); if (uc_priv->name[desc->offset]) return -EBUSY; That means that we are using name as indicator if gpio is used or not. This logic is also applied in get_function() where you can see if (skip_unused && !uc_priv->name[offset]) return GPIOF_UNUSED; Where assigned name is just indicator of if device is used or not. This doesn't sound right to me. And that's just open a question how Heiko's patch should work. Because you look for a name and you get that pin but you can't request is because this request IMHO has to failed because gpio core thinks that it is already in used and you shouldn't touch that pin. The whole code which calls dm_gpio_lookup_name for bank name should be fine because it is different entry in struct gpio_dev_priv. Definitely this patch needs to be dropped and changed because it can't work like this and the question is Heiko's patch should still stay in tree or should be reverted. Maybe we should create another entry in char **label; in struct gpio_dev_priv where these labels should be saved and look for instead of just name which is used for assignment. Thanks, Michal
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 3744a4660300..1b33cd4c0878 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -388,6 +388,8 @@ #gpio-cells = <2>; gpio-bank-name = "c"; sandbox,gpio-count = <10>; + gpio-line-names = "ZERO", "ONE", "TWO", "THREE", "FOUR", + "FIVE", "SIX", "SEVEL", "EIGHT", ""; }; }; diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 9c53299b6a3b..430c6849f4cd 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -1177,6 +1177,37 @@ static int gpio_post_probe(struct udevice *dev) if (!uc_priv->name) return -ENOMEM; + if (IS_ENABLED(CONFIG_DM_GPIO_LOOKUP_LABEL) && + dev_read_string(dev, "gpio-line-names")) { + int i, ret, count; + const char *name; + char *str; + + count = dev_read_string_count(dev, "gpio-line-names"); + + if (count != uc_priv->gpio_count) { + printf("Incorrect gpio-line-names count %d/%d\n", + count, uc_priv->gpio_count); + return -EINVAL; + } + + for (i = 0; i < uc_priv->gpio_count; i++) { + ret = dev_read_string_index(dev, "gpio-line-names", i, + &name); + if (ret) + return ret; + + if (strlen(name)) { + str = strdup(name); + if (!str) + return -ENOMEM; + + /* All the time there is pointer to name in DT */ + uc_priv->name[i] = (char *)str; + } + } + } + return gpio_renumber(NULL); } diff --git a/test/dm/gpio.c b/test/dm/gpio.c index 29701389fcd5..623a157ff1db 100644 --- a/test/dm/gpio.c +++ b/test/dm/gpio.c @@ -142,6 +142,12 @@ static int dm_test_gpio(struct unit_test_state *uts) ut_assert(gpio_lookup_name("hog_not_exist", &dev, &offset, &gpio)); + /* Check if lookup for names filled via gpio-line-names work */ + ut_assertok(gpio_lookup_name("EIGHT", &dev, &offset, &gpio)); + ut_asserteq_str(dev->name, "pinmux-gpios"); + ut_asserteq(8, offset); + ut_asserteq(CONFIG_SANDBOX_GPIO_COUNT + 20 + 10 + 8, gpio); + return 0; } DM_TEST(dm_test_gpio, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
The commit 2bd261dd1712 ("gpio: search for gpio label if gpio is not found through bank name") introduced the option to search gpio via labels (gpio hog). This patch is just follow up on this and fills pin name based on gpio-line-names DT property. Signed-off-by: Michal Simek <michal.simek@xilinx.com> --- arch/sandbox/dts/test.dts | 2 ++ drivers/gpio/gpio-uclass.c | 31 +++++++++++++++++++++++++++++++ test/dm/gpio.c | 6 ++++++ 3 files changed, 39 insertions(+)