Hey simon On December 8, 2016 11:21:32 PM CET, Simon Glass <s...@chromium.org> wrote: >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.
Yeah i think i agree, but i guess thats a separate cleanup step generally, no? > >Regards, >Simon -- Sent from my Android device with K-9 Mail. Please excuse my brevity. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot