Mike, Thanks for your quick review.
On 12/02/2011 03:30 PM, Mike Frysinger wrote: > On Friday 02 December 2011 15:21:48 Rob Herring wrote: >> --- /dev/null >> +++ b/drivers/net/calxedaxgmac.c >> >> + writel(value, dev->iobase + XGMAC_CORE_CONFIG); > > you should declare a C struct that represents the hardware's register layout, > and then use that rather than iobase+register_offset > Is that a suggestion or u-boot mandate? Because the Linux version of the driver does it the current way already, it's certainly done both ways in u-boot drivers already and personally I really don't like structs for register offsets. ... >> + macaddr[1] = readl(dev->iobase + XGMAC_CORE_MACADDR0HI); >> + macaddr[0] = readl(dev->iobase + XGMAC_CORE_MACADDR0LO); >> + memcpy(dev->enetaddr, macaddr, 6); > > does the initial mac regs really start off with useful info ? Yes. It contains the only value that will work. >> + sprintf(enetvar, id ? "eth%daddr" : "ethaddr", id); >> + eth_setenv_enetaddr(enetvar, dev->enetaddr); > > NAK: delete this PXE boot needs the MAC address to generate filenames and gets it from the env. See format_mac_pxe function in common/cmd_pxe.c. Should that be done differently? The user setting a MAC address on our platform won't work, so using the env setting as an override is not valid. Rob _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot