On December 7, 2016 4:47:23 AM CET, Simon Glass <[email protected]> wrote: >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? 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.) > >Regards, >Simon
-- Sent from my Android device with K-9 Mail. Please excuse my brevity. _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
