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 <mich...@walle.cc> wrote:
> > Signed-off-by: Michael Walle <mich...@walle.cc>
> > Cc: Joe Hershberger <joe.hershber...@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.
--
Michael
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
