| Message ID | 1375953293-1060-1-git-send-email-Shengzhou.Liu@freescale.com |
|---|---|
| State | Superseded |
| Delegated to: | York Sun |
| Headers | show |
On Thu, Aug 8, 2013 at 5:14 AM, Shengzhou Liu <Shengzhou.Liu@freescale.com> wrote: > 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.
> -----Original Message----- > From: Timur Tabi [mailto:timur@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 <Shengzhou.Liu@freescale.com> > wrote: > > 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. > And using the #ifdef to determine this is definitely the wrong way. Why? What's your way?
On 08/29/2013 04:56 AM, Liu Shengzhou-B36685 wrote: >> 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 . > 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 > 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/ >> 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?
> -----Original Message----- > From: Timur Tabi [mailto:timur@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 > > 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.
On 08/30/2013 06:04 AM, Liu Shengzhou-B36685 wrote: >> 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.
diff --git a/board/freescale/common/sys_eeprom.c b/board/freescale/common/sys_eeprom.c index d2ed036..6bfc376 100644 --- a/board/freescale/common/sys_eeprom.c +++ b/board/freescale/common/sys_eeprom.c @@ -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
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. Signed-off-by: Shengzhou Liu <Shengzhou.Liu@freescale.com> --- board/freescale/common/sys_eeprom.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)