[U-Boot] [PATCH] arm:kirkwood Wait for the link to come up on kirkwood network init

[Simon Kagstrom]

On Wed, 19 Aug 2009 10:08:02 -0700
Ben Warren <biggerbadderben at gmail.com> wrote:

> > -	u16 phyadr;
> > -	miiphy_read(dev->name, 0xEE, 0xEE, &phyadr);
> > -	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)

I'll remove the function name part completely.

> > -		return -1;
> > +	/* 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
> > +		miiphy_read(dev->name, 0xEE, 0xEE, &phyadr);
> >   
> What does '0xEE' mean?  I know you didn't write it, but magic numbers 
> are bad.

Good question. After looking around a bit, I end up in smi_reg_read in
the same file:

  static int smi_reg_read(char *devname, u8 phy_adr, u8 reg_ofs, u16 * data)
  {
	[...]
	/* Phyadr read request */
	if (phy_adr == 0xEE && reg_ofs == 0xEE) {
		/* */
		*data = (u16) (KWGBEREG_RD(regs->phyadr) & PHYADR_MASK);
		return 0;
	[...]
  }

which is registered for the PHY reads with miiphy_register. So it's a
file-local magic number. I'll cook up another patch which adresses this.

// Simon