[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Heiko Schocher]

if ethaddr is not setup in the environment, fill the device
enetaddr with the contents of the eeprom, and only
the device enetaddr, not the mac address registers!

Tested on the magnesium board.

Signed-off-by: Heiko Schocher <hs@denx.de>
---
- changes since v1 posted here:
  http://lists.denx.de/pipermail/u-boot/2010-March/069192.html

  - splitted in two patches as Wolfgang suggested

 drivers/net/fec_mxc.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 5af9cdb..98ff64b 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -749,10 +749,11 @@ static int fec_probe(bd_t *bd)

 	eth_register(edev);

-	if (fec_get_hwaddr(edev, ethaddr) == 0) {
-		printf("got MAC address from EEPROM: %pM\n", ethaddr);
-		memcpy(edev->enetaddr, ethaddr, 6);
-		fec_set_hwaddr(edev);
+	if (!eth_getenv_enetaddr("ethaddr", ethaddr)) {	
+		if (fec_get_hwaddr(edev, ethaddr) == 0) {
+			printf("got MAC address from EEPROM: %pM\n", ethaddr);
+			memcpy(edev->enetaddr, ethaddr, 6);
+		}
 	}

 	return 0;
-- 
1.6.2.5

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

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Wolfgang Denk]

Dear Heiko Schocher,

In message <4BB238E9.7060609@denx.de> you wrote:
> if ethaddr is not setup in the environment, fill the device
> enetaddr with the contents of the eeprom, and only
> the device enetaddr, not the mac address registers!
> 
> Tested on the magnesium board.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
> - changes since v1 posted here:
>   http://lists.denx.de/pipermail/u-boot/2010-March/069192.html
> 
>   - splitted in two patches as Wolfgang suggested

Thanks.  Note that it would also have been an excellent idea to put
the responsible custodian on Cc:


>  drivers/net/fec_mxc.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)

Applied, thanks.


Ben, this is (as far as I see it) an undisputed bug fix, so I'm
pulling this patch (and only this one from this series of 4)
directly. Hope this is ok with you.



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
Our management frequently gets lost in thought.   That's because it's
unfamiliar territory.

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Ben Warren]

Hi Wolfgang,

On 3/30/2010 1:34 PM, Wolfgang Denk wrote:
> Dear Heiko Schocher,
>
> In message<4BB238E9.7060609@denx.de>  you wrote:
>    
>> if ethaddr is not setup in the environment, fill the device
>> enetaddr with the contents of the eeprom, and only
>> the device enetaddr, not the mac address registers!
>>
>> Tested on the magnesium board.
>>
>> Signed-off-by: Heiko Schocher<hs@denx.de>
>> ---
>> - changes since v1 posted here:
>>    http://lists.denx.de/pipermail/u-boot/2010-March/069192.html
>>
>>    - splitted in two patches as Wolfgang suggested
>>      
> Thanks.  Note that it would also have been an excellent idea to put
> the responsible custodian on Cc:
>
>
>    
>>   drivers/net/fec_mxc.c |    9 +++++----
>>   1 files changed, 5 insertions(+), 4 deletions(-)
>>      
> Applied, thanks.
>
>
> Ben, this is (as far as I see it) an undisputed bug fix, so I'm
> pulling this patch (and only this one from this series of 4)
> directly. Hope this is ok with you.
>
>    
Sorry for not wading into this conversation earlier.  I have issues with 
this driver, in particular that it isn't truly a 'MULTI' driver.  This 
came to my attention when I noticed that Heiko's changes reference the 
'ethaddr' environment variable.  Hopefully somebody will fix it properly.

In any case, as you say, this fixes a real bug and doesn't make things 
worse.  Good enough, I guess.
>
> Best regards,
>
> Wolfgang Denk
>
>    
thanks,
Ben

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Mike Frysinger]

On Tuesday 30 March 2010 16:34:00 Wolfgang Denk wrote:
> Ben, this is (as far as I see it) an undisputed bug fix, so I'm
> pulling this patch (and only this one from this series of 4)
> directly. Hope this is ok with you.

are people just ignoring my e-mails ?  ive already pointed out multiple times 
why this is wrong and not what the current net standard is doing.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100330/67e0f702/attachment.pgp 

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Ben Warren]

Wolfgang,

On 3/30/2010 1:34 PM, Wolfgang Denk wrote:
> Dear Heiko Schocher,
>
> In message<4BB238E9.7060609@denx.de>  you wrote:
>    
>> if ethaddr is not setup in the environment, fill the device
>> enetaddr with the contents of the eeprom, and only
>> the device enetaddr, not the mac address registers!
>>
>> Tested on the magnesium board.
>>
>> Signed-off-by: Heiko Schocher<hs@denx.de>
>> ---
>> - changes since v1 posted here:
>>    http://lists.denx.de/pipermail/u-boot/2010-March/069192.html
>>
>>    - splitted in two patches as Wolfgang suggested
>>      
> Thanks.  Note that it would also have been an excellent idea to put
> the responsible custodian on Cc:
>
>
>    
>>   drivers/net/fec_mxc.c |    9 +++++----
>>   1 files changed, 5 insertions(+), 4 deletions(-)
>>      
> Applied, thanks.
>
>
> Ben, this is (as far as I see it) an undisputed bug fix, so I'm
> pulling this patch (and only this one from this series of 4)
> directly. Hope this is ok with you.
>
>
>    
Hold on a second.  This patch is wrong.  As Mike has pointed out, the 
net library already gets the MAC address from the environment.  The 
correct flow is:

1. Read from hardware in initialize() function
2. Read from environment in net/eth.c after initialize()
3. Give priority to the value in the environment if a conflict
4. Program hardware in the device's init() function.

If somebody wants to subvert the 'design philosophy', the right way is 
to call eth_dev->init() in board code.

regards,
Ben

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Heiko Schocher]

Hello Ben,

Ben Warren wrote:
> Wolfgang,
> 
> On 3/30/2010 1:34 PM, Wolfgang Denk wrote:
>> Dear Heiko Schocher,
>>
>> In message<4BB238E9.7060609@denx.de>  you wrote:
>>   
>>> if ethaddr is not setup in the environment, fill the device
>>> enetaddr with the contents of the eeprom, and only
>>> the device enetaddr, not the mac address registers!
>>>
>>> Tested on the magnesium board.
>>>
>>> Signed-off-by: Heiko Schocher<hs@denx.de>
>>> ---
>>> - changes since v1 posted here:
>>>    http://lists.denx.de/pipermail/u-boot/2010-March/069192.html
>>>
>>>    - splitted in two patches as Wolfgang suggested
>>>      
>> Thanks.  Note that it would also have been an excellent idea to put
>> the responsible custodian on Cc:
>>
>>
>>   
>>>   drivers/net/fec_mxc.c |    9 +++++----
>>>   1 files changed, 5 insertions(+), 4 deletions(-)
>>>      
>> Applied, thanks.
>>
>>
>> Ben, this is (as far as I see it) an undisputed bug fix, so I'm
>> pulling this patch (and only this one from this series of 4)
>> directly. Hope this is ok with you.
>>
>>
>>    
> Hold on a second.  This patch is wrong.  As Mike has pointed out, the

Now I got lost ... I think the "critical" part of my "patch v1" is splitted
out to "2/2 v2" ... or?

> net library already gets the MAC address from the environment.  The
> correct flow is:
> 
> 1. Read from hardware in initialize() function

Ah, Ok, so that is the right way? If so, then I should remove

"+	if (!eth_getenv_enetaddr("ethaddr", ethaddr)) {"

from my patch, and then it should be OK, right?

Actual fec_mxc.c driver is *not* correct, because if in eeprom
is a correct mac, it *always* programms this in the mac address
registers from the chip!

This is not OK, and must be fixed!

> 2. Read from environment in net/eth.c after initialize()
> 3. Give priority to the value in the environment if a conflict
> 4. Program hardware in the device's init() function.
> 
> If somebody wants to subvert the 'design philosophy', the right way is
> to call eth_dev->init() in board code.

Maybe this list should go in a doc?

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

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Mike Frysinger]

On Wednesday 02 June 2010 21:57:42 Heiko Schocher wrote:
> Actual fec_mxc.c driver is *not* correct, because if in eeprom
> is a correct mac, it *always* programms this in the mac address
> registers from the chip!
> 
> This is not OK, and must be fixed!

i agree 100%

