Re: [PATCH 10/17] bootwrapper: Add dt_set_mac_addresses().

[Timur Tabi <timur@freescale.com>]

David Gibson wrote:

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

Sorry, I was reading the code wrong.  I saw "if (devp)" and I thought devp was a pointer 
to the local-mac-address node.

> Second, I don't think the zImage should be setting
> mac-address anyway.  

Normally, that's true.  The problem is that device drivers first check mac-address and 
then local-mac-address (see of_get_mac_address()).  If the DTS define mac-address as 
something other than 00-00-00-00-00-00, the drivers are going to see mac-address that and 
use it.

Obviously, the DTS files shouldn't have mac-address in them.  But I haven't gotten around 
to cleaning that up, because I'm still waiting for the U-Boot maintainers to apply my 
pre-requisite patches.

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

Perhaps mac-address should be deleted instead of just ignored?  I don't know if that 
breaks kexec.

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

I think it's better if the DTS only specifies properties that the bootloader can't.  We 
already need the ability to insert properties, so what's wrong with using that?  I think 
it doesn't make sense for the DTS to specify a MAC address.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale