Skip to contentSkip to conversation

net/eth.c: fix eth_write_hwaddr() to use dev->enetaddr as fall back

PATCH New Zach Sadecki 3 participants 3 messages Jan 16, 2012 – Jan 17, 2012

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 net/eth.c: fix eth_write_hwaddr() to use dev->enetaddr as fall back

This patch will also fix a problem with mx28 boards that are depending on the OCOTP bits to set the MAC address (like the Denx m28 board). Now it simply fails with a "Warning: failed to set MAC address" if there's no environment variable instead of using the OCOTP bits like it should. This patch will fix that.

#1 lore raw

PATCH 1/1 net/eth.c: fix eth_write_hwaddr() to use dev->enetaddr as fall back

Dear Zach Sadecki,

Quoted 5 lines · In message <[email protected]> you wrote:

This patch will also fix a problem with mx28 boards that are depending on the OCOTP bits to set the MAC address (like the Denx m28 board). Now it simply fails with a "Warning: failed to set MAC address" if there's no environment variable instead of using the OCOTP bits like it should. This patch will fix that.

But there _should_ always be a warning message if the environment variable is missing.

Best regards,

Wolfgang Denk

Signature · 5 lines
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 at denx.de
Heavier than air flying machines are impossible.
                    -- Lord Kelvin, President, Royal Society, c. 1895
#2 lore raw

PATCH 1/1 net/eth.c: fix eth_write_hwaddr() to use dev->enetaddr as fall back

Dear Wolfgang and all,

Quoted 11 lines · Wolfgang Denk

Dear Zach Sadecki,

Quoted 5 lines · In message <[email protected]> you wrote:

This patch will also fix a problem with mx28 boards that are depending on the OCOTP bits to set the MAC address (like the Denx m28 board). Now it simply fails with a "Warning: failed to set MAC address" if there's no environment variable instead of using the OCOTP bits like it should. This patch will fix that.

But there _should_ always be a warning message if the environment variable is missing.

Let's see if I can summarize the issue:

If I understood correctly, we are talking about

http://git.denx.de/?p=u-boot.git;a=commitdiff;h=7616e7850804c7c69e0a22c179dfcba9e8f3f587

and there

int eth_initialize(bd_t *bis)
{
...
- eth_getenv_enetaddr_by_index(eth_number, env_enetaddr);

versus

+int eth_write_hwaddr(...)
+{
...
+ if (!eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr))
+ return -1;

I.e. the unconditional call of eth_getenv_enetaddr_by_index() was replaced by a return -1 if the environment variable is missing (resulting in "Warning: failed to set MAC address"? Btw: That's wrong? It should be "Warning: failed to get MAC address from the environment"?)

Now, Eric's patch [1] (which needs an update of the commit message, sure, but was acked by the author of the original patch [2]) tries to re-introduce the original behavior:

+ eth_getenv_enetaddr_by_index(base_name, eth_number, env_enetaddr);

So, what I understand for i.MXxx, Zach and Eric are talking about:

If the environment variable is missing, the system can use the OCOTP bits to set the MAC address. If the return -1 is removed by using Eric's patch.

And for overo:

From Philip's mail, on an overo based product that uses the SMSC911x
ethernet chip, if the environment variable is missing, the MAC address is set via EEPROM connected to the SMSC911x chip.

I.e. on these systems, with Eric's patch, we get a correct MAC address, even if the environment variable is missing. This sounds like a good option (?).

The question is then: Do we want a "Warning: failed to set MAC address" message, even if the MAC is set correctly? But not from the environment variable, but somehow read from the HW?

I know you will correct what I missed in this summary ;)

Best regards

Dirk

[1] http://lists.denx.de/pipermail/u-boot/2011-August/098686.html

[2] http://lists.denx.de/pipermail/u-boot/2011-August/098690.html