Re: [U-Boot] net, fec_mxc: init mac address before using network commands
On Tuesday 30 March 2010 01:38:33 Heiko Schocher wrote: initialize mac address with the value from ethaddr, before doing some network commands. This is not in line with u-boot design principle not to initalize not used devices, and maybe should go away, if there is a solution for passing the mac address to arm linux kernels. NACK like i already said -- net drivers should only ever be reading eth_driver-enetaddr. they should never touch the environment as the common net code takes care of this. i would instead fix the fec_probe() to stop writing ethaddr since it already writes to eth_driver-enetaddr. -mike signature.asc Description: This is a digitally signed message part. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] net, fec_mxc: init mac address before using network commands
Dear Heiko Schocher,
In message 4bb18e59.5000...@denx.de you wrote:
initialize mac address with the value from ethaddr, before
doing some network commands. This is not in line with u-boot
design principle not to initalize not used devices, and
maybe should go away, if there is a solution for passing
the mac address to arm linux kernels.
Tested on the magnesium board.
Note 1: I would have expected that this commit is marked as a
follow-up to your earlier posting from Wed, 24 Mar 2010,
titled [PATCH] net, fec_mxc: use mac address stored in env
before looking in eeprom
(http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/76173)
Unfortunately your new posting contains no indication of a
previous patch (i. e. there is no v2 or similar in the
Subject, nor is there a description of the changes against the
previous version below the --- line. In addition, the
Subject has been changed which makes it even more difficult
to match this to the older posting.
Note 2: I think this description is actually incomplete. You are
mixing two independent modifications here.
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 5af9cdb..9029490 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -749,11 +749,18 @@ static int fec_probe(bd_t *bd)
eth_register(edev);
- if (fec_get_hwaddr(edev, ethaddr) == 0) {
- printf(got MAC address from EEPROM: %pM\n, ethaddr);
- memcpy(edev-enetaddr, ethaddr, 6);
- fec_set_hwaddr(edev);
+ if (!eth_getenv_enetaddr(ethaddr, ethaddr)) {
+ /* ethaddr is not set in the environment */
+ if (fec_get_hwaddr(edev, ethaddr) == 0) {
+ printf(got MAC address from EEPROM: %pM\n, ethaddr);
+ eth_setenv_enetaddr(ethaddr, ethaddr);
+ } else {
+ printf (no MAC found\n);
+ return -1;
+ }
This part of the commit is the previously dicussed bug fix: without
this change, the board would, under certain conditions, ignore the
ethaddr setting stored in the environment. This is a clear bug fix
and as such should get added to the current code. As far as I can
tell, this part does not violate any design rules.
}
+ memcpy(edev-enetaddr, ethaddr, 6);
+ fec_set_hwaddr(edev);
This is a completely different issue. Here you add new code to change
the system behaviour in a way that indeed violates the design rules.
Please split this patch into two separate commits. The first part is
obviously a fix. I will ACK it and even pull it in the upcoming
release (assuming you are fast enough).
I dislike the second part, but I will not actively block it either.
Let's see what others (especially Ben) say about it.
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: w...@denx.de
HR Manager to job candidate I see you've had no computer training.
Although that qualifies you for upper management, it means you're
under-qualified for our entry level positions.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] net, fec_mxc: init mac address before using network commands
Hello Heiko, initialize mac address with the value from ethaddr, before doing some network commands. This is not in line with u-boot design principle not to initalize not used devices, and maybe should go away, if there is a solution for passing the mac address to arm linux kernels. Tested on the magnesium board. Signed-off-by: Heiko Schocher h...@denx.de Acked-by: Detlev Zundel d...@denx.de Cheers Detlev -- I think that level of generalization is too abstract for useful thinking. -- Richard Stallman in e19n344-0006q9...@fencepost.gnu.org -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] net, fec_mxc: init mac address before using network commands
initialize mac address with the value from ethaddr, before
doing some network commands. This is not in line with u-boot
design principle not to initalize not used devices, and
maybe should go away, if there is a solution for passing
the mac address to arm linux kernels.
Tested on the magnesium board.
Signed-off-by: Heiko Schocher h...@denx.de
---
posting this patch as a result of this discussion:
http://lists.denx.de/pipermail/u-boot/2010-March/069025.html
drivers/net/fec_mxc.c | 15 +++
1 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 5af9cdb..9029490 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -749,11 +749,18 @@ static int fec_probe(bd_t *bd)
eth_register(edev);
- if (fec_get_hwaddr(edev, ethaddr) == 0) {
- printf(got MAC address from EEPROM: %pM\n, ethaddr);
- memcpy(edev-enetaddr, ethaddr, 6);
- fec_set_hwaddr(edev);
+ if (!eth_getenv_enetaddr(ethaddr, ethaddr)) {
+ /* ethaddr is not set in the environment */
+ if (fec_get_hwaddr(edev, ethaddr) == 0) {
+ printf(got MAC address from EEPROM: %pM\n, ethaddr);
+ eth_setenv_enetaddr(ethaddr, ethaddr);
+ } else {
+ printf (no MAC found\n);
+ return -1;
+ }
}
+ memcpy(edev-enetaddr, ethaddr, 6);
+ fec_set_hwaddr(edev);
return 0;
}
--
1.6.2.5
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
