
Hi Joe,
Am Mittwoch 16 Mai 2012, 02:56:39 schrieb Joe Hershberger:
Hi Michael,
On Fri, May 11, 2012 at 5:50 PM, Michael Walle michael@walle.cc wrote:
Signed-off-by: Michael Walle michael@walle.cc Cc: Joe Hershberger joe.hershberger@gmail.com
include/net.h | 16 ++++++++++++++++ net/eth.c | 15 +++++++++++++++ 2 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/include/net.h b/include/net.h index f6aeba2..bdc3da6 100644 --- a/include/net.h +++ b/include/net.h @@ -104,7 +104,9 @@ extern struct eth_device *eth_get_dev_by_index(int index); /* get dev @ index */ extern int eth_get_dev_index (void); /* get the device index */ extern void eth_parse_enetaddr(const char *addr, uchar *enetaddr); extern int eth_getenv_enetaddr(char *name, uchar *enetaddr); +#ifdef CONFIG_SETENV_ENETADDR_BY_INDEX extern int eth_setenv_enetaddr(char *name, const uchar *enetaddr); +#endif
Which other function did you intend? You already guarded the only definition whose implementation is guarded.
That was a mistake. Will be fixed in the next version.
/*
- Get the hardware address for an ethernet interface .
@@ -118,6 +120,20 @@ extern int eth_setenv_enetaddr(char *name, const uchar *enetaddr); extern int eth_getenv_enetaddr_by_index(const char *base_name, int index, uchar *enetaddr);
+#ifdef CONFIG_SETENV_ENETADDR_BY_INDEX
Personally I don't like config options for something so tiny. If you really must guard against including it, use the CONFIG_RANDOM_MACADDR instead.
Me, too. But Wolfgang doesn't accept patches which increases the code size for other boards.
I don't think it is a good idea to use CONFIG_RANDOM_MACADDR for such a generic function, which will (hopefully) be used by others too.
I'd prefer to just let the linker exclude it when it's never referenced.
Correct, me if i'm wrong, but the linker can only discard unused sections. But there was some discussion on -ffunction-sections and -fdata-sections on the ML. So maybe theres some light at the end of the tunnel ;)
+/*
- Set the hardware address for an ethernet interface .
- Args:
base_name - base name for device (normally "eth")
index - device index number (0 for first)
enetaddr - returns 6 byte hardware address
- Returns:
0 on success, else 1.
- */
+extern int eth_setenv_enetaddr_by_index(const char *base_name, int index, + const uchar *enetaddr); +#endif
#ifdef CONFIG_RANDOM_MACADDR /*
- The u-boot policy does not allow hardcoded ethernet addresses. Under
the diff --git a/net/eth.c b/net/eth.c index afce863..d66e22a 100644 --- a/net/eth.c +++ b/net/eth.c @@ -67,6 +67,21 @@ int eth_getenv_enetaddr_by_index(const char *base_name, int index, return eth_getenv_enetaddr(enetvar, enetaddr); }
+#ifdef CONFIG_SETENV_ENETADDR_BY_INDEX +int eth_setenv_enetaddr_by_index(const char *base_name, int index,
const uchar *enetaddr)
+{
char enetvar[32];
if (index)
sprintf(enetvar, "%s%daddr", base_name, index);
else
sprintf(enetvar, "%saddr", base_name);
return eth_setenv_enetaddr(enetvar, enetaddr);
+} +#endif
You should synchronize with Rob Herring who is also trying to add this eth_setenv_enetaddr_by_index() function. http://patchwork.ozlabs.org/patch/152590/
ah thanks for the hint.
Also since Rob's patch would use this function all the time, the CONFIG_SETENV_ENETADDR_BY_INDEX would be removed, right?
right, but see above.