Hi Oliver, On 7 December 2016 at 02:26, Olliver Schinagl <oli...@schinagl.nl> wrote: > > > On December 7, 2016 4:47:23 AM CET, Simon Glass <s...@chromium.org> wrote: >>Hi Oliver, >> >>On 5 December 2016 at 03:28, Olliver Schinagl <oli...@schinagl.nl> >>wrote: >>> Hey Simon, >>> >>> >>> >>> On 05-12-16 07:24, Simon Glass wrote: >>>> >>>> Hi Oliver, >>>> >>>> On 2 December 2016 at 03:16, Olliver Schinagl <oli...@schinagl.nl> >>wrote: >>>>> >>>>> Hey Joe, >>>>> >>>>> >>>>> >>>>> On 30-11-16 21:40, Joe Hershberger wrote: >>>>>> >>>>>> On Fri, Nov 25, 2016 at 9:30 AM, Olliver Schinagl >><oli...@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 <oli...@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? >>>> >>>> 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? > The subclass driver can, the fdt fixup however still needs a weak > fdt_get_enetaddr()? (Which in our case calls then sunxi_get_enetaddr() i > think.)
OK - I feel that the fdt fixups need a bit of thought. At the moment it is all pretty ad-hoc. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot