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).

Olliver

Regards,
Simon

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to