> > 2. Read from environment in net/eth.c after initialize()
> > 3. Give priority to the value in the environment if a conflict
> > 4. Program hardware in the device's init() function.
> > 
> > If somebody wants to subvert the 'design philosophy', the right way is
> > to call eth_dev->init() in board code.
> 
> Maybe this list should go in a doc?

the 1. - 4. is already in the documents ive mentioned multiple times, but they 
arent short & to the point like Ben has summarized, so that would probably be 
good to add as a summary and/or intro to one of them.

Ben's suggestion on how to "subvert" things by forcibly calling eth_dev-
>init() sits best in my book for people insisting on throwing in a hack today.  
it could even be done today in the board-specific board_eth_init() function by 
calling eth_init() after all the NICs have been registered.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100331/d520b6e8/attachment.pgp 

[U-Boot] [PATCH 1/2 v3] net, fec_mxc: only setup the device enetaddr with eeprom value

[Heiko Schocher]

only fill the device enetaddr with the contents of the eeprom,
not program it in mac address registers

Signed-off-by: Heiko Schocher <hs@denx.de>
---
- changes since v1 posted here:
  http://lists.denx.de/pipermail/u-boot/2010-March/069192.html

  - splitted in two patches as Wolfgang suggested

- changes since v2 posted here:

  http://lists.denx.de/pipermail/u-boot/2010-March/069244.html

  - add comment from Ben and Mike:
    - not to look in content from "ethaddr"
    - only read the mac from eeprom to ethdevices enetaddr and
      do not program it in mac address registers

 drivers/net/fec_mxc.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index 5af9cdb..8c4ade5 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -752,7 +752,6 @@ static int fec_probe(bd_t *bd)
 	if (fec_get_hwaddr(edev, ethaddr) == 0) {
 		printf("got MAC address from EEPROM: %pM\n", ethaddr);
 		memcpy(edev->enetaddr, ethaddr, 6);
-		fec_set_hwaddr(edev);
 	}

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

[U-Boot] [PATCH 1/2 v3] net, fec_mxc: only setup the device enetaddr with eeprom value

[Ben Warren]

Hi Heiko,

Much better.  Thanks, and sorry again for not responding sooner.

Wolfgang - please apply this to the current release.

On Tue, Mar 30, 2010 at 11:34 PM, Heiko Schocher <hs@denx.de> wrote:

> only fill the device enetaddr with the contents of the eeprom,
> not program it in mac address registers
>
> Signed-off-by: Heiko Schocher <hs@denx.de>

Acked-by: Ben Warren <biggerbadderben@gmail.com>

>

---
> - changes since v1 posted here:
>  http://lists.denx.de/pipermail/u-boot/2010-March/069192.html
>
>  - splitted in two patches as Wolfgang suggested
>
> - changes since v2 posted here:
>
>  http://lists.denx.de/pipermail/u-boot/2010-March/069244.html
>
>  - add comment from Ben and Mike:
>    - not to look in content from "ethaddr"
>    - only read the mac from eeprom to ethdevices enetaddr and
>      do not program it in mac address registers
>
>  drivers/net/fec_mxc.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index 5af9cdb..8c4ade5 100644
> --- a/drivers/net/fec_mxc.c
> +++ b/drivers/net/fec_mxc.c
> @@ -752,7 +752,6 @@ static int fec_probe(bd_t *bd)
>        if (fec_get_hwaddr(edev, ethaddr) == 0) {
>                printf("got MAC address from EEPROM: %pM\n", ethaddr);
>                memcpy(edev->enetaddr, ethaddr, 6);
> -               fec_set_hwaddr(edev);
>        }
>
>        return 0;
> --
> 1.6.2.5
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>

[U-Boot] [PATCH 1/2 v3] net, fec_mxc: only setup the device enetaddr with eeprom value

[Wolfgang Denk]

Dear Ben Warren,

In message <w2if8328f7c1003302350o154a5e96k25d7f8674657820@mail.gmail.com> you wrote:
> 
> Wolfgang - please apply this to the current release.
> 
> On Tue, Mar 30, 2010 at 11:34 PM, Heiko Schocher <hs@denx.de> wrote:
> 
> > only fill the device enetaddr with the contents of the eeprom,
> > not program it in mac address registers
> >
> > Signed-off-by: Heiko Schocher <hs@denx.de>
> 
> Acked-by: Ben Warren <biggerbadderben@gmail.com>

Done, thanks.

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
As in certain cults it is possible to kill a process if you know  its
true name.                      -- Ken Thompson and Dennis M. Ritchie

[U-Boot] [PATCH 1/2 v3] net, fec_mxc: only setup the device enetaddr with eeprom value

[Wolfgang Denk]

Dear Heiko Schocher,

In message <4BB2ED06.3010907@denx.de> you wrote:
> only fill the device enetaddr with the contents of the eeprom,
> not program it in mac address registers
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
> - changes since v1 posted here:
>   http://lists.denx.de/pipermail/u-boot/2010-March/069192.html
> 
>   - splitted in two patches as Wolfgang suggested
> 
> - changes since v2 posted here:
> 
>   http://lists.denx.de/pipermail/u-boot/2010-March/069244.html
> 
>   - add comment from Ben and Mike:
>     - not to look in content from "ethaddr"
>     - only read the mac from eeprom to ethdevices enetaddr and
>       do not program it in mac address registers
> 
>  drivers/net/fec_mxc.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)

Thanks. Applied after reverting v2.

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
An Elephant is a mouse with an Operating System.              - Knuth

[U-Boot] Toggling pins using the BDI3000

[Can Aydin]

Hi All,

I'm not sure if this is the place to ask this, please redirect if it isn't.

I'm looking into using a BDI3000 with a P2020RDB evaluation kit we've 
already purchased from Denx, but there is some confusion over how the 
board hardware handles the power on boot configuration signals to the 
processor. The addenda to the P2020RDB manuals provided by Freescale 
seem to indicate that certain boot functions that are supposed to be 
configurable using dipswitches on the board do not work as they should, 
notably certain clock speeds and the SPI and eSDHC boot configurations, 
which are the two options I am keen to exercise before moving on to 
prototyping.

Is a BDI3000, (using bdiGDB?), able to override the signals on these 
pins using JTAG functionality? Essentially, what I would like is some 
way of finding out if the P2020 is obtaining the correct PLL ratio and 
boot ROM location from the signals, and more importantly, override any 
erroneus signals that the hardware on the platform (P2020RDB and later, 
the prototype) might be providing (or failing to provide) it. (without 
using an oscilloscope and a soldering iron that is).

Regards,

Can Aydin
-- 

*Can Aydin*
Software Engineer
Locata Corporation
can.aydin at locatacorp.com

Locata Corporation Pty Ltd
Level 1, 111 Canberra Ave
Griffith ACT 2603 Australia
Phone +612 6126 5734
Fax +612 6126 5704

[U-Boot] [PATCH] net, doc: how to setup mac address correct.

[Heiko Schocher]

as this seems unclear, document how the flow of setting up
the mac address is correct.

Signed-off-by: Heiko Schocher <hs@denx.de>
---
 doc/README.enetaddr |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/doc/README.enetaddr b/doc/README.enetaddr
index 1d75aa3..9edb6cd 100644
--- a/doc/README.enetaddr
+++ b/doc/README.enetaddr
@@ -28,6 +28,18 @@ Here are the places where MAC addresses might be stored:
 	      purpose of passing this information to an OS kernel we are about
 	      to boot

+-----------
+ correct flow of setting up the mac address (summarized)
+-----------
+
+1. Read from hardware in initialize() function
+2. Read from environment in net/eth.c after initialize()
+3. Give priority to the value in the environment if a conflict
+4. Program hardware in the device's init() function.
+
+If somebody wants to subvert the 'design philosophy', the right way is to
+call eth_dev->init() in board code.
+
 -------
  Usage
 -------
-- 
1.6.2.5

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

[U-Boot] [PATCH] net, doc: how to setup mac address correct.

[Wolfgang Denk]

Dear Heiko Schocher,

In message <4BB2ED0B.6030406@denx.de> you wrote:
> as this seems unclear, document how the flow of setting up
> the mac address is correct.
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
>  doc/README.enetaddr |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)

Thanks. Applied (as discussed internally) with slight changes,
adding Mike Frysinger's input.

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
It would seem that evil retreats when forcibly confronted
	-- Yarnek of Excalbia, "The Savage Curtain", stardate 5906.5

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Detlev Zundel]

Hi Ben,

