public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] powerpc/eeprom: update MAX_NUM_PORTS to fix program failure
@ 2013-08-08  9:14 Shengzhou Liu
  2013-08-11 23:50 ` Timur Tabi
  0 siblings, 1 reply; 6+ messages in thread
From: Shengzhou Liu @ 2013-08-08  9:14 UTC (permalink / raw)
  To: u-boot

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(-)

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] powerpc/eeprom: update MAX_NUM_PORTS to fix program failure
  2013-08-08  9:14 [U-Boot] [PATCH] powerpc/eeprom: update MAX_NUM_PORTS to fix program failure Shengzhou Liu
@ 2013-08-11 23:50 ` Timur Tabi
  2013-08-29  9:56   ` Liu Shengzhou-B36685
  0 siblings, 1 reply; 6+ messages in thread
From: Timur Tabi @ 2013-08-11 23:50 UTC (permalink / raw)
  To: u-boot

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] powerpc/eeprom: update MAX_NUM_PORTS to fix program failure
  2013-08-11 23:50 ` Timur Tabi
@ 2013-08-29  9:56   ` Liu Shengzhou-B36685
  2013-08-29 15:09     ` Timur Tabi
  0 siblings, 1 reply; 6+ messages in thread
From: Liu Shengzhou-B36685 @ 2013-08-29  9:56 UTC (permalink / raw)
  To: u-boot


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] powerpc/eeprom: update MAX_NUM_PORTS to fix program failure
  2013-08-29  9:56   ` Liu Shengzhou-B36685
@ 2013-08-29 15:09     ` Timur Tabi
  2013-08-30 11:04       ` Liu Shengzhou-B36685
  0 siblings, 1 reply; 6+ messages in thread
From: Timur Tabi @ 2013-08-29 15:09 UTC (permalink / raw)
  To: u-boot

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?

-- 
Timur Tabi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] powerpc/eeprom: update MAX_NUM_PORTS to fix program failure
  2013-08-29 15:09     ` Timur Tabi
@ 2013-08-30 11:04       ` Liu Shengzhou-B36685
  2013-08-30 13:19         ` Timur Tabi
  0 siblings, 1 reply; 6+ messages in thread
From: Liu Shengzhou-B36685 @ 2013-08-30 11:04 UTC (permalink / raw)
  To: u-boot



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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] powerpc/eeprom: update MAX_NUM_PORTS to fix program failure
  2013-08-30 11:04       ` Liu Shengzhou-B36685
@ 2013-08-30 13:19         ` Timur Tabi
  0 siblings, 0 replies; 6+ messages in thread
From: Timur Tabi @ 2013-08-30 13:19 UTC (permalink / raw)
  To: u-boot

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.

-- 
Timur Tabi

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-08-30 13:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-08  9:14 [U-Boot] [PATCH] powerpc/eeprom: update MAX_NUM_PORTS to fix program failure Shengzhou Liu
2013-08-11 23:50 ` Timur Tabi
2013-08-29  9:56   ` Liu Shengzhou-B36685
2013-08-29 15:09     ` Timur Tabi
2013-08-30 11:04       ` Liu Shengzhou-B36685
2013-08-30 13:19         ` Timur Tabi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox