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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot