Hi Oliver, On Sun, Dec 11, 2016 at 3:27 PM, Simon Glass <s...@chromium.org> wrote: > Hi Oliver, > > On 9 December 2016 at 02:25, Olliver Schinagl <oli...@schinagl.nl> wrote: >> 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? > > OK - are you able to take a look at this?
Any update on this or any of the other patches in your series that I did not apply last release? I was expecting a v2 to address some things such as this patch. Thanks! -Joe _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot