Skip to contentSkip to conversation

fsl: verify writes to the MAC address EEPROM

PATCH New Timur Tabi 3 participants 8 messages Aug 2, 2010 – Sep 30, 2010

lore
Share & tools
lore

Timur Tabi

author
#0 lore raw

PATCH 1/1 fsl: verify writes to the MAC address EEPROM

Update the code which writes to the on-board EEPROM so that it can detect if the write failed because the EEPROM is write-protected. Most of the 8xxx-class Freescale reference boards use an AT24C02 EEPROM to store MAC addresses and similar information. With this patch, if the EEPROM is protected, the "mac save" command will display an error message indicating that the write has not succeeded.

---
 board/freescale/common/sys_eeprom.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)
board/freescale/common/sys_eeprom.c +17 -1
index 5a8f4f5..c5f1c06 100644
@@ -204,7 +204,7 @@ static void update_crc(void)
  */
 static int prog_eeprom(void)
 {
-	int ret = 0; /* shut up gcc */
+	int ret = 0;
 	int i;
 	void *p;
 #ifdef CONFIG_SYS_EEPROM_BUS_NUM
@@ -225,6 +225,11 @@ static int prog_eeprom(void)
 	i2c_set_bus_num(CONFIG_SYS_EEPROM_BUS_NUM);
 #endif
 
+	/*
+	 * The AT24C02 datasheet says that data can only be written in page
+	 * mode, which means 8 bytes at a time, and it takes up to 5ms to
+	 * complete a given write.
+	 */
 	for (i = 0, p = &e; i < sizeof(e); i += 8, p += 8) {
 		ret = i2c_write(CONFIG_SYS_I2C_EEPROM_ADDR, i, CONFIG_SYS_I2C_EEPROM_ADDR_LEN,
 			p, min((sizeof(e) - i), 8));
@@ -233,12 +238,23 @@ static int prog_eeprom(void)
 		udelay(5000);	/* 5ms write cycle timing */
 	}
 
+	if (!ret) {
+		/* Verify the write by reading back the EEPROM and comparing */
+		struct eeprom e2;
+
+		ret = i2c_read(CONFIG_SYS_I2C_EEPROM_ADDR, 0,
+			CONFIG_SYS_I2C_EEPROM_ADDR_LEN, (void *)&e2, sizeof(e2));
+		if (!ret && memcmp(&e, &e2, sizeof(e)))
+			ret = -1;
+	}
+
 #ifdef CONFIG_SYS_EEPROM_BUS_NUM
 	i2c_set_bus_num(bus);
 #endif
 
 	if (ret) {
 		printf("Programming failed.\n");
+		has_been_read = 0;
 		return -1;
 	}
 
Signature · 1 line
1.7.0.1
#1 lore raw

PATCH 1/1 fsl: verify writes to the MAC address EEPROM

Dear Timur Tabi,

Quoted 10 lines · In message <[email protected]> you wrote:

Update the code which writes to the on-board EEPROM so that it can detect if the write failed because the EEPROM is write-protected. Most of the 8xxx-class Freescale reference boards use an AT24C02 EEPROM to store MAC addresses and similar information. With this patch, if the EEPROM is protected, the "mac save" command will display an error message indicating that the write has not succeeded.

---
 board/freescale/common/sys_eeprom.c |   18 +++++++++++++++++-

...

Quoted 12 lines
board/freescale/common/sys_eeprom.c +5 -0
+	/*
+	 * The AT24C02 datasheet says that data can only be written in page
+	 * mode, which means 8 bytes at a time, and it takes up to 5ms to
+	 * complete a given write.
+	 */
 	for (i = 0, p = &e; i < sizeof(e); i += 8, p += 8) {
 		ret = i2c_write(CONFIG_SYS_I2C_EEPROM_ADDR, i, CONFIG_SYS_I2C_EEPROM_ADDR_LEN,
 			p, min((sizeof(e) - i), 8));
@@ -233,12 +238,23 @@ static int prog_eeprom(void)
 		udelay(5000);	/* 5ms write cycle timing */
 	}
 

board/freescale/common/sys_eeprom.c static int prog_eeprom(void) · line 250

Quoted 10 lines
board/freescale/common/sys_eeprom.c +10 -0
+	if (!ret) {
+		/* Verify the write by reading back the EEPROM and comparing */
+		struct eeprom e2;
+
+		ret = i2c_read(CONFIG_SYS_I2C_EEPROM_ADDR, 0,
+			CONFIG_SYS_I2C_EEPROM_ADDR_LEN, (void *)&e2, sizeof(e2));
+		if (!ret && memcmp(&e, &e2, sizeof(e)))
+			ret = -1;
+	}
+

Why is this i2c_read() needed or actually useful? Should the error return code from the i2c_write() above not be sufficient indication that the writing failed? If that was the case, then some other parts of the code need fixing.

Best regards,

Wolfgang Denk

Signature · 4 lines
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
I'd rather be led to hell than managed to heaven.
author
#2 lore raw

PATCH 1/1 fsl: verify writes to the MAC address EEPROM

Quoted 4 lines · Wolfgang Denk

Why is this i2c_read() needed or actually useful? Should the error return code from the i2c_write() above not be sufficient indication that the writing failed? If that was the case, then some other parts of the code need fixing.

That's just the way the EEPROM chip works. When it's set into write-protect mode, it cheerfully accepts all of the I2C write commands, and acknowledges them appropriately, but it doesn't actually store the data into the EEPROM.
The read-back is the only way I've found to verify that the write has
actually occurred.

This patch fixes a real problem with all of our boards that use this EEPROM chip, which is almost all of our 83xx, 85xx, and 86xx boards. Some of our boards would be shipped with EEPROM write-protected. The current code caches the EEPROM contents in memory. After using the "mac save" command, the code would think that the EEPROM was updated. Issuing the "mac read" command again would *not* re-read from the EEPROM, because the code would think that the cached value is up-to-date. The only way you would know if the EEPROM write failed is to reboot the board.

#3 lore raw

PATCH 1/1 fsl: verify writes to the MAC address EEPROM

Dear Timur Tabi,

Quoted 11 lines · In message <[email protected]> you wrote:
Quoted 4 lines · Wolfgang Denk

Why is this i2c_read() needed or actually useful? Should the error return code from the i2c_write() above not be sufficient indication that the writing failed? If that was the case, then some other parts of the code need fixing.

That's just the way the EEPROM chip works. When it's set into write-protect mode, it cheerfully accepts all of the I2C write commands, and acknowledges them appropriately, but it doesn't actually store the data into the EEPROM.
The read-back is the only way I've found to verify that the write has
actually occurred.

Is it correct to assume the the WP signal is connected to some GPIO pin which can be set / unset in software?

Best regards,

Wolfgang Denk

Signature · 4 lines
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
Lots of folks confuse bad management with destiny.   -- Frank Hubbard
author
#4 lore raw

PATCH 1/1 fsl: verify writes to the MAC address EEPROM

Quoted 2 lines · Wolfgang Denk

Is it correct to assume the the WP signal is connected to some GPIO pin which can be set / unset in software?

No, it's a dip switch on the board. You have to physically set the dip switch in order write protect or unprotect the EEPROM.

#5 lore raw

PATCH 1/1 fsl: verify writes to the MAC address EEPROM

Dear Timur Tabi,

Quoted 6 lines · In message <[email protected]> you wrote:
Quoted 2 lines · Wolfgang Denk

Is it correct to assume the the WP signal is connected to some GPIO pin which can be set / unset in software?

No, it's a dip switch on the board. You have to physically set the dip switch in order write protect or unprotect the EEPROM.

And the setting cannot be read back through some GPIO pin eitheer? Arghh... what a broken design...

Thanks for the explanation.

Best regards,

Wolfgang Denk

Signature · 6 lines
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
"There's only one kind of woman ..." "Or man, for  that  matter.  You
either believe in yourself or you don't."
	-- Kirk and Harry Mudd, "Mudd's Women", stardate 1330.1
author
#6 lore raw

PATCH 1/1 fsl: verify writes to the MAC address EEPROM

Quoted 1 line · Wolfgang Denk

And the setting cannot be read back through some GPIO pin eitheer?

Technically, the FPGA (on ngPIXIS boards, of which there are only a few) reads all of the switches and allows software to set and read the values. But I've found it to be unreliable. For example, on one board, SW7 is set to 00001101, but the ngPIXIS shows 00000000.

And even if it were reliable, it wouldn't help, because the ngPIXIS reads the software-programmable settings only on boot time. So in order for me to override the dip switch, I would need to program the right value and then reboot the board.

And that assumes that the board doesn't have a broken FPGA firmware. I believe the current version is 11, or maybe 13. The version on my board is 4.

The majority of our boards that use this EEPROM don't have an ngPIXIS, they have the old-style PIXIS, which definitely does not allow software to read/set any dip switches.

And this isn't the worst part of our designs. On the P1022, for example, the same pins are used for the DVI signals and the local bus (used for NOR flash), so it's not possible to use flash memory while the video display is
active. That means we can't turn on the video until after relocation, and
when we do, we lose the ability to update flash.

Applied
#7 lore raw

PATCH 1/1 fsl: verify writes to the MAC address EEPROM

In reply to Timur Tabi
Quoted 11 lines · Timur Tabi

Update the code which writes to the on-board EEPROM so that it can detect if the write failed because the EEPROM is write-protected. Most of the 8xxx-class Freescale reference boards use an AT24C02 EEPROM to store MAC addresses and similar information. With this patch, if the EEPROM is protected, the "mac save" command will display an error message indicating that the write has not succeeded.

---
board/freescale/common/sys_eeprom.c | 18 +++++++++++++++++-

1 files changed, 17 insertions(+), 1 deletions(-)

applied to 85xx

- k