[U-Boot] Driver model test breakages

[Bin Meng]

On Tue, Dec 22, 2015 at 12:04 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Joe,
>
> On Tue, Dec 22, 2015 at 11:08 AM, Joe Hershberger
> <joe.hershberger at gmail.com> wrote:
>> Hi Bin,
>>
>> On Mon, Dec 21, 2015 at 9:00 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> Hi Joe,
>>>
>>> On Tue, Dec 22, 2015 at 10:39 AM, Joe Hershberger
>>> <joe.hershberger at gmail.com> wrote:
>>>> On Mon, Dec 21, 2015 at 8:36 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>>> Hi Joe,
>>>>>
>>>>> On Tue, Dec 22, 2015 at 10:32 AM, Joe Hershberger
>>>>> <joe.hershberger at gmail.com> wrote:
>>>>>> Hi Bin,
>>>>>>
>>>>>> On Mon, Dec 21, 2015 at 8:12 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>>>>> Hi Joe, Simon,
>>>>>>>
>>>>>>> On Tue, Dec 22, 2015 at 6:46 AM, Joe Hershberger
>>>>>>> <joe.hershberger at gmail.com> wrote:
>>>>>>>> Hi Simon and Bin
>>>>>>>>
>>>>>>>> On Thu, Dec 10, 2015 at 8:05 PM, Simon Glass <sjg at chromium.org> wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> The following three commits causes breakages in the driver model tests:
>>>>>>>>>
>>>>>>>>> 4efad20a      sf: Update status reg check in spi_flash_cmd_wait_ready
>>>>>>>>> 45b47734     net/arp: account for ARP delay, avoid duplicate packets on timeout
>>>>>>>>> 9961a0b6    sandbox: add a sandbox timer and basic test
>>>>>>>>>
>>>>>>>>> Can you please take a look? You can run them with ./test/dm/test-dm.sh
>>>>>>>>
>>>>>>>> It appears that ac1d313 (net: eth: Check return value in various
>>>>>>>> places) breaks the eth_rotate test.
>>>>>>>>
>>>>>>>> Looking into it. Bin, do you have any ideas?
>>>>>>>
>>>>>>> I will look into this.
>>>>>>>
>>>>>>> BTW: I applied the following two patches [1][2] to the tree based on
>>>>>>> dm/master, and got a segmentation fault:
>>>>>>>
>>>>>>> Test: dm_test_usb_keyb
>>>>>>> ./test/dm/test-dm.sh: line 14: 24902 Segmentation fault
>>>>>>> ./sandbox/u-boot -d ./sandbox/arch/sandbox/dts/test.dtb -c "ut dm"
>>>>>>>
>>>>>>> [1] http://patchwork.ozlabs.org/patch/555597/
>>>>>>> [2] http://patchwork.ozlabs.org/patch/559783/
>>>>>>
>>>>>> Interesting. I haven't tested on top of dm/master, but I tested it
>>>>>> based on origin/master and the issue is resolved.
>>>>>>
>>>>>
>>>>> Which issue is resolved? I just tested on top of origin/master with
>>>>> the above two patches, still the same segmentation fault.
>>>>
>>>> The net_retry dm test hang that Simon reported.
>>>>
>>>
>>> The segmentation fault happens after "Test: dm_test_usb_keyb". It
>>> seems to be a new issue.
>>
>> I agree it's a new issue that was masked by the net_retry hang.
>>
>>> I am looking into _dm_test_eth_rotate1 failure, but I don't understand
>>> the comments here.
>>>
>>> /* If ethrotate is no, then we should fail on a bad MAC */
>>> setenv("ethact", "eth at 10004000");
>>> setenv("ethrotate", "no");
>>> ut_asserteq(-EINVAL, net_loop(PING));
>>> ut_asserteq_str("eth at 10004000", getenv("ethact"));
>>>
>>> Does 'bad MAC' mean no MAC address? For eth at 10004000, it does have a
>>> MAC address defined in the env variable in sandbox.h.
>>
>> I think if you look at dm_test_eth_rotate(), the eth1addr is deleted
>> before the test. Later, ethaddr is deleted before the second part of
>> the test.
>>
>>> I also tested manually with the same test sequence from U-Boot shell,
>>> it can pass. I am still looking into it.
>>
>> It's not as though it hangs or something, but it now fails to return
>> the error code that it used to before your patch.
>>
>> Test: dm_test_eth_rotate
>> ../test/dm/eth.c:160, _dm_test_eth_rotate1(): -EINVAL ==
>> net_loop(PING): Expected -22, got 0
>>
>
> During debug I've noticed another strange issue:
>
> Sometimes the sandbox's setenv() does not successfully set the
> environment variable to the expected value. This occurs from time to
> time, even with the same u-boot image. Do you have any idea about
> this?
>

I may have been dazed that I looked the wrong message line. The issue
is complicated. My commit just exposed an existing issue that have
been in the dm eth codes from the beginning, that the eth_set_dev()
logic is not controlled by the env variable "ethrotate". If the
"ethact" is set to an eth device which cannot be probed (eg: no valid
ethaddr) and "ethrotate" is set to "no" (like in this test case), the
"ethact" will still point to the first probable eth device, in this
sandbox case, eth at 10002000.

I am still figuring out where to fix. Joe, you might want to have a look?

Regards,
Bin