[U-Boot] [PATCH 1/2] imx: Get fec mac address from fuse

The patch is to support getting FEC MAC address from fuse bank.
Signed-off-by: Jason Liu r64343@freescale.com --- arch/arm/include/asm/arch-mx25/imx-regs.h | 10 +++------- arch/arm/include/asm/arch-mx27/imx-regs.h | 5 ++--- arch/arm/include/asm/arch-mx5/imx-regs.h | 24 ++++++++++++++++++++++++ drivers/net/fec_mxc.c | 17 +++++------------ 4 files changed, 34 insertions(+), 22 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h b/arch/arm/include/asm/arch-mx25/imx-regs.h index f709bd8..35eb303 100644 --- a/arch/arm/include/asm/arch-mx25/imx-regs.h +++ b/arch/arm/include/asm/arch-mx25/imx-regs.h @@ -128,12 +128,8 @@ struct iim_regs { u32 iim_prev; u32 iim_srev; u32 iim_prog_p; - u32 res1[0x1f5]; - u32 iim_bank_area0[0x20]; - u32 res2[0xe0]; - u32 iim_bank_area1[0x20]; - u32 res3[0xe0]; - u32 iim_bank_area2[0x20]; + u32 res[0x1f5]; + u32 iim_bank_area[0x100 * 3]; }; #endif
@@ -311,6 +307,6 @@ struct iim_regs { #define WCR_WDE 0x04
/* FUSE bank offsets */ -#define IIM0_MAC 0x1a +#define IIM_MAC 0x1a
#endif /* _IMX_REGS_H */ diff --git a/arch/arm/include/asm/arch-mx27/imx-regs.h b/arch/arm/include/asm/arch-mx27/imx-regs.h index 6ecddaa..429f893 100644 --- a/arch/arm/include/asm/arch-mx27/imx-regs.h +++ b/arch/arm/include/asm/arch-mx27/imx-regs.h @@ -202,8 +202,7 @@ struct iim_regs { u32 iim_scs1; u32 iim_scs2; u32 iim_scs3; - u32 res[0x1F0]; - u32 iim_bank_area0[0x100]; + u32 iim_bank_area[0x100 * 2]; }; #endif
@@ -513,7 +512,7 @@ struct iim_regs { #define IIM_ERR_PARITYE (1 << 1)
/* Definitions for i.MX27 TO2 */ -#define IIM0_MAC 5 +#define IIM_MAC 5 #define IIM0_SCC_KEY 11 #define IIM1_SUID 1
diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h b/arch/arm/include/asm/arch-mx5/imx-regs.h index 3ddda40..892099c 100644 --- a/arch/arm/include/asm/arch-mx5/imx-regs.h +++ b/arch/arm/include/asm/arch-mx5/imx-regs.h @@ -204,6 +204,9 @@ #define BOARD_REV_1_0 0x0 #define BOARD_REV_2_0 0x1
+#define IMX_IIM_BASE (IIM_BASE_ADDR) +#define IIM_MAC 0x109 + #if !(defined(__KERNEL_STRICT_NAMES) || defined(__ASSEMBLY__)) #include <asm/types.h>
@@ -274,6 +277,27 @@ struct src { u32 sisr; u32 simr; }; + +struct iim_regs { + u32 stat; + u32 statm; + u32 err; + u32 emask; + u32 fctl; + u32 ua; + u32 la; + u32 sdat; + u32 prev; + u32 srev; + u32 preg_p; + u32 scs0; + u32 scs1; + u32 scs2; + u32 scs3; + u32 res[0x1f1]; + u32 iim_bank_area[0x100 * 4]; +}; + #endif /* __ASSEMBLER__*/
#endif /* __ASM_ARCH_MXC_MX51_H__ */ diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c index 3f09c2b..e8d7b98 100644 --- a/drivers/net/fec_mxc.c +++ b/drivers/net/fec_mxc.c @@ -312,21 +312,17 @@ static void fec_rbd_clean(int last, struct fec_bd *pRbd)
static int fec_get_hwaddr(struct eth_device *dev, unsigned char *mac) { -/* - * The MX27 can store the mac address in internal eeprom - * This mechanism is not supported now by MX51 or MX25 - */ -#if defined(CONFIG_MX51) || defined(CONFIG_MX25) - return -1; -#else + /* + * The MX27 can store the mac address in internal eeprom + * This mechanism is also supported now by MX51 or MX25 + */ struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE; int i;
for (i = 0; i < 6; i++) - mac[6-1-i] = readl(&iim->iim_bank_area0[IIM0_MAC + i]); + mac[6-1-i] = readl(&iim->iim_bank_area[IIM_MAC + i]);
return !is_valid_ether_addr(mac); -#endif }
static int fec_set_hwaddr(struct eth_device *dev) @@ -414,9 +410,6 @@ static int fec_init(struct eth_device *dev, bd_t* bd) uint32_t base; struct fec_priv *fec = (struct fec_priv *)dev->priv;
- /* Initialize MAC address */ - fec_set_hwaddr(dev); - /* * reserve memory for both buffer descriptor chains at once * Datasheet forces the startaddress of each chain is 16 byte

Fix the board version printing issue on MX51EVK. Need to read the board version via get_cpu_rev and not rely on system_rev due to the system_rev not initialized at boardchecking time.
Signed-off-by: Jason Liu r64343@freescale.com --- board/freescale/mx51evk/mx51evk.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/board/freescale/mx51evk/mx51evk.c b/board/freescale/mx51evk/mx51evk.c index d6bb71c..c532603 100644 --- a/board/freescale/mx51evk/mx51evk.c +++ b/board/freescale/mx51evk/mx51evk.c @@ -438,6 +438,8 @@ int board_late_init(void)
int checkboard(void) { + u32 system_rev = get_cpu_rev(); + puts("Board: MX51EVK ");
switch (system_rev & 0xff) {

On 10/22/2010 01:25 PM, Jason Liu wrote:
Fix the board version printing issue on MX51EVK. Need to read the board version via get_cpu_rev and not rely on system_rev due to the system_rev not initialized at boardchecking time.
Signed-off-by: Jason Liu r64343@freescale.com
board/freescale/mx51evk/mx51evk.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
Hi Jason,
diff --git a/board/freescale/mx51evk/mx51evk.c b/board/freescale/mx51evk/mx51evk.c index d6bb71c..c532603 100644 --- a/board/freescale/mx51evk/mx51evk.c +++ b/board/freescale/mx51evk/mx51evk.c @@ -438,6 +438,8 @@ int board_late_init(void)
int checkboard(void) {
u32 system_rev = get_cpu_rev();
puts("Board: MX51EVK ");
switch (system_rev & 0xff) {
Then we need to clean up other part of the code: system_rev should be not declared globally in the file and must be removed. In the same time, get_board_rev() should be changed. It seems it is in any case wrong, because it returns the same value, and this means get_cpu_rev().
As this is a cpu revision and not a board revision, it is not correct. If the board revision cannot be determined correctly at runtime, we should return a fixed value. but certainly not the cpu revision.
Best regards, Stefano Babic

Hi, Stefano,
2010/10/26 Stefano Babic sbabic@denx.de:
On 10/22/2010 01:25 PM, Jason Liu wrote:
Fix the board version printing issue on MX51EVK. Need to read the board version via get_cpu_rev and not rely on system_rev due to the system_rev not initialized at boardchecking time.
Signed-off-by: Jason Liu r64343@freescale.com
board/freescale/mx51evk/mx51evk.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
Hi Jason,
diff --git a/board/freescale/mx51evk/mx51evk.c b/board/freescale/mx51evk/mx51evk.c index d6bb71c..c532603 100644 --- a/board/freescale/mx51evk/mx51evk.c +++ b/board/freescale/mx51evk/mx51evk.c @@ -438,6 +438,8 @@ int board_late_init(void)
int checkboard(void) {
u32 system_rev = get_cpu_rev();
puts("Board: MX51EVK "); switch (system_rev & 0xff) {
Then we need to clean up other part of the code: system_rev should be not declared globally in the file and must be removed. In the same time, get_board_rev() should be changed. It seems it is in any case wrong, because it returns the same value, and this means get_cpu_rev().
As this is a cpu revision and not a board revision, it is not correct. If the board revision cannot be determined correctly at runtime, we should return a fixed value. but certainly not the cpu revision.
Yes, agree. Then we need clean up the code for vision2 board too.
Best regards, Stefano Babic
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On 10/26/2010 12:29 PM, Jason Liu wrote:
As this is a cpu revision and not a board revision, it is not correct. If the board revision cannot be determined correctly at runtime, we should return a fixed value. but certainly not the cpu revision.
Yes, agree. Then we need clean up the code for vision2 board too.
Of course. I will fix it in the vision2 board.
Stefano

On 10/22/2010 01:25 PM, Jason Liu wrote:
The patch is to support getting FEC MAC address from fuse bank.
Signed-off-by: Jason Liu r64343@freescale.com
Hi Jason,
patch is related to a network driver, so Ben should be informed, too.
- /*
* The MX27 can store the mac address in internal eeprom
* This mechanism is also supported now by MX51 or MX25
*/
If all supported processors implement the same way, we do not need to distinguish. The comment does not add any further info.
struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE; int i;
for (i = 0; i < 6; i++)
mac[6-1-i] = readl(&iim->iim_bank_area0[IIM0_MAC + i]);
mac[6-1-i] = readl(&iim->iim_bank_area[IIM_MAC + i]);
return !is_valid_ether_addr(mac);
-#endif
}
static int fec_set_hwaddr(struct eth_device *dev) @@ -414,9 +410,6 @@ static int fec_init(struct eth_device *dev, bd_t* bd) uint32_t base; struct fec_priv *fec = (struct fec_priv *)dev->priv;
- /* Initialize MAC address */
- fec_set_hwaddr(dev);
Not sure. Why is this call removed ? We have changed the storage for the MAC address, but we need always to set the FEC controller. What is the reason to drop it ?
Best regards, Stefano Babic

Hi, Stefano,
2010/10/26 Stefano Babic sbabic@denx.de:
On 10/22/2010 01:25 PM, Jason Liu wrote:
The patch is to support getting FEC MAC address from fuse bank.
Signed-off-by: Jason Liu r64343@freescale.com
Hi Jason,
patch is related to a network driver, so Ben should be informed, too.
OK, I will resend the patch and cc Ben. Thanks for reminder. :)
- /*
- * The MX27 can store the mac address in internal eeprom
- * This mechanism is also supported now by MX51 or MX25
- */
If all supported processors implement the same way, we do not need to distinguish. The comment does not add any further info.
OK, will remove it.
struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE; int i;
for (i = 0; i < 6; i++)
- mac[6-1-i] = readl(&iim->iim_bank_area0[IIM0_MAC + i]);
- mac[6-1-i] = readl(&iim->iim_bank_area[IIM_MAC + i]);
return !is_valid_ether_addr(mac); -#endif
}
static int fec_set_hwaddr(struct eth_device *dev) @@ -414,9 +410,6 @@ static int fec_init(struct eth_device *dev, bd_t* bd) uint32_t base; struct fec_priv *fec = (struct fec_priv *)dev->priv;
- /* Initialize MAC address */
- fec_set_hwaddr(dev);
Not sure. Why is this call removed ? We have changed the storage for the MAC address, but we need always to set the FEC controller. What is the reason to drop it ?
It's because, it will print some floating value of MAC address when bootup and in fact, we don't need re-set it again here.. The net framework will do that. This is why I remove. it.
Best regards, Stefano Babic
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On 10/26/2010 12:35 PM, Jason Liu wrote:
static int fec_set_hwaddr(struct eth_device *dev) @@ -414,9 +410,6 @@ static int fec_init(struct eth_device *dev, bd_t* bd) uint32_t base; struct fec_priv *fec = (struct fec_priv *)dev->priv;
/* Initialize MAC address */
fec_set_hwaddr(dev);
Not sure. Why is this call removed ? We have changed the storage for the MAC address, but we need always to set the FEC controller. What is the reason to drop it ?
It's because, it will print some floating value of MAC address when bootup and in fact, we don't need re-set it again here.. The net framework will do that. This is why I remove. it.
The call was added one month ago. It sets the MAC address any time the network is used, to be sure that the correct MAC is taken even if the ethaddr variable is changed. You can take a look at the patch:
http://www.mail-archive.com/u-boot@lists.denx.de/msg37629.html
Best regards, Stefano Babic

2010/10/27 Stefano Babic sbabic@denx.de:
On 10/26/2010 12:35 PM, Jason Liu wrote:
static int fec_set_hwaddr(struct eth_device *dev) @@ -414,9 +410,6 @@ static int fec_init(struct eth_device *dev, bd_t* bd) uint32_t base; struct fec_priv *fec = (struct fec_priv *)dev->priv;
- /* Initialize MAC address */
- fec_set_hwaddr(dev);
Not sure. Why is this call removed ? We have changed the storage for the MAC address, but we need always to set the FEC controller. What is the reason to drop it ?
It's because, it will print some floating value of MAC address when bootup and in fact, we don't need re-set it again here.. The net framework will do that. This is why I remove. it.
The call was added one month ago. It sets the MAC address any time the network is used, to be sure that the correct MAC is taken even if the ethaddr variable is changed. You can take a look at the patch:
http://www.mail-archive.com/u-boot@lists.denx.de/msg37629.html
Yes, I haved looked it before and I also tested this patch on MX51babbage 3.0 board. It will printting some confusing information with floating value of MAC address when the board boot up after you set the ethaddr enviorment. The reason is that, when it first call fec_init, the eth address of dev is not intialized and then it using the floating memory value.
Best regards, Stefano Babic
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================

On 10/27/2010 09:40 AM, Jason Liu wrote:
Yes, I haved looked it before and I also tested this patch on MX51babbage 3.0 board. It will printting some confusing information with floating value of MAC address when the board boot up after you set the ethaddr enviorment. The reason is that, when it first call fec_init, the eth address of dev is not intialized and then it using the floating memory value.
Dropping the call is not the solution and works only on the MX51 if the MAC is set into the fuse. Even on the MX51, this is not a general case: it should be always possible to set the ethaddr via the environment without using the fuse.
As sure, you should at least see a "Warning : MAC addresses don't match" if the ethaddr is set to a different value as in the fuse.
However, I do not yet understand why the address is not set: fec_probe is called before doing something on the network and at this point the address should be set from fuse.
Best regards, Stefano Babic

Hi, Stefano,
2010/10/27 Stefano Babic sbabic@denx.de:
On 10/27/2010 09:40 AM, Jason Liu wrote:
Yes, I haved looked it before and I also tested this patch on MX51babbage 3.0 board. It will printting some confusing information with floating value of MAC address when the board boot up after you set the ethaddr enviorment. The reason is that, when it first call fec_init, the eth address of dev is not intialized and then it using the floating memory value.
Dropping the call is not the solution and works only on the MX51 if the MAC is set into the fuse. Even on the MX51, this is not a general case:
FSL will program all the fuse with MAC address before shipping every chip. This applies to all the i.mx family including imx25/35/51/53/50.
it should be always possible to set the ethaddr via the environment without using the fuse.
Yes, without call this function, the ethaddr evnviroment still working. It will use the MAC set via ethaddr instead of the mac from FUSE. Net framework will do it.
As sure, you should at least see a "Warning : MAC addresses don't match" if the ethaddr is set to a different value as in the fuse.
However, I do not yet understand why the address is not set: fec_probe is called before doing something on the network and at this point the address should be set from fuse.
I have tested it and it's no need to add this function call. You can test it and see the result.
Best regards, Stefano Babic
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de =====================================================================

On Wed, Oct 27, 2010 at 5:21 AM, Jason Liu liu.h.jason@gmail.com wrote:
Hi, Stefano,
2010/10/27 Stefano Babic sbabic@denx.de:
On 10/27/2010 09:40 AM, Jason Liu wrote:
Yes, I haved looked it before and I also tested this patch on MX51babbage 3.0 board. It will printting some confusing information with floating value of MAC address when the board boot up after you set the ethaddr enviorment. The reason is that, when it first call fec_init, the eth address of dev is not intialized and then it using the floating memory value.
Dropping the call is not the solution and works only on the MX51 if the MAC is set into the fuse. Even on the MX51, this is not a general case:
FSL will program all the fuse with MAC address before shipping every chip. This applies to all the i.mx family including imx25/35/51/53/50.
it should be always possible to set the ethaddr via the environment without using the fuse.
Yes, without call this function, the ethaddr evnviroment still working. It will use the MAC set via ethaddr instead of the mac from FUSE. Net framework will do it.
As sure, you should at least see a "Warning : MAC addresses don't match" if the ethaddr is set to a different value as in the fuse.
However, I do not yet understand why the address is not set: fec_probe is called before doing something on the network and at this point the address should be set from fuse.
I have tested it and it's no need to add this function call. You can test it and see the result.
Best regards, Stefano Babic
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de ====================================================================
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
The patch fixes the particular case where you have no ethaddr in fuse, nor persistent env. The case is a board where the only persistent storage is an SD card and you set load a boot script from file in a FAT partition that has a setenv ethaddr in it. The other case is if a user sets the ethaddr at the command line it will not take effect until reboot in the case of persistent env or never in the case of no persistent env.

Dear John,
In message AANLkTikMZvAsVW1WtwG7od9QYeaPHHWdBC+3OX5Akv3L@mail.gmail.com you wrote:
The patch fixes the particular case where you have no ethaddr in fuse, nor persistent env. The case is a board where the only persistent storage is an SD card and you set load a boot script from file in a
Well, if ther eis no MAC address in the fuse, and there is no "ethaddr" setting in the current environment, then nothing should happen until you try to run the first network command, at wich point the command should fail because the MAC address is unset.
FAT partition that has a setenv ethaddr in it. The other case is if a user sets the ethaddr at the command line it will not take effect until reboot in the case of persistent env or never in the case of no persistent env.
This is broken behaviour; in this case, the network driver needs to be fixed. The driver initialization takes place when you start a network command, and it should pick up the MAC address setting then, mo matter if this was permanently stored or just entered on the command line.
Best regards,
Wolfgang Denk

On Wed, Oct 27, 2010 at 2:06 PM, Wolfgang Denk wd@denx.de wrote:
Dear John,
In message AANLkTikMZvAsVW1WtwG7od9QYeaPHHWdBC+3OX5Akv3L@mail.gmail.com you wrote:
The patch fixes the particular case where you have no ethaddr in fuse, nor persistent env. The case is a board where the only persistent storage is an SD card and you set load a boot script from file in a
Well, if ther eis no MAC address in the fuse, and there is no "ethaddr" setting in the current environment, then nothing should happen until you try to run the first network command, at wich point the command should fail because the MAC address is unset.
FAT partition that has a setenv ethaddr in it. The other case is if a user sets the ethaddr at the command line it will not take effect until reboot in the case of persistent env or never in the case of no persistent env.
This is broken behaviour; in this case, the network driver needs to be fixed. The driver initialization takes place when you start a network command, and it should pick up the MAC address setting then, mo matter if this was permanently stored or just entered on the command line.
Thanks for saying this because this is exactly what my original patch fixes. So the fix should stay.
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@denx.de "A child is a person who can't understand why someone would give away a perfectly good kitten." - Doug Larson
participants (5)
-
Jason Liu
-
Jason Liu
-
John Rigby
-
Stefano Babic
-
Wolfgang Denk