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

Fri May 15 08:01:06 CEST 2020

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