Skip to contentSkip to conversation

powerpc/eeprom: update MAX_NUM_PORTS to fix program failure

PATCH New Shengzhou Liu 3 participants 6 messages Aug 8, 2013 – Aug 30, 2013

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 powerpc/eeprom: update MAX_NUM_PORTS to fix program failure

On some boards, the size of EEPROM is 128 Bytes instead of 256. so we set default MAX_NUM_PORTS to 9 rather than previous 23 to avoid the programming failure, we can define MAX_NUM_PORTS in board-specific header file to overwrite the default value.

---
 board/freescale/common/sys_eeprom.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
board/freescale/common/sys_eeprom.c +3 -1
index d2ed036..6bfc376 100644
@@ -34,7 +34,9 @@
 #endif
 
 #ifdef CONFIG_SYS_I2C_EEPROM_NXID
-#define MAX_NUM_PORTS	23
+#ifndef MAX_NUM_PORTS
+#define MAX_NUM_PORTS	9
+#endif
 #define NXID_VERSION	1
 #endif
 
Signature · 1 line
1.8.0
#1 lore raw

PATCH 1/1 powerpc/eeprom: update MAX_NUM_PORTS to fix program failure

Quoted 4 lines · Shengzhou Liu

On some boards, the size of EEPROM is 128 Bytes instead of 256. so we set default MAX_NUM_PORTS to 9 rather than previous 23 to avoid the programming failure, we can define MAX_NUM_PORTS in board-specific header file to overwrite the default value.

NACK.

If the EEPROM is 128 bytes, then you have a non-conformant EEPROM. And using the #ifdef to determine this is definitely the wrong way.

#2 lore raw

PATCH 1/1 powerpc/eeprom: update MAX_NUM_PORTS to fix program failure

Quoted 11 lines

-----Original Message-----
From: Timur Tabi [mailto:timur at tabi.org]
Sent: Monday, August 12, 2013 7:51 AM
To: Liu Shengzhou-B36685
Cc: U-Boot Mailing List; sun york-R58495
Subject: Re: [U-Boot] [PATCH] powerpc/eeprom: update MAX_NUM_PORTS to fix program failure

On Thu, Aug 8, 2013 at 5:14 AM, Shengzhou Liu <[email protected]> wrote:

Quoted 8 lines
Quoted 4 lines

On some boards, the size of EEPROM is 128 Bytes instead of 256. so we set default MAX_NUM_PORTS to 9 rather than previous 23 to avoid the programming failure, we can define MAX_NUM_PORTS in board-specific header file to overwrite the default value.

NACK.

If the EEPROM is 128 bytes, then you have a non-conformant EEPROM.

What is a conformant EEPROM?
The size of struct of EEPROM_NXID should be able to conform to real size of EEPROM, regardless it's 128 or 256 EEPROM. It's not reasonable to limit MAX_NUM_PORTS to 23, generally we don't need 23 MAC addresses to store in EEPROM. 23 is just suitable to 256 bytes EEPROM.

Quoted 1 line

And using the #ifdef to determine this is definitely the wrong way.

Why? What's your way?

#3 lore raw

PATCH 1/1 powerpc/eeprom: update MAX_NUM_PORTS to fix program failure

Quoted 3 lines · On 08/29/2013 04:56 AM, Liu Shengzhou-B36685 wrote:
Quoted 1 line

If the EEPROM is 128 bytes, then you have a non-conformant EEPROM.

What is a conformant EEPROM?

A conformant EEPROM has a size of 256 bytes .

Quoted 1 line

The size of struct of EEPROM_NXID should be able to conform to real size of EEPROM, regardless it's 128 or 256 EEPROM.

The problem is that the CRC is at the end of the structure, so that

Quoted 2 lines

It's not reasonable to limit MAX_NUM_PORTS to 23, generally we don't need 23 MAC addresses to store in EEPROM. 23 is just suitable to 256 bytes EEPROM.

Actually, the 23 should be changed to 31. York, this patch needs to be
applied: http://patchwork.ozlabs.org/patch/170753/

Quoted 3 lines
Quoted 1 line

And using the #ifdef to determine this is definitely the wrong way.

Why? What's your way?

If you need to define a new EEPROM format, then the version number needs to be changed to v2, and the code needs to dynamically handle v1 and v2.

BTW, your patch breaks EVERY OTHER BOARD. You can't just change the 23 to a 9 for every board. Did you test your patch on other boards?

Signature · 1 line
Timur Tabi
#4 lore raw

PATCH 1/1 powerpc/eeprom: update MAX_NUM_PORTS to fix program failure

Quoted 10 lines

-----Original Message-----
From: Timur Tabi [mailto:timur at tabi.org]
Sent: Thursday, August 29, 2013 11:09 PM
To: Liu Shengzhou-B36685
Cc: U-Boot Mailing List; sun york-R58495
Subject: Re: [U-Boot] [PATCH] powerpc/eeprom: update MAX_NUM_PORTS to fix program failure

Quoted 2 lines

Actually, the 23 should be changed to 31. York, this patch needs to be
applied: http://patchwork.ozlabs.org/patch/170753/

According to AN3638, it should be 30 rather than 31 for 256-bytes EEPROM.

#5 lore raw

PATCH 1/1 powerpc/eeprom: update MAX_NUM_PORTS to fix program failure

Quoted 4 lines · On 08/30/2013 06:04 AM, Liu Shengzhou-B36685 wrote:
Quoted 2 lines

Actually, the 23 should be changed to 31. York, this patch needs to be
applied: http://patchwork.ozlabs.org/patch/170753/

According to AN3638, it should be 30 rather than 31 for 256-bytes EEPROM.

Are you talking about the C struct? That's just an example. Besides, the C struct has an error in it:

unsigned char res_u[7]; // F6-FB: reserved

this should be res_u[6].

Besides, the appnote says elsewhere:

"0xA2 ? 0xFB reserved
Reserved for additional MAC addresses or other data. NXID readers need only consider the MACSIZE field to determine if this space is used for additional MAC addresses."

Which means that you can fill the EEPROM with MAC addresses. If you do the math, that means 31 addresses.

Signature · 1 line
Timur Tabi