Hi Oliver, On 5 December 2016 at 03:28, Olliver Schinagl <[email protected]> wrote: > Hey Simon, > > > > On 05-12-16 07:24, Simon Glass wrote: >> >> Hi Oliver, >> >> On 2 December 2016 at 03:16, Olliver Schinagl <[email protected]> wrote: >>> >>> Hey Joe, >>> >>> >>> >>> On 30-11-16 21:40, Joe Hershberger wrote: >>>> >>>> On Fri, Nov 25, 2016 at 9:30 AM, Olliver Schinagl <[email protected]> >>>> 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 <[email protected]> >>>>> --- >>>>> 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? >> >> Would it be possible to use a name without 'board' in it? I think this >> / hope is actually sunxi-specific code, not board-specific? > > You are actually correct, we take the serial number of the SoC (sunxi > specific) and generate a serial/MAC from it. So nothing to do with the > board. So I can just name it sunxi_gen_enetaddr(). Would that be then (much) > better? > > The reason why I went to 'board' with my mind, is because a) the original > mac gen code and b) the location was in board/sunxi/board.c. I think it is > thus also sensible to move it out of board/sunxi/board.c as indeed, it has > nothing to do with board(s).
That sounds good to me - and you should be able to call it directly from the driver and avoid any weak functions, right? Regards, Simon _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
