[U-Boot] [PATCH 1/3] smc911x: write back the manually set MAC address
If the MAX address is given by the environment, write it back to the hardware.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Sascha Hauer s.hauer@pengutronix.de --- drivers/net/smc911x.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index 30f2dc2..8c9a2a8 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -41,8 +41,13 @@ static int smx911x_handle_mac_address(bd_t *bd) unsigned long addrh, addrl; uchar m[6];
- /* if the environment has a valid mac address then use it */ - if (!eth_getenv_enetaddr("ethaddr", m)) { + if (eth_getenv_enetaddr("ethaddr", m)) { + /* if the environment has a valid mac address then use it */ + addrl = m[0] | (m[1] << 8) | (m[2] << 16) | (m[3] << 24); + addrh = m[4] | (m[5] << 8); + smc911x_set_mac_csr(ADDRL, addrl); + smc911x_set_mac_csr(ADDRH, addrh); + } else { /* if not, try to get one from the eeprom */ addrh = smc911x_get_mac_csr(ADDRH); addrl = smc911x_get_mac_csr(ADDRL);
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Sascha Hauer s.hauer@pengutronix.de --- drivers/net/smc911x.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/smc911x.h b/drivers/net/smc911x.h index 80d2ce0..2b01cf5 100644 --- a/drivers/net/smc911x.h +++ b/drivers/net/smc911x.h @@ -382,6 +382,7 @@ static inline void smc911x_reg_write(u32 addr, u32 val) #define CHIP_9216 0x116a #define CHIP_9217 0x117a #define CHIP_9218 0x118a +#define CHIP_9220 0x9220
struct chip_id { u16 id; @@ -398,6 +399,7 @@ static const struct chip_id chip_ids[] = { { CHIP_9216, "LAN9216" }, { CHIP_9217, "LAN9217" }, { CHIP_9218, "LAN9218" }, + { CHIP_9220, "LAN9220" }, { 0, NULL }, };
On boards without EEPROMs, don't reset the chip on U-Boot's exit so that the MAC set by environment settings can be used by the OS later.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Sascha Hauer s.hauer@pengutronix.de --- drivers/net/smc911x.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index 8c9a2a8..f777ae9 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -225,7 +225,9 @@ int eth_send(volatile void *packet, int length)
void eth_halt(void) { +#ifndef CONFIG_DRIVER_SMC911X_NO_EEPROM smc911x_reset(); +#endif }
int eth_rx(void)
On Wednesday 08 April 2009 07:23:39 Daniel Mack wrote:
On boards without EEPROMs, don't reset the chip on U-Boot's exit so that the MAC set by environment settings can be used by the OS later.
that isnt how the MAC is passed to the OS ... this change is incorrect
the OS must be able to get the MAC address regardless of the state of the network controller -mike
On Wed, Apr 08, 2009 at 06:00:40PM -0400, Mike Frysinger wrote:
On Wednesday 08 April 2009 07:23:39 Daniel Mack wrote:
On boards without EEPROMs, don't reset the chip on U-Boot's exit so that the MAC set by environment settings can be used by the OS later.
that isnt how the MAC is passed to the OS ... this change is incorrect
the OS must be able to get the MAC address regardless of the state of the network controller
Not if the MAC is stored in the volatile smc911x registers. Issuing a soft reset flushes these values - if U-Boot does that, the OS has no change getting them.
That's what I saw here and it is also stated in the datasheet.
Daniel
On Wednesday 08 April 2009 20:08:38 Daniel Mack wrote:
On Wed, Apr 08, 2009 at 06:00:40PM -0400, Mike Frysinger wrote:
On Wednesday 08 April 2009 07:23:39 Daniel Mack wrote:
On boards without EEPROMs, don't reset the chip on U-Boot's exit so that the MAC set by environment settings can be used by the OS later.
that isnt how the MAC is passed to the OS ... this change is incorrect
the OS must be able to get the MAC address regardless of the state of the network controller
Not if the MAC is stored in the volatile smc911x registers. Issuing a soft reset flushes these values - if U-Boot does that, the OS has no change getting them.
then either your u-boot or your OS is misconfigured and you need to fix that. as clearly stated in docs/README.enetaddr, the environment is the place where mac addresses live when there is no dedicated storage (like an eeprom).
ignoring that, the mac address doesnt magically get programmed. if no network operation was initiated, then the part wouldnt have been programmed anyways, so you're still left with an OS that cant get itself functional.
That's what I saw here and it is also stated in the datasheet.
sounds like the driver and hardware is operating correctly then -mike
On Wed, Apr 08, 2009 at 11:57:37PM -0400, Mike Frysinger wrote:
Not if the MAC is stored in the volatile smc911x registers. Issuing a soft reset flushes these values - if U-Boot does that, the OS has no change getting them.
then either your u-boot or your OS is misconfigured and you need to fix that. as clearly stated in docs/README.enetaddr, the environment is the place where mac addresses live when there is no dedicated storage (like an eeprom).
ignoring that, the mac address doesnt magically get programmed. if no network operation was initiated, then the part wouldnt have been programmed anyways, so you're still left with an OS that cant get itself functional.
Ok, true. Thanks for pointing this out.
Daniel
On Tuesday 21 April 2009 07:13:10 Daniel Mack wrote:
On Wed, Apr 08, 2009 at 11:57:37PM -0400, Mike Frysinger wrote:
Not if the MAC is stored in the volatile smc911x registers. Issuing a soft reset flushes these values - if U-Boot does that, the OS has no change getting them.
then either your u-boot or your OS is misconfigured and you need to fix that. as clearly stated in docs/README.enetaddr, the environment is the place where mac addresses live when there is no dedicated storage (like an eeprom).
ignoring that, the mac address doesnt magically get programmed. if no network operation was initiated, then the part wouldnt have been programmed anyways, so you're still left with an OS that cant get itself functional.
Ok, true. Thanks for pointing this out.
usually what i suggest to people are things like: - pass $(ethaddr) via kernel command line and parse /proc/cmdline - use the u-boot tools to read the u-boot env directly - set the hw address with `ifconfig` or similar tool -mike
On Sun, Apr 26, 2009 at 11:14:06PM -0400, Mike Frysinger wrote:
On Tuesday 21 April 2009 07:13:10 Daniel Mack wrote:
On Wed, Apr 08, 2009 at 11:57:37PM -0400, Mike Frysinger wrote:
Not if the MAC is stored in the volatile smc911x registers. Issuing a soft reset flushes these values - if U-Boot does that, the OS has no change getting them.
then either your u-boot or your OS is misconfigured and you need to fix that. as clearly stated in docs/README.enetaddr, the environment is the place where mac addresses live when there is no dedicated storage (like an eeprom).
ignoring that, the mac address doesnt magically get programmed. if no network operation was initiated, then the part wouldnt have been programmed anyways, so you're still left with an OS that cant get itself functional.
Ok, true. Thanks for pointing this out.
usually what i suggest to people are things like:
- pass $(ethaddr) via kernel command line and parse /proc/cmdline
- use the u-boot tools to read the u-boot env directly
- set the hw address with `ifconfig` or similar tool
... which doesn't help much when it's about booting from NFS root. The particular problem here is that the MAC changes from what it's set to in U-Boot during the TFTP transaction and the randomly chosen one in Linux (which takes place because after the reset, the MAC is all 0xff). And that in turn confuses the ARP caches on other hosts. My patch solves that issue for that very case; that's why it's still in my local tree.
Daniel
On Monday 27 April 2009 10:44:16 Daniel Mack wrote:
On Sun, Apr 26, 2009 at 11:14:06PM -0400, Mike Frysinger wrote:
On Tuesday 21 April 2009 07:13:10 Daniel Mack wrote:
On Wed, Apr 08, 2009 at 11:57:37PM -0400, Mike Frysinger wrote:
Not if the MAC is stored in the volatile smc911x registers. Issuing a soft reset flushes these values - if U-Boot does that, the OS has no change getting them.
then either your u-boot or your OS is misconfigured and you need to fix that. as clearly stated in docs/README.enetaddr, the environment is the place where mac addresses live when there is no dedicated storage (like an eeprom).
ignoring that, the mac address doesnt magically get programmed. if no network operation was initiated, then the part wouldnt have been programmed anyways, so you're still left with an OS that cant get itself functional.
Ok, true. Thanks for pointing this out.
usually what i suggest to people are things like:
- pass $(ethaddr) via kernel command line and parse /proc/cmdline
- use the u-boot tools to read the u-boot env directly
- set the hw address with `ifconfig` or similar tool
... which doesn't help much when it's about booting from NFS root.
it works fine if you use an initramfs like everyone suggests -mike
On Mon, Apr 27, 2009 at 11:56:17AM -0400, Mike Frysinger wrote:
On Monday 27 April 2009 10:44:16 Daniel Mack wrote:
On Sun, Apr 26, 2009 at 11:14:06PM -0400, Mike Frysinger wrote:
usually what i suggest to people are things like:
- pass $(ethaddr) via kernel command line and parse /proc/cmdline
- use the u-boot tools to read the u-boot env directly
- set the hw address with `ifconfig` or similar tool
... which doesn't help much when it's about booting from NFS root.
it works fine if you use an initramfs like everyone suggests
Or when you use a device tree. :-)
-Scott
Daniel Mack wrote:
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Sascha Hauer s.hauer@pengutronix.de
drivers/net/smc911x.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/smc911x.h b/drivers/net/smc911x.h index 80d2ce0..2b01cf5 100644 --- a/drivers/net/smc911x.h +++ b/drivers/net/smc911x.h @@ -382,6 +382,7 @@ static inline void smc911x_reg_write(u32 addr, u32 val) #define CHIP_9216 0x116a #define CHIP_9217 0x117a #define CHIP_9218 0x118a +#define CHIP_9220 0x9220
struct chip_id { u16 id; @@ -398,6 +399,7 @@ static const struct chip_id chip_ids[] = { { CHIP_9216, "LAN9216" }, { CHIP_9217, "LAN9217" }, { CHIP_9218, "LAN9218" },
- { CHIP_9220, "LAN9220" }, { 0, NULL },
};
Applied to net/next branch.
thanks, Ben
On Wed, Apr 08, 2009 at 01:23:37PM +0200, Daniel Mack wrote:
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Sascha Hauer s.hauer@pengutronix.de
ping.
drivers/net/smc911x.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index 30f2dc2..8c9a2a8 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -41,8 +41,13 @@ static int smx911x_handle_mac_address(bd_t *bd) unsigned long addrh, addrl; uchar m[6];
- /* if the environment has a valid mac address then use it */
- if (!eth_getenv_enetaddr("ethaddr", m)) {
- if (eth_getenv_enetaddr("ethaddr", m)) {
/* if the environment has a valid mac address then use it */addrl = m[0] | (m[1] << 8) | (m[2] << 16) | (m[3] << 24);addrh = m[4] | (m[5] << 8);smc911x_set_mac_csr(ADDRL, addrl);smc911x_set_mac_csr(ADDRH, addrh);- } else { /* if not, try to get one from the eeprom */ addrh = smc911x_get_mac_csr(ADDRH); addrl = smc911x_get_mac_csr(ADDRL);
-- 1.6.2.1
On Wed, Apr 08, 2009 at 01:23:37PM +0200, Daniel Mack wrote:
If the MAX address is given by the environment, write it back to the hardware.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Sascha Hauer s.hauer@pengutronix.de
Acked-by: Sascha Hauer s.hauer@pengutronix.de
Anyway, you shouldn't rely on this. I'm the original author of this driver, but I do not use U-Boot-v1 anymore, so I can't tell if this breaks something or not.
Sascha
drivers/net/smc911x.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index 30f2dc2..8c9a2a8 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -41,8 +41,13 @@ static int smx911x_handle_mac_address(bd_t *bd) unsigned long addrh, addrl; uchar m[6];
- /* if the environment has a valid mac address then use it */
- if (!eth_getenv_enetaddr("ethaddr", m)) {
- if (eth_getenv_enetaddr("ethaddr", m)) {
/* if the environment has a valid mac address then use it */addrl = m[0] | (m[1] << 8) | (m[2] << 16) | (m[3] << 24);addrh = m[4] | (m[5] << 8);smc911x_set_mac_csr(ADDRL, addrl);smc911x_set_mac_csr(ADDRH, addrh);- } else { /* if not, try to get one from the eeprom */ addrh = smc911x_get_mac_csr(ADDRH); addrl = smc911x_get_mac_csr(ADDRL);
-- 1.6.2.1
Hi Sascha,
On Tue, Apr 21, 2009 at 01:38:23PM +0200, Sascha Hauer wrote:
On Wed, Apr 08, 2009 at 01:23:37PM +0200, Daniel Mack wrote:
If the MAX address is given by the environment, write it back to the hardware.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Sascha Hauer s.hauer@pengutronix.de
Acked-by: Sascha Hauer s.hauer@pengutronix.de
Anyway, you shouldn't rely on this. I'm the original author of this driver, but I do not use U-Boot-v1 anymore, so I can't tell if this breaks something or not.
No problem. I just Cc'ed all email addresses I could find in the code.
Who will pick that one and the one regarding support for the 9220 controller for mainline?
Thanks, Daniel
Hi Daniel,
On Tue, Apr 21, 2009 at 4:44 AM, Daniel Mack daniel@caiaq.de wrote:
Hi Sascha,
On Tue, Apr 21, 2009 at 01:38:23PM +0200, Sascha Hauer wrote:
On Wed, Apr 08, 2009 at 01:23:37PM +0200, Daniel Mack wrote:
If the MAX address is given by the environment, write it back to the hardware.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Sascha Hauer s.hauer@pengutronix.de
Acked-by: Sascha Hauer s.hauer@pengutronix.de
Anyway, you shouldn't rely on this. I'm the original author of this driver, but I do not use U-Boot-v1 anymore, so I can't tell if this breaks something or not.
No problem. I just Cc'ed all email addresses I could find in the code.
Who will pick that one and the one regarding support for the 9220 controller for mainline?
Network drivers are my responsibility. Please CC me in the future as it stands out more clearly. I guess I was waiting on you to resolve 3/3, but instead can pick up parts 1 and 2 if you like.
Thanks, Daniel
regards, Ben
On Tue, Apr 21, 2009 at 06:28:34AM -0700, Ben Warren wrote:
Anyway, you shouldn't rely on this. I'm the original author of this driver, but I do not use U-Boot-v1 anymore, so I can't tell if this breaks something or not.
No problem. I just Cc'ed all email addresses I could find in the code.
Who will pick that one and the one regarding support for the 9220 controller for mainline?
Network drivers are my responsibility. Please CC me in the future as it stands out more clearly. I guess I was waiting on you to resolve 3/3, but instead can pick up parts 1 and 2 if you like.
I'll try to solve 3/3 in another fashion, so feel free to merge the other two.
Thanks, Daniel
Daniel Mack wrote:
If the MAX address is given by the environment, write it back to the hardware.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Sascha Hauer s.hauer@pengutronix.de
drivers/net/smc911x.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index 30f2dc2..8c9a2a8 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -41,8 +41,13 @@ static int smx911x_handle_mac_address(bd_t *bd) unsigned long addrh, addrl; uchar m[6];
- /* if the environment has a valid mac address then use it */
- if (!eth_getenv_enetaddr("ethaddr", m)) {
- if (eth_getenv_enetaddr("ethaddr", m)) {
/* if the environment has a valid mac address then use it */addrl = m[0] | (m[1] << 8) | (m[2] << 16) | (m[3] << 24);addrh = m[4] | (m[5] << 8);smc911x_set_mac_csr(ADDRL, addrl);smc911x_set_mac_csr(ADDRH, addrh);- } else { /* if not, try to get one from the eeprom */ addrh = smc911x_get_mac_csr(ADDRH); addrl = smc911x_get_mac_csr(ADDRL);
Applied to net/next branch.
thanks, Ben
Ben Warren wrote:
Daniel Mack wrote:
If the MAX address is given by the environment, write it back to the hardware.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Sascha Hauer s.hauer@pengutronix.de
drivers/net/smc911x.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index 30f2dc2..8c9a2a8 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -41,8 +41,13 @@ static int smx911x_handle_mac_address(bd_t *bd) unsigned long addrh, addrl; uchar m[6];
- /* if the environment has a valid mac address then use it */
- if (!eth_getenv_enetaddr("ethaddr", m)) {
- if (eth_getenv_enetaddr("ethaddr", m)) {
/* if the environment has a valid mac address then use it */addrl = m[0] | (m[1] << 8) | (m[2] << 16) | (m[3] << 24);addrh = m[4] | (m[5] << 8);smc911x_set_mac_csr(ADDRL, addrl);smc911x_set_mac_csr(ADDRH, addrh);- } else { /* if not, try to get one from the eeprom */ addrh = smc911x_get_mac_csr(ADDRH); addrl = smc911x_get_mac_csr(ADDRL);
Applied to net/next branch.
Could we get this asap into mainline? Sounds like an urgent bugfix to me:
http://lists.denx.de/pipermail/u-boot/2009-May/053079.html
Many thanks
Dirk
Dear Ben,
In message 4A14E0AB.7010906@googlemail.com Dirk Behme wrote:
If the MAX address is given by the environment, write it back to the hardware.
Signed-off-by: Daniel Mack daniel@caiaq.de Cc: Sascha Hauer s.hauer@pengutronix.de
...
Applied to net/next branch.
Could we get this asap into mainline? Sounds like an urgent bugfix to me:
I agree with Dirk here.
Can we pull this into master, please?
Best regards,
Wolfgang Denk
On Wednesday 08 April 2009 07:23:37 Daniel Mack wrote:
If the MAX address is given by the environment, write it back to the hardware.
Signed-off-by: Mike Frysinger vapier@gentoo.org -mike
participants (7)
-
Ben Warren -
Daniel Mack -
Dirk Behme -
Mike Frysinger -
Sascha Hauer -
Scott Wood -
Wolfgang Denk