Hi Ben,

>>> Add a new function to the eth_device struct for programming a network
>>> controller's hardware address.
>>>
>>> After all network devices have been initialized and the proper MAC address 
>>> for
>>> each has been determined, make a device driver call to program the address
>>> into the device.  Only device instances with valid unicast addresses will be
>>> programmed.

Thanks for picking up this thread again.

>>> This is a significant departure from existing U-boot behavior, but costs 
>>> very
>>> little in startup time and addresses a very common complaint among 
>>> developers.

As I have discovered, there are a few device drivers _currently_ doing
this.  So actually we will close the gap of documentation/current
practice/design goals.

>> The thing is that this _is_ a violation of the design rules, and we
>> should not make assumptions that such an initialization is harmless
>> for all systems.
>>
>>    
> I know this differs from the existing design rules.  I'm not trying to 
> be subtle or create a loophole, I'm trying to change policy.  You'll 
> notice that by itself, this patch does nothing other than chew up a few 
> CPU cycles and bytes of flash.

I second this effort - actually I had a patch looking very much the same
in my repository.  I wanted to attach it to an existing driver before
posting it, but this is just as good.

>>  From the patch it is not clear to me who is supposed to implement
>> write_hwaddr() - it should be made clear that this should be be done
>> only when absolutely necessary, and then best in board specific code,
>>
>>    
> The new function is part of the 'eth_device struct', so will be 
> implemented in the network drivers.  As designed, MAC addresses will be 
> programmed on all controllers that have a valid entry either in their 
> NVRAM or the environment.  If somebody goes to the effort of putting a 
> valid address in one of these places, we should assume that he or she 
> wanted it to be used.  If there is no such entry or the driver doesn't 
> implement this method, nothing happens.  I have an idea for providing a 
> board-level 'opt-out' ability, but doubt that it would be used much.

As stated above, I also would have implemented it inside of a netword
driver and thus not at a board level.

> I'm interested in knowing use cases where programming a MAC address is 
> harmful, keeping in mind that this new code only programs valid MAC 
> addresses.

>From my perspective, I would wait for such a case and _then_ provide an
opt-out.  This may seem pragmatic, but it also follows the KISS
principle.

>> The patch should add such a description to the documentation.
>>    
> Absolutely.  This is an RFC and if we can reach agreement that it's a 
> good thing, all the appropriate documentation will be revised.
>> Also, we should remove / adapt existing code that performs basicly the
>> same action.
>>    
> Most if not all drivers currently have a private function for 
> programming MAC addresses.  We can either modify them as time goes by, 
> or I'll take on the effort of fixing them all up with the obvious caveat 
> that very little testing will be performed by me.

While looking at all this, I came to the conclusion that this is quite a
lot to fix, so I would vote for incremental changes on a per-driver
level.  Maybe provide a deprecated #warning or something comparable to
NET_MULTI.

In any case -

Acked-by: Detlev Zundel <d...@denx.de>

And I will surely help you transform drivers that I can test!

Cheers
  Detlev

-- 
Shin: a device for finding furniture in the dark.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to