2013/11/29 Alexey Brodkin <alexey.brod...@synopsys.com>: > Hi Kuo-Jung, > > On Fri, 2013-11-29 at 08:59 +0800, Kuo-Jung Su wrote: >> No it's a common misunderstanding, I also made the same mistake before. >> Please check my bug fix for Faraday FTI2C010 I2C driver: >> >> http://patchwork.ozlabs.org/patch/294732/ >> >> The address should be rebuild inside the i2c driver in u-boot's I2C model. >> >> Another example could be found at drivers/i2c/soft_i2c.c, line 382 - 389 >> >> static int soft_i2c_read (...) >> { >> ...... >> shift = (alen-1) * 8; >> while(alen-- > 0) { >> if(write_byte(addr >> shift)) { >> PRINTD("i2c_read, address not <ACK>ed\n"); >> return(1); >> } >> shift -= 8; >> } >> ...... >> } >> >> However the root cause is the u-boot i2c driver model, the address should >> never be passed to i2c_read/i2c_write in uint format. >> >> Please take a peak at how ecos defineds the relative i2c routines: >> >> externC cyg_uint32 cyg_i2c_transaction_tx(const cyg_i2c_device*, >> cyg_bool, const cyg_uint8*, cyg_uint32, cyg_bool); >> externC cyg_uint32 cyg_i2c_transaction_rx(const cyg_i2c_device*, >> cyg_bool, cyg_uint8*, cyg_uint32, cyg_bool, cyg_bool); >> >> In this way, the eeprom address would be forced to rebuild in address >> pointer (i.e., addr[]) as what we did in SPI mode. > > Unfortunately I still cannot agree with you. > In my opinion I2C driver has nothing to do with current situation. >
Yes, that's why I said the root cause is U-Boot's I2C model. The address should never be rebuilt/reformated inside the I2C driver. The better solution is to update the i2c_read/i2c_write to: int i2c_read(struct i2c_adapter *adap, u8 dev, uint *addr, int alen, uchar *buf, int len) or int i2c_read(struct i2c_adapter *adap, u8 dev, uchar *buf, int len) i.e., drop the use of uint 'addr' > It's purely how manufacturers of EEPROM use I2C. > For example I we use "PCF8594C-2" EEPROM. > Here's a datasheet - > http://www.nxp.com/documents/data_sheet/PCF8594C_2.pdf > > Its capacity is 512 bytes. And as you may see from "Fig 3. Device > addressing." Each IC houses 2 virtual EEPROMs (each housing 256 bytes of > data). > Yes, it looks like a special case which use BIT0 of device address for page selection. Which means we can't directly pass the device address to i2c_read/i2c_write. But it's still o.k to directly the 'offset' as what we did before. > And if you accept this philosophy you'll understand why "I2C chip > address" is modified and why following approach is used in > "cmd_eeprom.c" for "CONFIG_SYS_I2C_EEPROM_ADDR_LEN == 1" (which says > that EEPROM has address length of 1 byte): > ================== > addr[0] = offset >> 8; /* block number */ > addr[1] = blk_off; /* block offset */ > addr[0] |= dev_addr; /* insert device address */ > ================== > > From source code above you see that virtually it addresses multiple 256 > byte EEPROMs. > > Regards, > Alexey > -- Best wishes, Kuo-Jung Su _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot