arm: mxs: fix mac address of second interface
Subsystems (2)
PATCH 1/1 arm: mxs: fix mac address of second interface
In the rare case that an overflow occurs, propagate it.
--- arch/arm/cpu/arm926ejs/mxs/mxs.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
arch/arm/cpu/arm926ejs/mxs/mxs.c +10 -2
@@ -238,11 +238,19 @@ int cpu_eth_init(bd_t *bis)__weak void mx28_adjust_mac (int dev_id,unsigned char *mac) { +uint32_t data; + mac[0 ] =0x00 ; mac[1 ] =0x04 ;/* Use FSL vendor MAC address by default */ -if (dev_id ==1 )/* Let MAC1 be MAC0 + 1 by default */ - mac[5 ] +=1 ; +if (dev_id ==1 ) {/* Let MAC1 be MAC0 + 1 by default */ + data = (mac[3 ] <<16 ) | (mac[4 ] <<8 ) | mac[5 ]; + data++; + + mac[3 ] = (data >>16 ) &0xff ; + mac[4 ] = (data >>8 ) &0xff ; + mac[5 ] = data &0xff ; + } }#ifdef CONFIG_MX28_FEC_MAC_IN_OCOTP
Signature · 1 line
1.7.10.4
PATCH 1/1 arm: mxs: fix mac address of second interface
Quoted 1 line · Michael Heimpold
In the rare case that an overflow occurs, propagate it.
Hi!
Quoted 21 lines
Signed-off-by Michael Heimpold <[email protected]>Cc Stefano Babic <[email protected]>Cc Marek Vasut <[email protected]>CC Fabio Estevam <[email protected]>--- arch/arm/cpu/arm926ejs/mxs/mxs.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)(quoted diff) +0
-0
b/arch/arm/cpu/arm926ejs/mxs/mxs.cindex b1d8721..42057ad 100644arch/arm/cpu/arm926ejs/mxs/mxs.c +2
-0@@ -238,11 +238,19 @@ int cpu_eth_init(bd_t *bis)__weak void mx28_adjust_mac (int dev_id,unsigned char *mac) { +uint32_t data; + mac[0 ] =0x00 ; mac[1 ] =0x04 ;/* Use FSL vendor MAC address by default */
arch/arm/cpu/arm926ejs/mxs/mxs.c int cpu_eth_init(bd_t *bis) · line 253
Quoted 10 lines
arch/arm/cpu/arm926ejs/mxs/mxs.c +8
-2-if (dev_id ==1 )/* Let MAC1 be MAC0 + 1 by default */ - mac[5 ] +=1 ; +if (dev_id ==1 ) {/* Let MAC1 be MAC0 + 1 by default */ + data = (mac[3 ] <<16 ) | (mac[4 ] <<8 ) | mac[5 ]; + data++; + + mac[3 ] = (data >>16 ) &0xff ; + mac[4 ] = (data >>8 ) &0xff ; + mac[5 ] = data &0xff ; + }
I'm not very fond of the added complexity. If an overflow happens, the last nibble just becomes 0x00, which should be okayish. In case you need some sort of special handling of the ethernet address, I'd suggest to implement your own thing in board file.
Best regards,
Marek Vasut
PATCH 1/1 arm: mxs: fix mac address of second interface
Hi Marek,
Am Sonntag, 16. August 2015, 00:49:36 schrieb Marek Vasut:
Quoted 26 lines · Michael Heimpold
Quoted 1 line
In the rare case that an overflow occurs, propagate it.
Hi!
Quoted 21 lines
Signed-off-by Michael Heimpold <[email protected]>Cc Stefano Babic <[email protected]>Cc Marek Vasut <[email protected]>CC Fabio Estevam <[email protected]>--- arch/arm/cpu/arm926ejs/mxs/mxs.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)(quoted diff) +0
-0
b/arch/arm/cpu/arm926ejs/mxs/mxs.cindex b1d8721..42057ad 100644arch/arm/cpu/arm926ejs/mxs/mxs.c +2
-0@@ -238,11 +238,19 @@ int cpu_eth_init(bd_t *bis)__weak void mx28_adjust_mac (int dev_id,unsigned char *mac) { +uint32_t data; + mac[0 ] =0x00 ; mac[1 ] =0x04 ;/* Use FSL vendor MAC address by default */
Quoted 12 lines
arch/arm/cpu/arm926ejs/mxs/mxs.c int cpu_eth_init(bd_t *bis) · line 253
Quoted 10 lines
arch/arm/cpu/arm926ejs/mxs/mxs.c +8
-2-if (dev_id ==1 )/* Let MAC1 be MAC0 + 1 by default */ - mac[5 ] +=1 ; +if (dev_id ==1 ) {/* Let MAC1 be MAC0 + 1 by default */ + data = (mac[3 ] <<16 ) | (mac[4 ] <<8 ) | mac[5 ]; + data++; + + mac[3 ] = (data >>16 ) &0xff ; + mac[4 ] = (data >>8 ) &0xff ; + mac[5 ] = data &0xff ; + }I'm not very fond of the added complexity. If an overflow happens,
At least for me, that would be the "algorithm of least surprise" and I don't feel that this is so complex but YMMV :-)
Quoted 3 lines
the last nibble just becomes
0x00, which should be okayish. In case you need some sort of special handling of the ethernet address, I'd suggest to implement your own thing in board file.
If this is a NAK, then yes, I can handle this in every board file where needed because I've to adjust the vendor part anyway...
Quoted 3 lines
Best regards,
Marek Vasut
Best regards,
Michael
PATCH 1/1 arm: mxs: fix mac address of second interface
Quoted 1 line · Michael Heimpold
Hi Marek,
Hi!
Quoted 46 lines
Am Sonntag, 16. August 2015, 00:49:36 schrieb Marek Vasut:
Quoted 29 lines · Michael Heimpold
Quoted 1 line
In the rare case that an overflow occurs, propagate it.
Hi!
Quoted 24 lines
Signed-off-by Michael Heimpold <[email protected]>Cc Stefano Babic <[email protected]>Cc Marek Vasut <[email protected]>CC Fabio Estevam <[email protected]>---
arch/arm/cpu/arm926ejs/mxs/mxs.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)(quoted diff) +0
-0
b/arch/arm/cpu/arm926ejs/mxs/mxs.cindex b1d8721..42057ad 100644arch/arm/cpu/arm926ejs/mxs/mxs.c +2
-0@@ -238,11 +238,19 @@ int cpu_eth_init(bd_t *bis)__weak void mx28_adjust_mac (int dev_id,unsigned char *mac) { +uint32_t data; + mac[0 ] =0x00 ; mac[1 ] =0x04 ;/* Use FSL vendor MAC address by default */ Quoted 12 lines
arch/arm/cpu/arm926ejs/mxs/mxs.c int cpu_eth_init(bd_t *bis) · line 253
Quoted 10 lines
arch/arm/cpu/arm926ejs/mxs/mxs.c +8
-2-if (dev_id ==1 )/* Let MAC1 be MAC0 + 1 by default */ - mac[5 ] +=1 ; +if (dev_id ==1 ) {/* Let MAC1 be MAC0 + 1 by default */ + data = (mac[3 ] <<16 ) | (mac[4 ] <<8 ) | mac[5 ]; + data++; + + mac[3 ] = (data >>16 ) &0xff ; + mac[4 ] = (data >>8 ) &0xff ; + mac[5 ] = data &0xff ; + }I'm not very fond of the added complexity. If an overflow happens,
At least for me, that would be the "algorithm of least surprise" and I don't feel that this is so complex but YMMV :-)
Quoted 6 lines
Quoted 3 lines
the last nibble just becomes
0x00, which should be okayish. In case you need some sort of special handling of the ethernet address, I'd suggest to implement your own thing in board file.If this is a NAK, then yes, I can handle this in every board file where needed because I've to adjust the vendor part anyway...
Let's hear from the others first.
Best regards,
Marek Vasut
PATCH 1/1 arm: mxs: fix mac address of second interface
In reply to Michael HeimpoldHi Michael,
Quoted 25 lines · Michael Heimpold
In the rare case that an overflow occurs, propagate it.
Signed-off-by Michael Heimpold <[email protected]>Cc Stefano Babic <[email protected]>Cc Marek Vasut <[email protected]>CC Fabio Estevam <[email protected]>--- arch/arm/cpu/arm926ejs/mxs/mxs.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)arch/arm/cpu/arm926ejs/mxs/mxs.c +2
-2@@ -238,11 +238,19 @@ int cpu_eth_init(bd_t *bis)__weak void mx28_adjust_mac (int dev_id,unsigned char *mac) { +uint32_t data; + mac[0 ] =0x00 ; mac[1 ] =0x04 ;/* Use FSL vendor MAC address by default */ -if (dev_id ==1 )/* Let MAC1 be MAC0 + 1 by default */ - mac[5 ] +=1 ;
arch/arm/cpu/arm926ejs/mxs/mxs.c int cpu_eth_init(bd_t *bis) · line 255
Quoted 10 lines
arch/arm/cpu/arm926ejs/mxs/mxs.c +8
-0+if (dev_id ==1 ) {/* Let MAC1 be MAC0 + 1 by default */ + data = (mac[3 ] <<16 ) | (mac[4 ] <<8 ) | mac[5 ]; + data++; + + mac[3 ] = (data >>16 ) &0xff ; + mac[4 ] = (data >>8 ) &0xff ; + mac[5 ] = data &0xff ; + } }
Even if on a theoretical point of view this is correct, I guess there is something fundamentally wrong.
We cannot set any MAC address we want. MAC addresses must be bought, even by the manufacturer (Freescale) if it wants to deliver SOC with preinstalled MAC addresses. In most cases but not all, the user of the SOC wants to have his Vendor-id, and he buys a range of addresses. If the target has two ethernet, he will reserve two addresses for each board and it is common practise to have two consecutive addresses, without any roll up - this is avoided by the logistic assigning a even number of addresses to the board.
The case you want to fix should never happen - not due to the code, but how addresses are managed in practise. And delivering a board with two very different addresses, as in case of an overflow, can be also seen weird by the customer.
Is this a real case ?
Best regards,
Stefano Babic
Signature · 5 lines
===================================================================== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de =====================================================================
PATCH 1/1 arm: mxs: fix mac address of second interface
Hi Stefano,
Am Sonntag, 23. August 2015, 17:38:05 schrieb Stefano Babic:
Quoted 42 lines
Hi Michael,
Quoted 25 lines · Michael Heimpold
In the rare case that an overflow occurs, propagate it.
Signed-off-by Michael Heimpold <[email protected]>Cc Stefano Babic <[email protected]>Cc Marek Vasut <[email protected]>CC Fabio Estevam <[email protected]>--- arch/arm/cpu/arm926ejs/mxs/mxs.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)arch/arm/cpu/arm926ejs/mxs/mxs.c +2
-2@@ -238,11 +238,19 @@ int cpu_eth_init(bd_t *bis)__weak void mx28_adjust_mac (int dev_id,unsigned char *mac) { +uint32_t data; + mac[0 ] =0x00 ; mac[1 ] =0x04 ;/* Use FSL vendor MAC address by default */ -if (dev_id ==1 )/* Let MAC1 be MAC0 + 1 by default */ - mac[5 ] +=1 ;arch/arm/cpu/arm926ejs/mxs/mxs.c int cpu_eth_init(bd_t *bis) · line 255
Quoted 10 lines
arch/arm/cpu/arm926ejs/mxs/mxs.c +8
-0+if (dev_id ==1 ) {/* Let MAC1 be MAC0 + 1 by default */ + data = (mac[3 ] <<16 ) | (mac[4 ] <<8 ) | mac[5 ]; + data++; + + mac[3 ] = (data >>16 ) &0xff ; + mac[4 ] = (data >>8 ) &0xff ; + mac[5 ] = data &0xff ; + } }Even if on a theoretical point of view this is correct, I guess there is something fundamentally wrong.
Quoted 9 lines
We cannot set any MAC address we want. MAC addresses must be bought, even by the manufacturer (Freescale) if it wants to deliver SOC with preinstalled MAC addresses. In most cases but not all, the user of the SOC wants to have his Vendor-id, and he buys a range of addresses. If the target has two ethernet, he will reserve two addresses for each board and it is common practise to have two consecutive addresses, without any roll up - this is avoided by the logistic assigning a even number of addresses to the board.
The simplest approach for assigning MAC addresses to a board is IMO that the board gets the first free one or -when the board has two ethernets- the first two free (consecutive) addresses of the bought address pool during manufacturing process. Without any reservation/pre-allocation, simply first come first served.
Thus there is no guarantee in case of a two-ethernet-board that the first address is an even one, because you don't know the order of manufacturing of products which require only one and/or which require two addresses. Using this very simple algorithm does not prevent such an overflow and when the board/devices has a label with the two printed addresses the second one does not match. So this is/was my real case.
I totally agree that there might be smarter algorithms and/or better best practises to assign MAC addresses; and I really don't know, how FSL/Denx/... handle this in practise.
I can only tell for the company I'm working for or I worked for...
Quoted 4 lines
The case you want to fix should never happen - not due to the code, but how addresses are managed in practise. And delivering a board with two very different addresses, as in case of an overflow, can be also seen weird by the customer.
Maybe. More important to the customer would be that in case of rollover the second address might be already used (legally) by another device (at least when the manufacturing process is not as smart as you assumed).
Yes, I know that this is even more theoretical that exactly this customer has exactly this board in exactly this very same network :-)
Quoted 6 lines
Is this a real case ?
Best regards,
Stefano Babic
Best regards,
Michael Heimpold