[U-Boot] [PATCH 1/2] net: add Calxeda xgmac driver

Mike Frysinger
2011-12-02T23:54:32Z

On Friday 02 December 2011 17:46:39 Rob Herring wrote:
> 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:
> >>>> + 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.

isn't this a board-specific assumption ? shouldn't those lines be protected by
a CONFIG_xxx ifdef that boards can enable as they see fit ?

> >>>> + 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.

i vaguely recall someone else mentioning this, but it doesn't change the
driver logic. no net driver may touch the env, FIN. we can talk about
improving the core/common net code. but let's do so in a dedicated thread.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20111202/54e86324/attachment.pgp>