Re: [U-Boot] [PATCH v5 1/1] imx: Get fec mac address from fuse
On 11/18/2010 11:33 AM, Jason Liu wrote: >> >>> + for (i = 0; i < 6; i++) >>> + mac[6-1-i] = readl(&fuse->mac_addr[i]); >> ^ >> |--- missing spaces > > do you mean it need change to mac[6 - 1 - i] ? Yes, this is a coding style issue. > I think that the original mx27 code should be wrong. I come with the same conclusion. 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: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 1/1] imx: Get fec mac address from fuse
Hi, Stefano,
2010/11/18 Stefano Babic :
> On 11/18/2010 09:09 AM, Jason Liu wrote:
>> The patch is to support getting FEC MAC address from fuse bank.
>>
>> Signed-off-by: Jason Liu
>
> Hi Jason,
>
>> + for (i = 0; i < 6; i++)
>> + mac[i] = readl(&fuse->mac_addr[i]);
>
> This works, but implicitely converts the integer to a char. Should we
> add a mask to make clear that only the LSB of the read value is taken ?
In fact, I think it's not need to add a mask to do it. But for
clearance, I can add it per your request.
mac[i] = readl(&fuse->mac_addr[i]) & 0xff;
>
>> + for (i = 0; i < 6; i++)
>> + mac[6-1-i] = readl(&fuse->mac_addr[i]);
> ^
> |--- missing spaces
do you mean it need change to mac[6 - 1 - i] ?
>
>> +
>> +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 res0[0x1f1];
>> + struct fuse_bank {
>> + u32 fuse_regs[0x20];
>> + u32 fuse_rsvd[0xe0];
>> + } bank[4];
>
> I see a discrepancy between i.mx27 and i.mx51 and it is not clear to me
> if it is correct. Both processor has the same register map (at least as
> meaning) until scs3. The offset for this register is for both processors
> 0x3c. The fuse bank0 starts for both processor at the offset 0x804, as I
> see in manuals. However, you reserved in one case 0x1f0 integers and in
> the other case 0x1f1. Is it correct ?
I think that the original mx27 code should be wrong. I will fix it.
Thank you for your review.
>
> 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: off...@denx.de
> =
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v5 1/1] imx: Get fec mac address from fuse
On 11/18/2010 09:09 AM, Jason Liu wrote:
> The patch is to support getting FEC MAC address from fuse bank.
>
> Signed-off-by: Jason Liu
Hi Jason,
> + for (i = 0; i < 6; i++)
> + mac[i] = readl(&fuse->mac_addr[i]);
This works, but implicitely converts the integer to a char. Should we
add a mask to make clear that only the LSB of the read value is taken ?
> + for (i = 0; i < 6; i++)
> + mac[6-1-i] = readl(&fuse->mac_addr[i]);
^
|--- missing spaces
> +
> +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 res0[0x1f1];
> + struct fuse_bank {
> + u32 fuse_regs[0x20];
> + u32 fuse_rsvd[0xe0];
> + } bank[4];
I see a discrepancy between i.mx27 and i.mx51 and it is not clear to me
if it is correct. Both processor has the same register map (at least as
meaning) until scs3. The offset for this register is for both processors
0x3c. The fuse bank0 starts for both processor at the offset 0x804, as I
see in manuals. However, you reserved in one case 0x1f0 integers and in
the other case 0x1f1. Is it correct ?
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: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v5 1/1] imx: Get fec mac address from fuse
The patch is to support getting FEC MAC address from fuse bank.
Signed-off-by: Jason Liu
---
Changes for v2:
- correct the mac address byte order according to review comments
- add memset(edev, 0. sizeof(*edev)) when do fec_probe,
- fix some code comments
Changes for v3:
- rebase
- address the comments of Stefano, make the imx_get_mac_from_fuse(),
declared independently inside each imx-regs.h to make it specific
for each processor, because it accesses to different structures.
- address the comments of Wolfgang and Stefano, use struct to access
mac_addr fuse.
Changes for v4:
- Address the comments of Wolfgang, use fuse_regs instead of bank addres
and change from fuse1_6 to fuse0_5 naming convention and use fuse_bank0_regs
instead of fuse_bank0 to kill misleading.
Changes for v5:
- Fix the typo error of fuse5_31[0x11] as Wolfgang point out
- Change the print message "got MAC address from EEPROM to
"got MAC address from fuse" which comply with this commit title.
---
arch/arm/cpu/arm926ejs/mx25/generic.c | 12 ++
arch/arm/cpu/arm926ejs/mx27/generic.c | 12 ++
arch/arm/cpu/armv7/mx5/soc.c | 14
arch/arm/include/asm/arch-mx25/imx-regs.h | 19 +--
arch/arm/include/asm/arch-mx27/imx-regs.h | 18 ++-
arch/arm/include/asm/arch-mx5/imx-regs.h | 34 +
drivers/net/fec_mxc.c | 17 +
7 files changed, 97 insertions(+), 29 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/mx25/generic.c
b/arch/arm/cpu/arm926ejs/mx25/generic.c
index b80a389..4ac7bbb 100644
--- a/arch/arm/cpu/arm926ejs/mx25/generic.c
+++ b/arch/arm/cpu/arm926ejs/mx25/generic.c
@@ -260,4 +260,16 @@ void mx25_fec_init_pins (void)
writel (outpadctl, &padctl->pad_fec_tdata1);
}
+
+void imx_get_mac_from_fuse(unsigned char *mac)
+{
+ int i;
+ struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE;
+ struct fuse_bank *bank = &iim->bank[0];
+ struct fuse_bank0_regs *fuse =
+ (struct fuse_bank0_regs *)bank->fuse_regs;
+
+ for (i = 0; i < 6; i++)
+ mac[i] = readl(&fuse->mac_addr[i]);
+}
#endif /* CONFIG_FEC_MXC */
diff --git a/arch/arm/cpu/arm926ejs/mx27/generic.c
b/arch/arm/cpu/arm926ejs/mx27/generic.c
index ae2ce58..ceaac48 100644
--- a/arch/arm/cpu/arm926ejs/mx27/generic.c
+++ b/arch/arm/cpu/arm926ejs/mx27/generic.c
@@ -313,6 +313,18 @@ void mx27_fec_init_pins(void)
for (i = 0; i < ARRAY_SIZE(mode); i++)
imx_gpio_mode(mode[i]);
}
+
+void imx_get_mac_from_fuse(unsigned char *mac)
+{
+ int i;
+ struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE;
+ struct fuse_bank *bank = &iim->bank[0];
+ struct fuse_bank0_regs *fuse =
+ (struct fuse_bank0_regs *)bank->fuse_regs;
+
+ for (i = 0; i < 6; i++)
+ mac[6-1-i] = readl(&fuse->mac_addr[i]);
+}
#endif /* CONFIG_FEC_MXC */
#ifdef CONFIG_MXC_MMC
diff --git a/arch/arm/cpu/armv7/mx5/soc.c b/arch/arm/cpu/armv7/mx5/soc.c
index 7c7a565..c22d6dc 100644
--- a/arch/arm/cpu/armv7/mx5/soc.c
+++ b/arch/arm/cpu/armv7/mx5/soc.c
@@ -100,6 +100,20 @@ int cpu_eth_init(bd_t *bis)
return rc;
}
+#if defined(CONFIG_FEC_MXC)
+void imx_get_mac_from_fuse(unsigned char *mac)
+{
+ int i;
+ struct iim_regs *iim = (struct iim_regs *)IMX_IIM_BASE;
+ struct fuse_bank *bank = &iim->bank[1];
+ struct fuse_bank1_regs *fuse =
+ (struct fuse_bank1_regs *)bank->fuse_regs;
+
+ for (i = 0; i < 6; i++)
+ mac[i] = readl(&fuse->mac_addr[i]);
+}
+#endif
+
/*
* Initializes on-chip MMC controllers.
* to override, implement board_mmc_init()
diff --git a/arch/arm/include/asm/arch-mx25/imx-regs.h
b/arch/arm/include/asm/arch-mx25/imx-regs.h
index f5a2929..55ad115 100644
--- a/arch/arm/include/asm/arch-mx25/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx25/imx-regs.h
@@ -36,6 +36,7 @@
#ifndef __ASSEMBLY__
#ifdef CONFIG_FEC_MXC
extern void mx25_fec_init_pins(void);
+extern void imx_get_mac_from_fuse(unsigned char *mac);
#endif
/* Clock Control Module (CCM) registers */
@@ -129,12 +130,17 @@ struct iim_regs {
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];
+ struct fuse_bank {
+ u32 fuse_regs[0x20];
+ u32 fuse_rsvd[0xe0];
+ } bank[3];
};
+
+struct fuse_bank0_regs {
+ u32 fuse0_25[0x1a];
+ u32 mac_addr[6];
+};
+
#endif
/* AIPS 1 */
@@ -312,7 +318,4 @@ struct iim_regs {
#define WSR_UNLOCK10x
#define WSR_UNLOCK20x
-/* FUSE bank offsets */
-#define IIM0_MAC 0x1a
-
#endif /* _IMX_REGS_H */
diff --git a/arch/arm/include/asm/arch-mx27/imx-regs.h
