diff mbox series

[v3,1/2] sandbox, test: add test for GPIO_HOG function

Message ID 211b4f57-a2d9-7bc7-66e6-9aa0cd577fa5@denx.de
State New
Headers show
Series [v3,1/2] sandbox, test: add test for GPIO_HOG function | expand

Commit Message

Heiko Schocher May 12, 2020, 7:32 a.m. UTC
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


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.

bye,
Heiko

Comments

Patrick Delaunay May 14, 2020, 1:47 p.m. UTC | #1
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
Heiko Schocher May 15, 2020, 6:01 a.m. UTC | #2
Hello Patrick,

Am 14.05.2020 um 15:47 schrieb Patrick DELAUNAY:
> 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;

Ok, this fix works also, thanks!

> 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

Moved this into _dm_gpio_set_dir_flags()
> Sorry for the issue.

No problem, more problematic was, that I missed to add sandbox
tests before and patch which adds them is pending for a while!
> Do you prefer that send a sperate patch to correct this error ?

I would see this in this patchseries, travis build runs, and I post
v5 if all is fine, thanks!

https://travis-ci.org/github/hsdenx/u-boot-test/builds/687305014

bye,
Heiko
Patrick Delaunay May 15, 2020, 7:13 a.m. UTC | #3
Hi Heiko,

> From: Heiko Schocher <hs at denx.de>
> Sent: vendredi 15 mai 2020 08:01
> 
> Hello Patrick,
> 
> Am 14.05.2020 um 15:47 schrieb Patrick DELAUNAY:
> > Hi Heiko
> >
> >> From: Heiko Schocher <hs at denx.de>
> >> Sent: mardi 12 mai 2020 09:32
> >>
> >> Hello tom, Patrick,
> >>
[...]
> >>>
> >>> 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;
> 
> Ok, this fix works also, thanks!
> 
> > 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
> 
> Moved this into _dm_gpio_set_dir_flags()
> > Sorry for the issue.
> 
> No problem, more problematic was, that I missed to add sandbox tests before and
> patch which adds them is pending for a while!
> > Do you prefer that send a sperate patch to correct this error ?
> 
> I would see this in this patchseries, travis build runs, and I post
> v5 if all is fine, thanks!
> 
> https://travis-ci.org/github/hsdenx/u-boot-test/builds/687305014

Ok, your patch is good for me.

Thanks 
Patrick
diff mbox series

Patch

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