From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Wed, 31 Mar 2010 05:56:38 -0000 Subject: [U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup In-Reply-To: <4BB2785B.50003@gmail.com> References: <4BB238E9.7060609@denx.de> <20100330203400.8EBE5E73028@gemini.denx.de> <4BB2785B.50003@gmail.com> Message-ID: <4C070C16.5000001@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hello Ben, Ben Warren wrote: > Wolfgang, > > On 3/30/2010 1:34 PM, Wolfgang Denk wrote: >> Dear Heiko Schocher, >> >> In message<4BB238E9.7060609@denx.de> you wrote: >> >>> if ethaddr is not setup in the environment, fill the device >>> enetaddr with the contents of the eeprom, and only >>> the device enetaddr, not the mac address registers! >>> >>> Tested on the magnesium board. >>> >>> Signed-off-by: Heiko Schocher >>> --- >>> - changes since v1 posted here: >>> http://lists.denx.de/pipermail/u-boot/2010-March/069192.html >>> >>> - splitted in two patches as Wolfgang suggested >>> >> Thanks. Note that it would also have been an excellent idea to put >> the responsible custodian on Cc: >> >> >> >>> drivers/net/fec_mxc.c | 9 +++++---- >>> 1 files changed, 5 insertions(+), 4 deletions(-) >>> >> Applied, thanks. >> >> >> Ben, this is (as far as I see it) an undisputed bug fix, so I'm >> pulling this patch (and only this one from this series of 4) >> directly. Hope this is ok with you. >> >> >> > Hold on a second. This patch is wrong. As Mike has pointed out, the Now I got lost ... I think the "critical" part of my "patch v1" is splitted out to "2/2 v2" ... or? > net library already gets the MAC address from the environment. The > correct flow is: > > 1. Read from hardware in initialize() function Ah, Ok, so that is the right way? If so, then I should remove "+ if (!eth_getenv_enetaddr("ethaddr", ethaddr)) {" from my patch, and then it should be OK, right? Actual fec_mxc.c driver is *not* correct, because if in eeprom is a correct mac, it *always* programms this in the mac address registers from the chip! This is not OK, and must be fixed! > 2. Read from environment in net/eth.c after initialize() > 3. Give priority to the value in the environment if a conflict > 4. Program hardware in the device's init() function. > > If somebody wants to subvert the 'design philosophy', the right way is > to call eth_dev->init() in board code. Maybe this list should go in a doc? bye Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany