From: Wolfgang Denk Date: Wed, 19 Aug 2009 21:25:10 +0200 Subject: [U-Boot] [PATCH] arm:kirkwood Wait for the link to come up on kirkwood network init In-Reply-To: <4A8C3172.70001@gmail.com> References: <20090819122415.0487b2d8@marrow.netinsight.se> <4A8C3172.70001@gmail.com> Message-ID: <20090819192510.BD75D833DBD2@gemini.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 Dear Ben Warren, In message <4A8C3172.70001@gmail.com> you wrote: > > > - if (!miiphy_link(dev->name, phyadr)) { > > - printf("%s: No link on %s\n", __FUNCTION__, dev->name); > > > Please use __func__ instead. It's defined in C99, while __FUNCTION__ > isn't (or so I've read) Do we really need the function name at all? This sounds more like a debug message for the developper than a standard error message. > > + /* Wait up to 5s for the link status */ > > + for (i = 0; i < 5; i++) { > > + u16 phyadr; > > > Please put this variable declaration outside of the 'for' loop Why? If it's only used in this block it's actually a good thing to restrict it to this scope. 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 at denx.de Man is the best computer we can put aboard a spacecraft ... and the only one that can be mass produced with unskilled labor. -- Wernher von Braun