| Message ID | 1330780211-90427-1-git-send-email-gwenhael.goavec-merou@armadeus.com |
|---|---|
| State | Rejected |
| Delegated to: | Joe Hershberger |
| Headers | show |
Dear Gwenhael Goavec-Merou, In message <1330780211-90427-1-git-send-email-gwenhael.goavec-merou@armadeus.com> you wrote: > The return value for eth_getenv_enetaddr_by_index produces > a Warning when ethXaddr is not set (env_enetaddr is equal > to 00:00:00:00:00:00). > > The validity of the mac address is tested later in this > function to avoid to write an erroneous address. > > The test of the function's return is consequently useless > and adds a warning. Which use case do you have in mind? Why do you think this warning is bad? I tend to consider it helpful instead. Best regards, Wolfgang Denk
On Sat, 03 Mar 2012 15:44:48 +0100 Wolfgang Denk <wd@denx.de> wrote: > Dear Gwenhael Goavec-Merou, > > In message <1330780211-90427-1-git-send-email-gwenhael.goavec-merou@armadeus.com> you wrote: > > The return value for eth_getenv_enetaddr_by_index produces > > a Warning when ethXaddr is not set (env_enetaddr is equal > > to 00:00:00:00:00:00). > > > > The validity of the mac address is tested later in this > > function to avoid to write an erroneous address. > > > > The test of the function's return is consequently useless > > and adds a warning. > > Which use case do you have in mind? Why do you think this warning is > bad? I tend to consider it helpful instead. > For boards with mac address stored on an eeprom for example, the variable ethaddr is not needed. But with this test, a warning is always shown, without any reason. Best regards, Gwenhael Goavec-Merou > > > Best regards, > > Wolfgang Denk >
Dear "gwenhael.goavec", In message <20120303155719.3ae222cc@trabucayre.com> you wrote: > > > Which use case do you have in mind? Why do you think this warning is > > bad? I tend to consider it helpful instead. > > For boards with mac address stored on an eeprom for example, the variable > ethaddr is not needed. But with this test, a warning is always shown, without > any reason. The environment variable is always mandatory. If it does not exist, or if it's value is different from the one stored somewhere else, a warning shall be issued (and the environment value takes precedence). So I think current behaviour is actually correct. Best regards, Wolfgang Denk
Hi, On Sat, Mar 3, 2012 at 7:11 AM, Wolfgang Denk <wd@denx.de> wrote: > Dear "gwenhael.goavec", > > In message <20120303155719.3ae222cc@trabucayre.com> you wrote: >> >> > Which use case do you have in mind? Why do you think this warning is >> > bad? I tend to consider it helpful instead. >> >> For boards with mac address stored on an eeprom for example, the variable >> ethaddr is not needed. But with this test, a warning is always shown, without >> any reason. > > The environment variable is always mandatory. If it does not exist, > or if it's value is different from the one stored somewhere else, a > warning shall be issued (and the environment value takes precedence). > > So I think current behaviour is actually correct. > The code is not ideal - I suspect with a bit of improvement we can do the right thing in both cases. See the discussion started here: http://lists.denx.de/pipermail/u-boot/2012-February/117783.html I think this might be the third patch to do the same thing, so perhaps one of these people might like to take on the clean-up? Regards, Simon > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de > In the beginning, there was nothing, which exploded. > - Terry Pratchett, _Lords and Ladies_
diff --git a/net/eth.c b/net/eth.c index b4b9b43..1e61aed 100644 --- a/net/eth.c +++ b/net/eth.c @@ -175,8 +175,7 @@ int eth_write_hwaddr(struct eth_device *dev, const char *base_name, unsigned char env_enetaddr[6]; int ret = 0; - if (!eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr)) - return -1; + (void)eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr); if (memcmp(env_enetaddr, "\0\0\0\0\0\0", 6)) { if (memcmp(dev->enetaddr, "\0\0\0\0\0\0", 6) &&
The return value for eth_getenv_enetaddr_by_index produces a Warning when ethXaddr is not set (env_enetaddr is equal to 00:00:00:00:00:00). The validity of the mac address is tested later in this function to avoid to write an erroneous address. The test of the function's return is consequently useless and adds a warning. Signed-off-by: Gwenhael Goavec-Merou <gwenhael.goavec-merou@armadeus.com> --- net/eth.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)