Hey Joe,

On 26-03-17 16:10, Joe Hershberger wrote:
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.

I have a few after rebasing u-boot-net/master and I just started yesterday to get back up to speed. I'll double check all review comments and send a v2 with the remaining patches. Sorry for taking so long here!

Olliver


Thanks!
-Joe


--
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to