On Mon, 2013-12-02 at 16:02 +0800, Kuo-Jung Su wrote:
From: Kuo-Jung Su dantesu@faraday-tech.com
diff --git a/common/cmd_eeprom.c b/common/cmd_eeprom.c index 02539c4..3924805 100644 --- a/common/cmd_eeprom.c +++ b/common/cmd_eeprom.c @@ -161,7 +161,7 @@ int eeprom_read (unsigned dev_addr, unsigned offset, uchar *buffer, unsigned cnt #if defined(CONFIG_SPI) && !defined(CONFIG_ENV_EEPROM_IS_ON_I2C) spi_read (addr, alen, buffer, len); #else
if (i2c_read (addr[0], addr[1], alen-1, buffer, len) != 0)
if (i2c_read(addr[0], offset, alen - 1, buffer, len)) rcode = 1;#endif buffer += len;
I think this change is whether incomplete or improper. Let's look at source code above line 161: ============================= 125 #if CONFIG_SYS_I2C_EEPROM_ADDR_LEN == 1 && !defined(CONFIG_SPI_X) 126 uchar addr[2]; 127 128 blk_off = offset & 0xFF; /* block offset */ 129 130 addr[0] = offset >> 8; /* block number */ 131 addr[1] = blk_off; /* block offset */ 132 alen = 2; 133 #else 134 uchar addr[3]; 135 136 blk_off = offset & 0xFF; /* block offset */ 137 138 addr[0] = offset >> 16; /* block number */ 139 addr[1] = offset >> 8; /* upper address octet */ 140 addr[2] = blk_off; /* lower address octet */ 141 alen = 3; 142 #endif /* CONFIG_SYS_I2C_EEPROM_ADDR_LEN, CONFIG_SPI_X */ 143 144 addr[0] |= dev_addr; /* insert device address */ =============================
From these line you see that "addr[0]" is set like this:
=========== If "CONFIG_SYS_I2C_EEPROM_ADDR_LEN == 1": addr[0] = offset >> 8;
If "CONFIG_SYS_I2C_EEPROM_ADDR_LEN > 1": addr[0] = offset >> 16;
And in both cases then OR with "dev_addr": addr[0] |= dev_addr; ===========
In other words it gets both real I2S slave address + MSB bits of offset. But note that "offset" value stays unchanged.
So if you pass both "addr[0]" (which already has MSB bits of "offset") and "offset" itself then you'll get completely incorrect I2C command.
@@ -339,7 +339,7 @@ int eeprom_write (unsigned dev_addr, unsigned offset, uchar *buffer, unsigned cn /* Write is enabled ... now write eeprom value. */ #endif
if (i2c_write (addr[0], addr[1], alen-1, buffer, len) != 0)
if (i2c_write(addr[0], offset, alen - 1, buffer, len)) rcode = 1;#endif
Same goes to "eeprom_write".
Moreover I'd say that this address/offset tricks are very EEPROM-specific and because of this we'd better keep it here and don't modify generic I2C code.
Regards, Alexey