Hi Oliver,
On 7 December 2016 at 02:26, Olliver Schinagl oliver@schinagl.nl wrote:
On December 7, 2016 4:47:23 AM CET, Simon Glass sjg@chromium.org wrote:
Hi Oliver,
On 5 December 2016 at 03:28, Olliver Schinagl oliver@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 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
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).
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.
Regards, Simon