[U-Boot] [RFC] Program net device MAC addresses after initializing

[U-Boot] [RFC] Program net device MAC addresses after initializing

[Ben Warren]

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.

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.

Signed-off-by: Ben Warren <biggerbadderben@gmail.com>
---
 include/net.h |    1 +
 net/eth.c     |    4 ++++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/net.h b/include/net.h
index 3f6a5d1..a180881 100644
--- a/include/net.h
+++ b/include/net.h
@@ -105,6 +105,7 @@ struct eth_device {
 #ifdef CONFIG_MCAST_TFTP
 	int (*mcast) (struct eth_device*, u32 ip, u8 set);
 #endif
+	int  (*write_hwaddr) (struct eth_device*);
 	struct eth_device *next;
 	void *priv;
 };
diff --git a/net/eth.c b/net/eth.c
index b650a20..ee7678a 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -241,6 +241,10 @@ int eth_initialize(bd_t *bis)
 
 				memcpy(dev->enetaddr, env_enetaddr, 6);
 			}
+			if (dev->write_hwaddr &&
+				is_valid_ether_addr(dev->enetaddr)) {
+				dev->write_hwaddr(dev);
+			}
 
 			eth_number++;
 			dev = dev->next;
-- 
1.6.0.4

[U-Boot] [RFC] Program net device MAC addresses after initializing

[Heiko Schocher]

Hello Ben,

Ben Warren wrote:
> 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.
> 
> 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.
> 
> Signed-off-by: Ben Warren <biggerbadderben@gmail.com>

Acked-by: Heiko Schocher <hs@denx.de>

bye
Heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [RFC] Program net device MAC addresses after initializing

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de 
> [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Heiko Schocher
> Sent: Tuesday, April 06, 2010 12:49 PM
> To: Ben Warren
> Cc: u-boot at lists.denx.de
> Subject: Re: [U-Boot] [RFC] Program net device MAC addresses 
> after initializing
> 
> Hello Ben,
> 
> Ben Warren wrote:
> > 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.
> > 
> > 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.
> > 
> > Signed-off-by: Ben Warren <biggerbadderben@gmail.com>
> 
> Acked-by: Heiko Schocher <hs@denx.de>

Acked-by: Prafulla Wadaskar <prafulla@marvell.com>

Regards..
Prafulla . .

> 
> bye
> Heiko
> -- 
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 

[U-Boot] [RFC] Program net device MAC addresses after initializing

[Wolfgang Denk]

Dear Ben Warren,

In message <1270450929-17004-1-git-send-email-biggerbadderben@gmail.com> you wrote:
> 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.
> 
> 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.

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.

[U-Boot] [RFC] Program net device MAC addresses after initializing

[Ben Warren]

Hi Wolfgang,

On 4/6/2010 5:57 AM, Wolfgang Denk wrote:
> Dear Ben Warren,
>
> In message<1270450929-17004-1-git-send-email-biggerbadderben@gmail.com>  you wrote:
>    
>> 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.
>>
>> 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.
>>      
> 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.
>  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.

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.

> 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.
> Thanks.
>
> Best regards,
>
> Wolfgang Denk
>
>    
regards,
Ben

[U-Boot] [RFC] Program net device MAC addresses after initializing

[Detlev Zundel]

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.

[U-Boot] [RFC] Program net device MAC addresses after initializing

[Wolfgang Denk]

Dear Ben Warren,

In message <4BBB6470.30604@gmail.com> you wrote:
> 
> 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.

I think such an 'opt-out' ability is important.

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

There are zillions of different Ethernet controllers out there. To be
able to program the MAC you might need to
- map the respective memory area first, i. e. twiddle memory
  controller settings
- power of the Ethernet controller (which might be kept powered off or
  otherwise disable normally to minimize power consumption)
- take the controller out of reset
- configure clocks needed to breath life into the controller
...

I don;t have a specific example in mind where it would actually cause
harm, but we might run into such situations and should be prepared to
handle them gracefully.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Men of peace usually are [brave].
	-- Spock, "The Savage Curtain", stardate 5906.5

[U-Boot] [RFC] Program net device MAC addresses after initializing

[Ben Warren]

On 4/9/2010 12:58 PM, Wolfgang Denk wrote:
> Dear Ben Warren,
>
> In message<4BBB6470.30604@gmail.com>  you wrote:
>    
>> 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.
>>      
> I think such an 'opt-out' ability is important.
>
>    
>> 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.
>>      
> There are zillions of different Ethernet controllers out there. To be
> able to program the MAC you might need to
> - map the respective memory area first, i. e. twiddle memory
>    controller settings
> - power of the Ethernet controller (which might be kept powered off or
>    otherwise disable normally to minimize power consumption)
> - take the controller out of reset
> - configure clocks needed to breath life into the controller
> ...
>
> I don;t have a specific example in mind where it would actually cause
> harm, but we might run into such situations and should be prepared to
> handle them gracefully.
>
>    
OK.  The next spin will have an easy opt-out, apart from 'setenv 
eth1addr 00:00:00:00:00:00'
> Best regards,
>
> Wolfgang Denk
>
>    
regards,
Ben

[U-Boot] [RFC] Program net device MAC addresses after initializing

[Wolfgang Denk]

Dear Ben Warren,

In message <4BBFC784.20604@gmail.com> you wrote:
>
> > I don;t have a specific example in mind where it would actually cause
> > harm, but we might run into such situations and should be prepared to
> > handle them gracefully.
> >
> >    
> OK.  The next spin will have an easy opt-out, apart from 'setenv 
> eth1addr 00:00:00:00:00:00'

Thanks - setting an invalid MAC address is something that makes
little sense to me: valid MAC address information might be needed
anyway, for example to passs it on to some OS we are booting.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Weekends were made for programming.                 - Karl Lehenbauer