[U-Boot] [PATCH] 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.
Signed-off-by: Timur Tabi timur@freescale.com --- board/freescale/common/sys_eeprom.c | 18 +++++++++++++++++- 1 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/board/freescale/common/sys_eeprom.c b/board/freescale/common/sys_eeprom.c index 5a8f4f5..c5f1c06 100644 --- a/board/freescale/common/sys_eeprom.c +++ b/board/freescale/common/sys_eeprom.c @@ -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; }
Dear Timur Tabi,
In message 1280772203-8859-1-git-send-email-timur@freescale.com 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.
Signed-off-by: Timur Tabi timur@freescale.com
board/freescale/common/sys_eeprom.c | 18 +++++++++++++++++-
...
- /*
* 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.*/@@ -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;- }
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
Wolfgang Denk wrote:
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.
Dear Timur Tabi,
In message 4C57336E.7050704@freescale.com you wrote:
Wolfgang Denk wrote:
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
Dear Timur Tabi,
In message 4C573530.9050004@freescale.com you wrote:
Wolfgang Denk wrote:
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
Wolfgang Denk wrote:
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.
On Aug 2, 2010, at 1:03 PM, Timur Tabi 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.
Signed-off-by: Timur Tabi timur@freescale.com
board/freescale/common/sys_eeprom.c | 18 +++++++++++++++++- 1 files changed, 17 insertions(+), 1 deletions(-)
applied to 85xx
- k
participants (3)
-
Kumar Gala -
Timur Tabi -
Wolfgang Denk