On Mon, 2013-12-02 at 16:02 +0800, Kuo-Jung Su wrote: > From: Kuo-Jung Su <dant...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