> Hold on a second.  This patch is wrong.  As Mike has pointed out, the 
> net library already gets the MAC address from the environment.  The 
> correct flow is:
>
> 1. Read from hardware in initialize() function
> 2. Read from environment in net/eth.c after initialize()
> 3. Give priority to the value in the environment if a conflict
> 4. Program hardware in the device's init() function.
>
> If somebody wants to subvert the 'design philosophy', the right way is 
> to call eth_dev->init() in board code.

This would mean that for the real problem of a missing mac address, the
device then is initialized completely adding the autonegotation timeout
to every bootup sequence, correct?

If it is, then it doesn't solve my problem in an acceptable way.  On the
other hand a different route occured to Wolfgang and me in a lengthy
discussion.  This will need a little broader interpretation of the
design guidelines, but as I still cannot see any downside to this and it
will also fix some inconsistent behaviour _that we currently have_
("setenv ethaddr ...", do not do any network transfer and boot linux), I
want to follow this route.

I will try to implement this in form of a patch in order to keep the
discussion close to the effects on the code base.

Cheers
  Detlev

-- 
#!/usr/bin/perl
$c="print\"\#\!\/usr\/bin\/perl\
\\\$c\=\\\"\"\.quotemeta\(\$c\)\.\"\\\"\;\\n\$c;\"";
print"#!/usr/bin/perl\n\$c=\"".quotemeta($c)."\";\n$c;";
--
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: dzu at denx.de

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Ben Warren]

Hi Detlev,

On Wed, Mar 31, 2010 at 6:44 AM, Detlev Zundel <dzu@denx.de> wrote:

> Hi Ben,
>
> > Hold on a second.  This patch is wrong.  As Mike has pointed out, the
> > net library already gets the MAC address from the environment.  The
> > correct flow is:
> >
> > 1. Read from hardware in initialize() function
> > 2. Read from environment in net/eth.c after initialize()
> > 3. Give priority to the value in the environment if a conflict
> > 4. Program hardware in the device's init() function.
> >
> > If somebody wants to subvert the 'design philosophy', the right way is
> > to call eth_dev->init() in board code.
>
> This would mean that for the real problem of a missing mac address, the
> device then is initialized completely adding the autonegotation timeout
> to every bootup sequence, correct?
>
> My suggestion here is a crude hack, and definitely does more than needed.
 It has the advantage of having narrow scope (is limited to the board in
question).

> If it is, then it doesn't solve my problem in an acceptable way.  On the
> other hand a different route occured to Wolfgang and me in a lengthy
> discussion.  This will need a little broader interpretation of the
> design guidelines, but as I still cannot see any downside to this and it
> will also fix some inconsistent behaviour _that we currently have_
> ("setenv ethaddr ...", do not do any network transfer and boot linux), I
> want to follow this route.
>
> I will try to implement this in form of a patch in order to keep the
> discussion close to the effects on the code base.
>
> I'm looking forward to seeing what you come up with.  I personally don't
have a problem with adding the few ns to boot-up time that programming an
SOC's MAC would take, but dislike the piece-meal approach that's been done
so far.

> Cheers
>  Detlev
>
> regards,
Ben

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Detlev Zundel]

Hi Ben,

> Hi Detlev,
>
> On Wed, Mar 31, 2010 at 6:44 AM, Detlev Zundel <dzu@denx.de> wrote:
>
>     Hi Ben,
>
>     > Hold on a second. ?This patch is wrong. ?As Mike has pointed out, the
>     > net library already gets the MAC address from the environment. ?The
>     > correct flow is:
>     >
>     > 1. Read from hardware in initialize() function
>     > 2. Read from environment in net/eth.c after initialize()
>     > 3. Give priority to the value in the environment if a conflict
>     > 4. Program hardware in the device's init() function.
>     >
>     > If somebody wants to subvert the 'design philosophy', the right way is
>     > to call eth_dev->init() in board code.
>
>     This would mean that for the real problem of a missing mac address, the
>     device then is initialized completely adding the autonegotation timeout
>     to every bootup sequence, correct?
>
>
> My suggestion here is a crude hack, and definitely does more than needed. ?It
> has the advantage of having narrow scope (is limited to the board in
> question).?

This specific problem in the meantime hit me on multiple arm boards with
different network interfaces so I think it has a broader audience than a
single board.

>     If it is, then it doesn't solve my problem in an acceptable way. ?On the
>     other hand a different route occured to Wolfgang and me in a lengthy
>     discussion. ?This will need a little broader interpretation of the
>     design guidelines, but as I still cannot see any downside to this and it
>     will also fix some inconsistent behaviour _that we currently have_
>     ("setenv ethaddr ...", do not do any network transfer and boot linux), I
>     want to follow this route.
>
>     I will try to implement this in form of a patch in order to keep the
>     discussion close to the effects on the code base.
>
>
> I'm looking forward to seeing what you come up with. ?I personally don't have a
> problem with adding the few ns to boot-up time that programming an SOC's MAC
> would take, but dislike the piece-meal approach that's been done so far.?

I fully agree.  Previously I was under the impression that we already
have a "fast initialization" (probe) and a "full initialization" (init)
of the network interfaces, where programming the mac would (on a first
stab) fit into the probe part (and some drivers obviously do/did this).

In the meantime it seems like it is a broader problem of keeping
"ethaddr" and friends in sync with the real hardware.  Although this is
something I personally always took for granted, it currently is most of
the time but not always correct.

If we solve the latter problem in a nice way, the initial problem will
simply disappear (or so I hope) ;)

Cheers
  Detlev

-- 
You live and learn
--
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: dzu at denx.de

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Mike Frysinger]

On Wednesday 31 March 2010 10:46:25 Detlev Zundel wrote:
> I fully agree.  Previously I was under the impression that we already
> have a "fast initialization" (probe) and a "full initialization" (init)
> of the network interfaces, where programming the mac would (on a first
> stab) fit into the probe part (and some drivers obviously do/did this).
> 
> In the meantime it seems like it is a broader problem of keeping
> "ethaddr" and friends in sync with the real hardware.  Although this is
> something I personally always took for granted, it currently is most of
> the time but not always correct.
> 
> If we solve the latter problem in a nice way, the initial problem will
> simply disappear (or so I hope) ;)

the latter problem is already solved in the general case.  the first 
initialize path gets the hw mac address and gives it to the common layers.  
common layers take care of keeping the structures in sync.  when the slow init 
path gets called to actually bring up the hardware, the mac has already been 
synced to the structures.  correctly written devices never have to care about 
anything other than dev->enetaddr and should not be programming the MAC with 
the MAC address in the fast initialize path.

really what you want is a new eth entry point like eth_write_mac() that goes 
through the common layers and programs the MAC with the MAC address.  this 
isnt a problem with the current net framework.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100331/9e141553/attachment.pgp 

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Ben Warren]

Hi Mike,

On 3/31/2010 12:59 PM, Mike Frysinger wrote:
> On Wednesday 31 March 2010 10:46:25 Detlev Zundel wrote:
>    
>> I fully agree.  Previously I was under the impression that we already
>> have a "fast initialization" (probe) and a "full initialization" (init)
>> of the network interfaces, where programming the mac would (on a first
>> stab) fit into the probe part (and some drivers obviously do/did this).
>>
>> In the meantime it seems like it is a broader problem of keeping
>> "ethaddr" and friends in sync with the real hardware.  Although this is
>> something I personally always took for granted, it currently is most of
>> the time but not always correct.
>>
>> If we solve the latter problem in a nice way, the initial problem will
>> simply disappear (or so I hope) ;)
>>      
> the latter problem is already solved in the general case.  the first
> initialize path gets the hw mac address and gives it to the common layers.
> common layers take care of keeping the structures in sync.  when the slow init
> path gets called to actually bring up the hardware, the mac has already been
> synced to the structures.  correctly written devices never have to care about
> anything other than dev->enetaddr and should not be programming the MAC with
> the MAC address in the fast initialize path.
>
> really what you want is a new eth entry point like eth_write_mac() that goes
> through the common layers and programs the MAC with the MAC address.  this
> isnt a problem with the current net framework.
>    
Detlev and I were just talking off-list about exactly that.  I guess 
'great minds think alike', as my grandma used to say.
> -mike
>    
regards,
Ben

[U-Boot] [PATCH 1/2 v2] net, fec_mxc: only setup the device enetaddr with eeprom value, if ethaddr is not setup

[Mike Frysinger]

thanks Heiko for the followups
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100331/df266e1d/attachment.pgp