diff mbox

[U-Boot] net: write enetaddr down to hardware on env_callback

Message ID 1472820497-4362-1-git-send-email-oe5hpm@oevsv.at
State Accepted
Commit c86ff7fdb2272e25c8abba2e36d4f28f9282cd37
Delegated to: Joe Hershberger
Headers show

Commit Message

Hannes Schmelzer Sept. 2, 2016, 12:48 p.m. UTC
If mac-address is changed using "setenv ethaddr ...." command the new
mac-adress also must be written into the responsible ethernet driver.

Signed-off-by: Hannes Schmelzer <oe5hpm@oevsv.at>

---

 net/eth-uclass.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Joe Hershberger Sept. 2, 2016, 1 p.m. UTC | #1
On Fri, Sep 2, 2016 at 7:48 AM, Hannes Schmelzer <oe5hpm@oevsv.at> wrote:
> If mac-address is changed using "setenv ethaddr ...." command the new
> mac-adress also must be written into the responsible ethernet driver.
>
> Signed-off-by: Hannes Schmelzer <oe5hpm@oevsv.at>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>
Bin Meng Sept. 6, 2016, 1:54 a.m. UTC | #2
Hi,

On Fri, Sep 2, 2016 at 9:00 PM, Joe Hershberger
<joe.hershberger@gmail.com> wrote:
> On Fri, Sep 2, 2016 at 7:48 AM, Hannes Schmelzer <oe5hpm@oevsv.at> wrote:
>> If mac-address is changed using "setenv ethaddr ...." command the new
>> mac-adress also must be written into the responsible ethernet driver.
>>
>> Signed-off-by: Hannes Schmelzer <oe5hpm@oevsv.at>
>
> Acked-by: Joe Hershberger <joe.hershberger@ni.com>

Why is this needed? The MAC address is supposed to be programmed in
the driver's probe routine.

Regards,
Bin
Hannes Schmelzer Sept. 6, 2016, 4:33 a.m. UTC | #3
"U-Boot" <u-boot-bounces@lists.denx.de> schrieb am 06.09.2016 03:54:52:

> Von: Bin Meng <bmeng.cn@gmail.com>
> An: Joe Hershberger <joe.hershberger@gmail.com>, 
> Kopie: u-boot <u-boot@lists.denx.de>, Hannes Schmelzer 
<oe5hpm@oevsv.at>, Joe 
> Hershberger <joe.hershberger@ni.com>
> Datum: 06.09.2016 03:57
> Betreff: Re: [U-Boot] [PATCH] net: write enetaddr down to hardware on 
env_callback
> Gesendet von: "U-Boot" <u-boot-bounces@lists.denx.de>
> 
> Hi,
Hi Bin,

> 
> On Fri, Sep 2, 2016 at 9:00 PM, Joe Hershberger
> <joe.hershberger@gmail.com> wrote:
> > On Fri, Sep 2, 2016 at 7:48 AM, Hannes Schmelzer <oe5hpm@oevsv.at> 
wrote:
> >> If mac-address is changed using "setenv ethaddr ...." command the new
> >> mac-adress also must be written into the responsible ethernet driver.
> >>
> >> Signed-off-by: Hannes Schmelzer <oe5hpm@oevsv.at>
> >
> > Acked-by: Joe Hershberger <joe.hershberger@ni.com>
> 
> Why is this needed? The MAC address is supposed to be programmed in
> the driver's probe routine.

For example on my custom ZYNQ board.
The Ethernetcontroller within the ZYNQ has no ROM to store his own 
MAC-Address, further no MAC is stored within environment.
So Ethernet gets probed with a "random MAC-Address" if configured.

Later somebody (like me) oder something (my bootscript) runs "setenv 
ethaddr ...." on the console.
Result is (was before this patch), that all networking infrastructre is 
running this new mac, but hardware is running the old (random) one and 
networking is simple not functional.

ok?

> 
> Regards,
> Bin
cheers,
Hannes
Bin Meng Sept. 6, 2016, 7:07 a.m. UTC | #4
Hi Hannes,

On Tue, Sep 6, 2016 at 12:33 PM, Hannes Schmelzer
<Hannes.Schmelzer@br-automation.com> wrote:
> "U-Boot" <u-boot-bounces@lists.denx.de> schrieb am 06.09.2016 03:54:52:
>
>> Von: Bin Meng <bmeng.cn@gmail.com>
>> An: Joe Hershberger <joe.hershberger@gmail.com>,
>> Kopie: u-boot <u-boot@lists.denx.de>, Hannes Schmelzer
> <oe5hpm@oevsv.at>, Joe
>> Hershberger <joe.hershberger@ni.com>
>> Datum: 06.09.2016 03:57
>> Betreff: Re: [U-Boot] [PATCH] net: write enetaddr down to hardware on
> env_callback
>> Gesendet von: "U-Boot" <u-boot-bounces@lists.denx.de>
>>
>> Hi,
> Hi Bin,
>
>>
>> On Fri, Sep 2, 2016 at 9:00 PM, Joe Hershberger
>> <joe.hershberger@gmail.com> wrote:
>> > On Fri, Sep 2, 2016 at 7:48 AM, Hannes Schmelzer <oe5hpm@oevsv.at>
> wrote:
>> >> If mac-address is changed using "setenv ethaddr ...." command the new
>> >> mac-adress also must be written into the responsible ethernet driver.
>> >>
>> >> Signed-off-by: Hannes Schmelzer <oe5hpm@oevsv.at>
>> >
>> > Acked-by: Joe Hershberger <joe.hershberger@ni.com>
>>
>> Why is this needed? The MAC address is supposed to be programmed in
>> the driver's probe routine.
>
> For example on my custom ZYNQ board.
> The Ethernetcontroller within the ZYNQ has no ROM to store his own
> MAC-Address, further no MAC is stored within environment.
> So Ethernet gets probed with a "random MAC-Address" if configured.
>
> Later somebody (like me) oder something (my bootscript) runs "setenv
> ethaddr ...." on the console.
> Result is (was before this patch), that all networking infrastructre is
> running this new mac, but hardware is running the old (random) one and
> networking is simple not functional.
>

This indicates that your ethernet driver does not program the latest
MAC address everyone when it gets probed. The driver should be fixed.

> ok?
>

Regards,
Bin
Hannes Schmelzer Sept. 6, 2016, 7:23 a.m. UTC | #5
Bin Meng <bmeng.cn@gmail.com> schrieb am 06.09.2016 09:07:39:

[...]
> >>
> >> Why is this needed? The MAC address is supposed to be programmed in
> >> the driver's probe routine.
> >
> > For example on my custom ZYNQ board.
> > The Ethernetcontroller within the ZYNQ has no ROM to store his own
> > MAC-Address, further no MAC is stored within environment.
> > So Ethernet gets probed with a "random MAC-Address" if configured.
> >
> > Later somebody (like me) oder something (my bootscript) runs "setenv
> > ethaddr ...." on the console.
> > Result is (was before this patch), that all networking infrastructre 
is
> > running this new mac, but hardware is running the old (random) one and
> > networking is simple not functional.
> >
> 
> This indicates that your ethernet driver does not program the latest
> MAC address everyone when it gets probed. The driver should be fixed.

The driver doesn't get probed again if mac-address is changed using 
"setenv ethaddr",
so it has no chance to program anything.

For program i understand, in this case with zynq, overtaking mac-address 
into his registers.
There is per default no ROM to store some mac.

> 
> Regards,
> Bin
cheers,
Hannes
Bin Meng Sept. 6, 2016, 7:28 a.m. UTC | #6
Hi Hannes,

On Tue, Sep 6, 2016 at 3:23 PM, Hannes Schmelzer
<Hannes.Schmelzer@br-automation.com> wrote:
> Bin Meng <bmeng.cn@gmail.com> schrieb am 06.09.2016 09:07:39:
>
> [...]
>> >>
>> >> Why is this needed? The MAC address is supposed to be programmed in
>> >> the driver's probe routine.
>> >
>> > For example on my custom ZYNQ board.
>> > The Ethernetcontroller within the ZYNQ has no ROM to store his own
>> > MAC-Address, further no MAC is stored within environment.
>> > So Ethernet gets probed with a "random MAC-Address" if configured.
>> >
>> > Later somebody (like me) oder something (my bootscript) runs "setenv
>> > ethaddr ...." on the console.
>> > Result is (was before this patch), that all networking infrastructre
> is
>> > running this new mac, but hardware is running the old (random) one and
>> > networking is simple not functional.
>> >
>>
>> This indicates that your ethernet driver does not program the latest
>> MAC address everyone when it gets probed. The driver should be fixed.
>
> The driver doesn't get probed again if mac-address is changed using
> "setenv ethaddr",
> so it has no chance to program anything.
>

Yes, driver is not probed when "setenv ethaddr", but next time when
you type any ethernet related command (like tftpboot), the driver will
be probed.

> For program i understand, in this case with zynq, overtaking mac-address
> into his registers.
> There is per default no ROM to store some mac.
>

I have feeling that for some ethernet controllers, simply calling
eth_write_hwaddr() may not change its MAC address correctly. It may
need some special programming sequence like firstly turn off, reset,
then program the MAC.

Regards,
Bin
Hannes Schmelzer Sept. 6, 2016, 7:56 a.m. UTC | #7
Bin Meng <bmeng.cn@gmail.com> schrieb am 06.09.2016 09:28:13:

> Von: Bin Meng <bmeng.cn@gmail.com>
> An: Hannes Schmelzer <Hannes.Schmelzer@br-automation.com>, 
> Kopie: Joe Hershberger <joe.hershberger@gmail.com>, Joe Hershberger 
> <joe.hershberger@ni.com>, Hannes Schmelzer <oe5hpm@oevsv.at>, u-boot <u-
> boot@lists.denx.de>, U-Boot <u-boot-bounces@lists.denx.de>
> Datum: 06.09.2016 09:28
> Betreff: Re: Re: Re: [U-Boot] [PATCH] net: write enetaddr down to 
hardware on 
> env_callback
> 
> Hi Hannes,
> 
> On Tue, Sep 6, 2016 at 3:23 PM, Hannes Schmelzer
> <Hannes.Schmelzer@br-automation.com> wrote:
> > Bin Meng <bmeng.cn@gmail.com> schrieb am 06.09.2016 09:07:39:
> >
> > [...]
> >> >>
> >> >> Why is this needed? The MAC address is supposed to be programmed 
in
> >> >> the driver's probe routine.
> >> >
> >> > For example on my custom ZYNQ board.
> >> > The Ethernetcontroller within the ZYNQ has no ROM to store his own
> >> > MAC-Address, further no MAC is stored within environment.
> >> > So Ethernet gets probed with a "random MAC-Address" if configured.
> >> >
> >> > Later somebody (like me) oder something (my bootscript) runs 
"setenv
> >> > ethaddr ...." on the console.
> >> > Result is (was before this patch), that all networking 
infrastructre
> > is
> >> > running this new mac, but hardware is running the old (random) one 
and
> >> > networking is simple not functional.
> >> >
> >>
> >> This indicates that your ethernet driver does not program the latest
> >> MAC address everyone when it gets probed. The driver should be fixed.
> >
> > The driver doesn't get probed again if mac-address is changed using
> > "setenv ethaddr",
> > so it has no chance to program anything.
> >
> 
> Yes, driver is not probed when "setenv ethaddr", but next time when
> you type any ethernet related command (like tftpboot), the driver will
> be probed.
No. The ethernet driver isn't probed everytime a network transaction is 
coming.
What i can see, the probe is done one time.

> 
> > For program i understand, in this case with zynq, overtaking 
mac-address
> > into his registers.
> > There is per default no ROM to store some mac.
> >
> 
> I have feeling that for some ethernet controllers, simply calling
> eth_write_hwaddr() may not change its MAC address correctly. It may
> need some special programming sequence like firstly turn off, reset,
> then program the MAC.
That might be possible. Maybe this is the reason why not all drivers 
provide this function.

> 
> Regards,
> Bin
Joe Hershberger Oct. 13, 2016, 5:38 p.m. UTC | #8
Hi Hannes,

https://patchwork.ozlabs.org/patch/665202/ was applied to u-boot-net.git.

Thanks!
-Joe
diff mbox

Patch

diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index c15cc4d..a32961e 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -227,6 +227,7 @@  static int on_ethaddr(const char *name, const char *value, enum env_op op,
 		case env_op_create:
 		case env_op_overwrite:
 			eth_parse_enetaddr(value, pdata->enetaddr);
+			eth_write_hwaddr(dev);
 			break;
 		case env_op_delete:
 			memset(pdata->enetaddr, 0, 6);