[U-Boot] [PATCH 12/14] fdt: eth_fixup: Add hook for board to override MAC

[Olliver Schinagl]

Resend  with maxime's correct e-mail address and Jagain added to CC

On 10-04-17 17:46, Olliver Schinagl wrote:
> Hey Simon,
>
> On 01-12-16 03:20, Simon Glass wrote:
>> Hi,
>>
>> On 25 November 2016 at 08:30, Olliver Schinagl <oliver at schinagl.nl>
>> wrote:
>>> This patch adds a method for the board to set the MAC address if the
>>> environment is not yet set. The environment based MAC addresses are not
>>> touched, but if the fdt has an alias set, it is parsed and put into the
>>> environment.
>>>
>>> E.g. The environment contains ethaddr and eth1addr, and the fdt contains
>>> an ethernet1 nothing happens. If the fdt contains ethernet2 however, it
>>> is parsed and inserted into the environment as eth2addr.
>>>
>>> Signed-off-by: Olliver Schinagl <oliver at schinagl.nl>
>>> ---
>>>  common/fdt_support.c | 8 +++++++-
>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/common/fdt_support.c b/common/fdt_support.c
>>> index c34a13c..f127392 100644
>>> --- a/common/fdt_support.c
>>> +++ b/common/fdt_support.c
>>> @@ -465,6 +465,11 @@ int fdt_fixup_memory(void *blob, u64 start, u64
>>> size)
>>>         return fdt_fixup_memory_banks(blob, &start, &size, 1);
>>>  }
>>>
>>> +__weak int board_get_enetaddr(const int i, unsigned char *mac_addr)
>>> +{
>>> +       return -ENOSYS;
>>> +}
>>> +
>>>  void fdt_fixup_ethernet(void *fdt)
>>>  {
>>>         int i, prop;
>>> @@ -507,7 +512,8 @@ void fdt_fixup_ethernet(void *fdt)
>>>                         if (fdt_eth_addr)
>>>                                 eth_parse_enetaddr(fdt_eth_addr,
>>> mac_addr);
>>>                         else
>>> -                               continue;
>>> +                               if (board_get_enetaddr(i, mac_addr) < 0)
>>> +                                       continue;
>>>
>>>                         do_fixup_by_path(fdt, path, "mac-address",
>>>                                          &mac_addr, 6, 0);
>>> --
>>> 2.10.2
>>>
>>
>> Much as I don't want to pin this on any one patch, but I feel that our
>> DT fixup stuff is a bit out of control. We have so many functions that
>> do this, and they are called from various places. At some point we
>> need to think about the infrastructure.
>>
>> IMO we should move to a linker list approach a bit like SPL did
>> recently (SPL_LOAD_IMAGE_METHOD). Then boards and subsystems can
>> register (at build-time) a fixup routine and they all get called one
>> after the other.
>>
>> We could also (later) add run-time support for registering fixups,
>> that drivers could use.
>
> We kinda left this out in the cold last time (or I did?) and I just sent
> my v5 of this patch-series (without this one).
>
> The gist from what I remember here was, to not introduce a weak function
> here, and have boards go back-forth to other subsystems. I belive one
> suggestion was to use a more specific name, lets say sunxi_get_hwaddr()
> for example and use that. Quite obviously this being the
> common/fdt_support, it would have to be generic.
>
> So I'm still at loss as to how to handle this here.
>
> Some background here, what the idea was I belive, that certain network
> adapters rely on the fdt to supply the MAC address to the driver, and so
> u-boot injects the mac address into the fdt.
>
> We currently hack this together I belive, by generating environment
> variables for non-existant devices (if the fdt has ethernet aliases) and
> when doing this bit of code, inject the generated environment variables
> into the fdt.
>
> I can't rewrite/fixup the fdt stuff, but am also not sure what an
> acceptable solution for this would be. Granted we really are hacking
> this into u-boot but I suppose we cannot escape this for legacy reasons.
> Devices expect the same generated mac address as before.
>
> Again, bar rewriting this stuff, what can we do as the current code in
> board/sunxi/board.c is just as ugly a wart.
>
> Olliver
>
>>
>> Regards,
>> Simon
>>