[U-Boot] dm eth rotation is broken under some circumstances

[Joe Hershberger]

Hi Bin,

On Thu, Sep 10, 2015 at 4:46 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Joe,
>
> I found under some circumstances the eth rotation functionality is broken.
>
> Test scenario:
>
> 1). Say we have two ethernet devices, eth0 and eth1. Both devices are
> not probed yet (or probe failed due to ethaddr and eth1addr not set)
>
> Now we set the eth1addr to a valid MAC address, set ethact to NULL.
> Test a "ping 10.10.0.100", it fails.
>
> The supposed eth rotation should switch to eth1 for 'ping', right? But
> debugging shows that eth1's probe routine never gets called (see log
> below).

It seems like we should be skipping a device that fails probe. I guess
there may be more smarts that can be added to the eth_init() function
that you were already looking at.

> => ping 10.10.0.100
>
> Error: eth_designware#0 address not set.
>
> Error: eth_designware#0 address not set.
>
> Error: eth_designware#0 address not set.
>
> Error: eth_designware#0 address not set.
> No ethernet found.
>
> Error: eth_designware#0 address not set.
> ping failed; host 10.10.0.100 is not alive
>
> 2). The same test scenario above, but set ethact to eth1, we should
> get "ping 10.10.0.100" successfully pinged. Unfortunately, it still
> fails, reporting 'No ethernet found".
>
> => ping 10.10.0.100
>
> Error: eth_designware#0 address not set.
>
> Error: eth_designware#0 address not set.
>
> Error: eth_designware#0 address not set.
>
> Error: eth_designware#1 address not set.
>
> Error: eth_designware#0 address not set.
>
> Error: eth_designware#0 address not set.
> No ethernet found.
>
> Error: eth_designware#0 address not set.
> ping failed; host 10.10.0.100 is not alive
>
> This time eth1's probe routine gets called, but it still failed (see
> log "Error: eth_designware#1 address not set.")
>
> This is because with eth0 probe failure before, when eth1 get probed,
> eth1's dev->seq will be set to 0 as it is the first ethernet device
> seen by dm eth, so its corresponding MAC address is ethaddr, NOT
> eth1addr. So its probe phase will still fail in eth_post_probe(), and
> rotation does not work as expected. I think this "dev->seq" match to
> "eth%daddr" is something we should think about with driver model
> conversion.

The second case is expected. It seems clear that your device tree does
not have aliases defined for your network adapters. If you ask DM to
assign a dynamic sequence number, then you get this behavior. Also, it
doesn't seem like a real use-case to be assigning only the 2nd MAC
address.

> So far the eth rotation seems worked pretty well if everything is good
> (i.e. probe OK), but on some exception path like above two it does not
> function as expected :(

There are test cases for the typical situations, so I would expect
that they work pretty well. :)

Maybe we need to add a test case for the first situation and extend
the capability. I think the second case need not be adjusted.

Cheers,
-Joe