On Sat, Mar 17, 2007 at 07:22:27PM -0500, Timur Tabi wrote:

David Gibson wrote:

quoted

       while ((addr = va_arg(ap, void *))) {
               devp = find_node_by_prop_value(NULL,
linux,network-index",
                                              &index, sizeof(index));

               if (devp)
                      setprop(devp, "local-mac-address", addr, 6);

The problem with this version is that it only updates local-mac-address, 
and only if it already exists.  

Uh.. no.  Both my version and Scott's use setprop.  I think at present
that won't add the property if it doesn't exist, but that's a bug
which one of Scott's other patches fixes.  In any case, they have the
same behaviour in terms of whether the property needs to exist first.

Scott's version also updates 
mac-address.  

Well, first, details shmetails.  It's the interface and approach I
thin is better.  Second, I don't think the zImage should be setting
mac-address anyway.  In OF that property is based on what the
interface has been used for during booting, which the zImage doesn't
know.  The kernel doesn't need mac-address if local-mac-address is
present, so we should just leave it out.

Also, in the future, we hope to eliminate *all* MAC 
address entries from the DTS files, and the boot loader and/or 
bootwrapper will add one.

I don't think that's really a good idea.  The bootloader certainly
should be able to add the property if it's not there, but it seems
silly to make the bootloader do memmove()s to insert a new property
when it's basically just as easy to set up the device tree to allow an
in-place edit.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson