On Wed, 19 Aug 2009 10:08:02 -0700
Ben Warren <biggerbadder...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
