[U-Boot] [PATCH] fdt: Rewrite the logic in fdt_fixup_ethernet()

[Bin Meng]

Hi Simon,

On Thu, Oct 29, 2015 at 9:53 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi,
>
> On 28 October 2015 at 19:40, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Hi Joe,
>>
>> On Thu, Oct 29, 2015 at 4:52 AM, Joe Hershberger
>> <joe.hershberger at gmail.com> wrote:
>>> Hi Bin,
>>>
>>> On Tue, Oct 27, 2015 at 11:10 AM, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>> Currently in fdt_fixup_ethernet() the MAC address fix up is
>>>> handled in a loop of which the exit condition is to test the
>>>> "eth%daddr" env is not NULL. However this creates unnecessary
>>>> constrains that those "eth%daddr" env variables must be
>>>> sequential even if "ethernet%d" does not start from 0 in the
>>>> "/aliases" node. For example, with "/aliases" node below:
>>>>
>>>>     aliases {
>>>>         ethernet3 = &enet3;
>>>>         ethernet4 = &enet4;
>>>>     };
>>>>
>>>> "ethaddr", "eth1addr", "eth2addr" must exist in order to fix
>>>> up ethernet3's MAC address successfully.
>>>>
>>>> Now we change the loop logic to iterate the properties in the
>>>> "/aliases" node. For each property, test if it is in a format
>>>> of "ethernet%d", then get its MAC address from corresponding
>>>> "eth%daddr" env and fix it up in the dtb.
>>>>
>>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>>
>>> Acked-by: Joe Hershberger <joe.hershberger at ni.com>
>>
>> Thanks! Do you have any comments regarding to "usbethaddr" env that I
>> raised here [1]? I originally wanted to remove "usbethaddr" handling
>> completely in fdt_fixup_ethernet(), but did not do that when I
>> submitted this patch.
>>
>> [1] http://lists.denx.de/pipermail/u-boot/2015-October/231791.html
>
> My understanding is that we were going to drop the usbethaddr
> variables and just use ethaddr.
>

Yep, I agree, and driver model eth device only uses "ethaddr" as the
MAC address source. I guess when we introduced "usbethaddr" to U-Boot
at the beginning we didn't think about MAC address fix up. Later
commit b1f49ab was added to only handle "usbethaddr" assuming that
there is only one usb ethernet on the board, which is not necessarily
the case. Using two sources ("usbethaddr" and "ethaddr") just causes
trouble in the logic in fdt_fixup_ethernet().

Regards,
Bin