All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Warren <biggerbadderben@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] Blackfin: bfin_mac: hook up new write_hwaddr function
Date: Tue, 27 Apr 2010 13:07:21 -0700	[thread overview]
Message-ID: <4BD743F9.3050802@gmail.com> (raw)
In-Reply-To: <201004271532.47989.vapier@gentoo.org>

On 4/27/2010 12:32 PM, Mike Frysinger wrote:
> On Tuesday 27 April 2010 14:46:34 Ben Warren wrote:
>    
>> On 4/27/2010 11:42 AM, Mike Frysinger wrote:
>>      
>>> On Tuesday 27 April 2010 14:33:09 Ben Warren wrote:
>>>        
>>>> On 4/27/2010 11:15 AM, Mike Frysinger wrote:
>>>>          
>>>>> -	bfin_EMAC_setup_addr(dev->enetaddr);
>>>>> +	bfin_EMAC_setup_addr(dev);
>>>>>            
>>>> Are you sure you still want to program it on every init() call?  There's
>>>> nothing wrong with that, BTW...
>>>>          
>>> that's the correct documented behavior ... otherwise, there's no way of
>>> syncing the env to the hardware.  this is init(), not initialize().
>>>        
>> Hardware will already be sync'd to the env, since write_hwaddr() now
>> automatically gets called after initialize().  The only thing you gain
>> from having this call in init() is to program hardware on interfaces
>> that were skipped due to the 'ethmacskip' environment variable being set.
>>      
> your v4 patch writes the env to the hardware in the initialize() step, not the
> init() step.  if someone changes ethaddr on the fly, the common net funcs sync
> env->eth_device, but it does not call the new write_hwaddr() func.  perhaps
> you intended to make that change, but it currently doesnt do it.
> -mike
>    
I see.  I think this logic could use some rework.  It seems to me that 
we should only worry about sync'ing the device that we're trying to 
use.  If its address in the environment is different from the one in the 
device struct, we could sync and program, instead of doing all of them.  
Does that make sense?

I'll take your patch and maybe next release we can work on improving this.

regards,
Ben

  reply	other threads:[~2010-04-27 20:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-27 18:15 [U-Boot] [PATCH] Blackfin: bfin_mac: hook up new write_hwaddr function Mike Frysinger
2010-04-27 18:33 ` Ben Warren
2010-04-27 18:42   ` Mike Frysinger
2010-04-27 18:46     ` Ben Warren
2010-04-27 19:32       ` Mike Frysinger
2010-04-27 20:07         ` Ben Warren [this message]
2010-04-27 20:24           ` Mike Frysinger
2010-04-30 12:40             ` Detlev Zundel
2010-04-30 13:10               ` Mike Frysinger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BD743F9.3050802@gmail.com \
    --to=biggerbadderben@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.