On 12/02/2011 04:14 PM, Mike Frysinger wrote:
On Friday 02 December 2011 17:02:00 Rob Herring wrote:
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.
i think Wolfgang would tell you it's a mandate, and code you see using register offsets are the old style that should get updated. sorry.
- 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.
what i mean is that on embedded peripheral blocks, the device powers on with blank register settings and the core needs to program them. how did your device get a mac address already programmed into it ? did something run before u-boot and initialize the registers ? does the hardware block preseed the registers itself by talking to some internal storage ? certainly the mac address isn't programmed into the hardware block itself :).
Something else runs and sets it up and u-boot does not have access to it.
- 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.
device drivers should not be touching the env. common code takes care of that. your driver should only be writing dev->enetaddr.
The common code does not set the env setting. The env setting is normally an override of the h/w value and may not even exist. So how should the pxe boot command get the MAC address?
- move setting of ethXaddr env to the highbank board file - Have PXE call eth_get_dev and get it directly from struct eth_device.
The latter is the only way to enable all boards at once.
Rob