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