[U-Boot] [PATCH] fsl: verify writes to the MAC address EEPROM

[Wolfgang Denk <wd@denx.de>]

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.
> +	 */
>  	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;
> +	}
> +

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

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