Hey Simon,
On 05-12-16 07:24, Simon Glass wrote:
Hi Oliver,
On 2 December 2016 at 03:16, Olliver Schinagl oliver@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 oliver@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 oliver@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).
Olliver
Regards, Simon