[PATCH v3 1/2] sandbox, test: add test for GPIO_HOG function
Patrick DELAUNAY
patrick.delaunay at st.com
Thu May 14 15:47:47 CEST 2020
Hi Heiko
> From: Heiko Schocher <hs at denx.de>
> Sent: mardi 12 mai 2020 09:32
>
> Hello tom, Patrick,
>
> Am 12.05.2020 um 08:26 schrieb Heiko Schocher:
> > Hello Tom, Patrick,
> >
> > Am 27.04.2020 um 08:47 schrieb Heiko Schocher:
> >> Hello Tom, Patrick,
> >>
> >> Am 27.04.2020 um 07:16 schrieb Heiko Schocher:
> >>> Hello Tom,
> >>>
> >>> Am 24.04.2020 um 19:45 schrieb Tom Rini:
> >>>> On Wed, Feb 05, 2020 at 07:19:58AM +0100, Heiko Schocher 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 at denx.de>
> >>>>> Reviewed-by: Simon Glass <sjg at chromium.org>
> >>>>
> >>>> This no longer applies cleanly/obviously, please rebase, thanks!
> >>>
> >>> Done, unfortunately, aristainetos2 does not boot anymore... got:
> >>>
> >>> ├─UBOOT (ari-ub)
> >>> │ │ <> ### Connect to "aristainetos" using command:
> >>> /usr/bin/telnet ts2 7015 │ │ <> Trying 192.168.1.202...
> >>> │ │ <> Connected to ts2.
> >>> │ │ <> Escape character is '^]'.
> >>> │ │ <> <debug_uart> unrecognized JEDEC id bytes: 00, 00, 00 │
> >>> │ <> *** Warning - spi_flash_probe_bus_cs() failed, using default
> >>> environment │ │ <> │ │ <> alloc space exhausted │ │
> >>> <> alloc space exhausted │ │ <> alloc space exhausted │ │
> >>> <> himport_r: can't insert "loadbootscriptUSB=ext4load usb 0 ${loadaddr}
> ${script};"
> >>> into hash table
> >>> │ │ <> alloc space exhausted
> >>> │ │ <> alloc space exhausted
> >>>
> >>> Seems early SPI NOR detection fails ...
> >>>
> >>> Have to start bisect, try to find some time...
> >>
> >> Ok, commit:
> >>
> >> commit 788ea834124bd6169ea10b2d37d5de48a2dd28a0 (bisect-788ea83412)
> >> Author: Patrick Delaunay <patrick.delaunay at st.com>
> >> Date: Mon Jan 13 11:35:03 2020 +0100
> >>
> >> gpio: add function _dm_gpio_set_dir_flags
> >>
> >> Introduce the function _dm_gpio_set_dir_flags to set dir flags
> >> without check if the GPIO is reserved.
> >>
> >> Separate the reserved check for "set_dir" and "set_dir_flags".
> >>
> >> This patch is a preliminary step to add new ops.
> >>
> >> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
> >> Reviewed-by: Simon Glass <sjg at chromium.org>
> >>
> >> breaks the aristainetos2 board ... reverting this patch (and
> >> therefore I needed some more patches to revert as it was a patchseries):
> >>
> >> * dbf06f0e6c - (HEAD -> aristainetos-denx) Revert "gpio: add function
> >> _gpio_get_value" (vor 5
> >> Minuten) <Heiko Schocher>
> >> * 5c85a7cc26 - Revert "gpio: add function _dm_gpio_set_dir_flags"
> >> (vor 5 Minuten) <Heiko Schocher>
> >> * c226d65d88 - Revert "gpio: add function check_dir_flags" (vor 5
> >> Minuten) <Heiko Schocher>
> >> * 1423a40c69 - Revert "gpio: add helper GPIOD_FLAGS_OUTPUT" (vor 5
> >> Minuten) <Heiko Schocher>
> >> * fb0176450f - Revert "gpio: update dir_flags management" (vor 5
> >> Minuten) <Heiko Schocher>
> >> * 9d74cc5ecb - Revert "gpio: add support of new GPIO direction flag"
> >> (vor 5 Minuten) <Heiko Schocher>
> >> * 3bf361c206 - Revert "gpio: add ops to get dir flags" (vor 5
> >> Minuten) <Heiko Schocher>
> >> * beb6d3c2d9 - Revert "gpio: add ops to set dir flags" (vor 5
> >> Minuten) <Heiko Schocher>
> >>
> >> And board boots again fine ...
> >>
> >> I do not see, why commit 788ea834124bd6169ea10b2d37d5de48a2dd28a0
> >> makes SPI not working anymore ...
> >>
> >> Any ideas?
> >
> > Just looking with sandbox into it ... added debug patch:
> >
> > diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> > index 3fb6b6e69c..2e68f558bd 100644
> > --- a/drivers/gpio/gpio-uclass.c
> > +++ b/drivers/gpio/gpio-uclass.c
> > @@ -273,9 +273,11 @@ static int gpio_hog_probe(struct udevice *dev)
> > struct gpio_hog_priv *priv = dev_get_priv(dev);
> > int ret;
> >
> > + printf("%s: --------- dev->name: %s nr: %d flag: %d platflags:
> > +%d\n", __func__, dev->name,
> > plat->val[0], plat->val[1], plat->gpiod_flags);
> > ret = gpio_dev_request_index(dev->parent, dev->name,
> > "gpio-hog",
> > plat->val[0], plat->gpiod_flags,
> > plat->val[1], &priv->gpiod);
> > + printf("%s: --------- gpiod: %p\n", __func__, &priv->gpiod);
> > if (ret < 0) {
> > debug("%s: node %s could not get gpio.\n", __func__,
> > dev->name);
> > @@ -291,6 +293,10 @@ static int gpio_hog_probe(struct udevice *dev)
> > }
> > }
> >
> > + if (1) {
> > + struct gpio_desc *gpiod = &priv->gpiod;
> > + printf("%s: gpiod_flags: %x\n", __func__,
> > +gpiod->flags);
> > + }
> > return 0;
> > }
> >
> >
> > and see:
> >
> > => ut dm gpio
> > Test: dm_test_gpio: gpio.c
> > gpio_hog_probe_all: ---------
> > gpio_hog_probe: --------- dev->name: hog_input_active_low nr: 0 flag:
> > 1 platflags: 4
> > gpio_hog_probe: --------- gpiod: 0000000015901050
> > gpio_hog_probe: gpiod_flags: 8
> > gpio_hog_probe: --------- dev->name: hog_input_active_high nr: 1 flag:
> > 0 platflags: 4
> > gpio_hog_probe: --------- gpiod: 00000000159010a0
> > gpio_hog_probe: gpiod_flags: 0
> > gpio_hog_probe: --------- dev->name: hog_output_low nr: 2 flag: 0
> > platflags: 2
> > gpio_hog_probe: --------- gpiod: 00000000159010f0
> > gpio_hog_probe: gpiod_flags: 0
> > gpio_hog_probe: --------- dev->name: hog_output_high nr: 3 flag: 0
> > platflags: 2
> > gpio_hog_probe: --------- gpiod: 0000000015901130
> > gpio_hog_probe: gpiod_flags: 0
> > gpio_hog_lookup_name: name: hog_input_active_low
> > gpio_hog_lookup_name: --------- gpiod: 15901050 test/dm/gpio.c:115,
> > dm_test_gpio(): GPIOD_IS_IN | GPIOD_ACTIVE_LOW == desc->flags:
> > Expected 0xc (12), got 0x8 (8)
> >
> > It looks like the plat->gpiod_flags are not written anymore into
> > gpiod through the gpio_dev_request_index() function ...
>
> Ok, I did now the following change:
>
> https://github.com/hsdenx/u-boot-test/commits/aristainetos-gpio-v4
>
> $ git show c4a3295fa6
> commit c4a3295fa67f23408ba3ff5552532222f48142f6
> Author: Heiko Schocher <hs at denx.de>
> Date: Tue May 12 08:56:40 2020 +0200
>
> gpio-uclass.c: save the GPIOD flags also in the gpio descriptor
>
> save the GPIOD_ flags also in the gpio descriptor.
>
> Signed-off-by: Heiko Schocher <hs at denx.de>
>
> Patch-cc: Patrick Delaunay <patrick.delaunay at st.com>
>
> Series-changes: 4
> - new in version 4
>
> diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index
> 757ab7106e..fc94334160 100644
> --- a/drivers/gpio/gpio-uclass.c
> +++ b/drivers/gpio/gpio-uclass.c
> @@ -572,6 +572,9 @@ static int _dm_gpio_set_dir_flags(struct gpio_desc
> *desc, ulong flags)
> return ret;
> }
>
> + /* save the flags also in descriptor */
> + desc->flags = flags;
> +
> /* GPIOD_ are directly managed by driver in set_dir_flags*/
> if (ops->set_dir_flags) {
> ret = ops->set_dir_flags(dev, desc->offset, flags);
>
> and with it, the sandbox tests and the aristainetos2 board works fine again. I have
> just started a travis build for it:
>
> https://travis-ci.org/github/hsdenx/u-boot-test/builds/685995112
>
> If no errors pop up, I send v4 version of my patchseries.
In my serie, normally the update of the desc->flags was done in dm_gpio_set_dir_flags()
with the line
/* update the descriptor flags */
if (ret)
desc->flags = flags;
not needed in dm_gpio_set_dir (as requested with desc->flags)
At first lok, I don't understood the execution path to have desc->flags not updated...
But in fact I inverse the test (again :-<)
/* update the descriptor flags */
+ if (!ret)
- if (ret)
desc->flags = flags;
but this test could move in _ dm_gpio_set_deit to be more clear,
but I think we need continue to test ret value before to update descriptor
Sorry for the issue.
Do you prefer that send a sperate patch to correct this error ?
Regards
Patrick
More information about the U-Boot
mailing list