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

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

[Jason Liu]

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
-- 
1.7.0.4

[U-Boot] [PATCH 2/2] MX5: board version not printed corretly on MX51EVK

[Jason Liu]

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) {
-- 
1.7.0.4

[U-Boot] [PATCH 2/2] MX5: board version not printed corretly on MX51EVK

[Stefano Babic]

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

-- 
=====================================================================
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 at denx.de
=====================================================================

[U-Boot] [PATCH 2/2] MX5: board version not printed corretly on MX51EVK

[Jason Liu]

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 at denx.de
> =====================================================================
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH 2/2] MX5: board version not printed corretly on MX51EVK

[Stefano Babic]

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

-- 
=====================================================================
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 at denx.de
=====================================================================

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

[Stefano Babic]

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

-- 
=====================================================================
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 at denx.de
=====================================================================

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

[Jason Liu]

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 at denx.de
> =====================================================================
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

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

[Stefano Babic]

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 at lists.denx.de/msg37629.html

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 at denx.de
=====================================================================

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

[Jason Liu]

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 at 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 at denx.de
> =====================================================================
>

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

[Stefano Babic]

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

-- 
=====================================================================
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 at denx.de
=====================================================================

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

[Jason Liu]

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 at denx.de
> =====================================================================
>

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

[John Rigby]

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 at denx.de
>> ====================================================================
>>
> _______________________________________________
> U-Boot mailing list
> U-Boot at 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.

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

[Wolfgang Denk]

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

-- 
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 at denx.de
"A child is a person who can't understand why someone would give away
a perfectly good kitten."                               - Doug Larson

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

[John Rigby]

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 at denx.de
> "A child is a person who can't understand why someone would give away
> a perfectly good kitten." ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? - Doug Larson
>

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

[Shawn Guo]

I was not in the mail list when Jason posted the patch, so I emulated
the reply message.

> 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
> +	 */
>
The comment was added to distinguish the MX27 support from MX51/25 on
mac fuse.  Since the support becomes common, can we just simply remove
the comment?  Also, the mac is implemented in fuse like the message
title tells than "eeprom".

>  	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]);
>
There is a bug.  The fix could be as below (tested on mx51evk).

-               mac[6-1-i] = readl(&iim->iim_bank_area0[IIM0_MAC + i]);
+              mac[i] = readl(&iim->iim_bank_area[IIM_MAC + i]);

> 	return !is_valid_ether_addr(mac);
> -#endif
> }


-- 
Regards,
Shawn