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

[Olliver Schinagl]

Hey Joe,


On 30-11-16 21:40, Joe Hershberger wrote:
> On Fri, Nov 25, 2016 at 9:30 AM, 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)
> Ugh. This collides with a function in board/v38b/ethaddr.c and in
> board/intercontrol/digsy_mtc/digsy_mtc.c
>
> Also, it's so generic, and only gets called by the fdt fixup stuff...
> This function should probably be named in such a way that its
> association with fdt is clear.
I did not notice that, sorry! But naming suggestions are welcome :)

Right now, I use it in two unrelated spots however.

from the fdt as seen above and in a subclass driver (which will come up 
for review) as suggested by Simon.

There I do:

+static int sunxi_gmac_eth_read_rom_hwaddr(struct udevice *dev)
+{
+       struct eth_pdata *pdata = dev_get_platdata(dev);
+
+       return board_get_enetaddr(dev->seq, pdata->enetaddr);
+}
+
  const struct eth_ops sunxi_gmac_eth_ops = {
         .start                  = designware_eth_start,
         .send                   = designware_eth_send,
@@ -102,6 +110,7 @@ const struct eth_ops sunxi_gmac_eth_ops = {
         .free_pkt               = designware_eth_free_pkt,
         .stop                   = designware_eth_stop,
         .write_hwaddr           = designware_eth_write_hwaddr,
+       .read_rom_hwaddr        = sunxi_gmac_eth_read_rom_hwaddr,
  };

which is completly unrelated to the fdt.

So naming suggestion or overal suggestion how to handle this nice and 
generically?

Based from the name however I would think that all board_get_enetaddr's 
work the same however so should have been interchangeable? Or was that 
silly thinking?

Olliver

>
>> +{
>> +       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
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot