[U-Boot] Driver model test breakages

[Joe Hershberger]

Hi Bin,

On Tue, Dec 22, 2015 at 12:43 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Joe,
>
> On Tue, Dec 22, 2015 at 12:50 PM, Bin Meng <bmeng.cn at gmail.com> wrote:
>> 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?
>>
>
> Please check the following two patches for dm_eth_rorate test.
>
> http://patchwork.ozlabs.org/patch/559882/
> http://patchwork.ozlabs.org/patch/559883/

These seem reasonable and also fix the test. I'm surprised the
behavior happened to work before without this explicit check.

Thanks,
-Joe