Patchwork [U-Boot,v5,1/1] imx: Get fec mac address from fuse

login
register
mail settings
Submitter Liu Hui-R64343
Date Nov. 18, 2010, 8:09 a.m.
Message ID <1290067754-28200-1-git-send-email-r64343@freescale.com>
Download mbox | patch
Permalink /patch/72063/
State Superseded
Delegated to: Stefano Babic
Headers show

Comments

Liu Hui-R64343 - Nov. 18, 2010, 8:09 a.m.
The patch is to support getting FEC MAC address from fuse bank.

Signed-off-by: Jason Liu <r64343@freescale.com>

---
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(-)
Stefano Babic - Nov. 18, 2010, 9:06 a.m.
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 <r64343@freescale.com>

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
Jason Liu - Nov. 18, 2010, 10:33 a.m.
Hi, Stefano,

2010/11/18 Stefano Babic <sbabic@denx.de>:
> 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 <r64343@freescale.com>
>
> 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: office@denx.de
> =====================================================================
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
Stefano Babic - Nov. 18, 2010, 10:55 a.m.
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

Patch

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_UNLOCK1		0x5555
 #define WSR_UNLOCK2		0xAAAA
 
-/* FUSE bank offsets */
-#define IIM0_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..3095839 100644
--- a/arch/arm/include/asm/arch-mx27/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx27/imx-regs.h
@@ -34,6 +34,7 @@  extern void mx27_uart_init_pins(void);
 
 #ifdef CONFIG_FEC_MXC
 extern void mx27_fec_init_pins(void);
+extern void imx_get_mac_from_fuse(unsigned char *mac);
 #endif /* CONFIG_FEC_MXC */
 
 #ifdef CONFIG_MXC_MMC
@@ -203,8 +204,18 @@  struct iim_regs {
 	u32 iim_scs2;
 	u32 iim_scs3;
 	u32 res[0x1F0];
-	u32 iim_bank_area0[0x100];
+	struct fuse_bank {
+		u32 fuse_regs[0x20];
+		u32 fuse_rsvd[0xe0];
+	} bank[1];
 };
+
+struct fuse_bank0_regs {
+	u32 fuse0_3[4];
+	u32 mac_addr[6];
+	u32 fuse10_31[0x16];
+};
+
 #endif
 
 #define IMX_IO_BASE		0x10000000
@@ -512,9 +523,4 @@  struct iim_regs {
 #define IIM_ERR_SNSE	(1 << 2)
 #define IIM_ERR_PARITYE	(1 << 1)
 
-/* Definitions for i.MX27 TO2 */
-#define IIM0_MAC		5
-#define IIM0_SCC_KEY		11
-#define IIM1_SUID		1
-
 #endif				/* _IMX_REGS_H */
diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h b/arch/arm/include/asm/arch-mx5/imx-regs.h
index 0b6249a..d618bca 100644
--- a/arch/arm/include/asm/arch-mx5/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx5/imx-regs.h
@@ -205,9 +205,13 @@ 
 #define BOARD_REV_1_0           0x0
 #define BOARD_REV_2_0           0x1
 
+#define IMX_IIM_BASE            (IIM_BASE_ADDR)
+
 #if !(defined(__KERNEL_STRICT_NAMES) || defined(__ASSEMBLY__))
 #include <asm/types.h>
 
+extern void imx_get_mac_from_fuse(unsigned char *mac);
+
 #define __REG(x)	(*((volatile u32 *)(x)))
 #define __REG16(x)	(*((volatile u16 *)(x)))
 #define __REG8(x)	(*((volatile u8 *)(x)))
@@ -275,6 +279,36 @@  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 	res0[0x1f1];
+	struct fuse_bank {
+		u32 	fuse_regs[0x20];
+		u32 	fuse_rsvd[0xe0];
+	} bank[4];
+};
+
+struct fuse_bank1_regs {
+	u32	fuse0_8[9];
+	u32 	mac_addr[6];
+	u32 	fuse15_31[0x11];
+};
+
 #endif /* __ASSEMBLER__*/
 
 #endif				/*  __ASM_ARCH_MXC_MX51_H__ */
diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
index c17f937..0d0f392 100644
--- a/drivers/net/fec_mxc.c
+++ b/drivers/net/fec_mxc.c
@@ -312,21 +312,8 @@  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
-	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]);
-
+	imx_get_mac_from_fuse(mac);
 	return !is_valid_ether_addr(mac);
-#endif
 }
 
 static int fec_set_hwaddr(struct eth_device *dev)
@@ -754,7 +741,7 @@  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);
+		printf("got MAC address from fuse: %pM\n", ethaddr);
 		memcpy(edev->enetaddr, ethaddr, 6);
 	}