On Mon, 14 Nov 2011 08:48:38 +0000, Tang Yuantian-B29983 wrote: > > > > -----Original Message----- > > From: Jean Delvare [mailto:khali@xxxxxxxxxxxx] > > Sent: 2011年11月14日 16:02 > > To: Tang Yuantian-B29983 > > Cc: lm-sensors@xxxxxxxxxxxxxx; Tang Yuantian-B29983; Huang Changming- > > R66093; Tabi Timur-B04825 > > Subject: Re: [PATCH] hwmon: (pmbus) Fix two issues > > > > Hi Tang, > > > > On Mon, 14 Nov 2011 13:41:06 +0800, Yuantian.Tang@xxxxxxxxxxxxx wrote: > > > From: Tang Yuantian <B29983@xxxxxxxxxxxxx> > > > > > > 1. Not all platforms support i2c_smbus_read_block_data function. > > > Use i2c_smbus_read_i2c_block_data instead. > > > > You can't do that. These are two different transfer formats, they are not > > interchangeable. If the device wants the SMBus block format, then you > > have to use this in the driver. If the bus driver doesn't support that, > > it has to be fixed. In general it is only a matter of adding support for > > I2C_M_RECV_LEN to the bus driver. You can check in > > i2c-core:i2c_smbus_xfer_emulated() for a reference implementation. > > > [Yuantian:] almost all platform's i2c driver don't support i2c_smbus_read_block_data. > Why can't we just use i2c_smbus_read_i2c_block_data instead. > Beside, according my test, i2c_smbus_read_block_data can't return correct value, > But i2c_smbus_read_i2c_block_data. > I think i2c_smbus_read_i2c_block_data is only right choice. Again, no. PMBus goes on top of SMBus which includes SMBus block reads (i2c_smbus_read_block_data) but not I2C block reads (i2c_smbus_read_i2c_block_data). As a result, many hardware SMBus controllers do not support I2C block reads, and it can't be added (only the set of transfers supported by the hardware controller can be implemented by the driver) which means that your proposed change would break support on existing systems. This is not acceptable. On the other hand, an I2C controller which supports I2C block reads can almost always get SMBus block reads added, it's only a matter of adding a few lines of code to the bus driver. Which i2c bus driver are you using? From which kernel version? > > > 2. Not all zlxx chips's id start with zlxx. zl6100's id starts with > > > 0x10. Take this situation into account. > > > > No, this is incorrect. 0x10 is the first byte returned because SMBus > > block reads receive the block length as the first byte. 0x10 == 16, which > > is the length of the ID for these chips, this is no coincidence. > > This is a bug introduced by your above change of SMBus block read for I2C > > block read. > > > > So, nack. > > > [Yuantian:] I misunderstand the 0x10. But the existing zl6100 driver didn't consider it too. > And still need to be fixed. I didn't write the driver, and don't have the device, but I think the driver is OK as is. The block length (0x10) is handled by the underlying bus driver, the device driver never gets to see it, if you properly call i2c_smbus_read_block_data() as you are supposed to. -- Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
