On 11/09/2011 12:37 PM, Jean Delvare wrote:
On Wed, 09 Nov 2011 07:56:13 -0200, Mauro Carvalho Chehab wrote:ret = i2c_transfer(i2c_cfg->adapter, msg, 2); Produces a different result. In the latter case, I2C core avoids having any other transaction in the middle of the 2 messages.This is correct, but this isn't the only difference. The second difference is that, with the code above, a repeated-start condition is used between both messages, instead of a stop condition followed by a start condition. While ideally all controllers, all controller drivers and all slaves would support that, I don't think this is true in practice.
I agree, as we just replied same time to that message :)
I agree that it makes some sense. We recently added helper functions for swapped word reads, to avoid code duplication amongst device drivers. This would follow a similar logic. However you should bear in mind that different I2C devices have different expectations and requirements. Some do automatic register address increment, some don't. Some support arbitrary read/write length and alignment, some don't. It is common that write constraints differ from read constraints. So you won't possibly come up with universal I2C read and write functions. There is a reason why it was originally decided to only provide the low-level transfer functions in i2c-core and leave the rest up to individual device drivers.
So true. Some chips allow even configure auto increment or not and even more. But I take that most common behaviour way. It is never possible to support all I2C access formats chip vendors will define. But that first reg then values with auto increment is very common, maybe over 90% cases.
If code is duplicated, then something should indeed be done about it. But preferably after analyzing properly what the helper functions should look like, and for this you'll have to look at "all" drivers that could benefit from it. At the moment only the tda18218 driver was reported to need it, that's not enough to generalize.
Only?, I think you missed part of my first mail I mentioned 7 cases from *my* drivers where splitting is needed. Actually, I just remember one more, it is AF9015 & AF9033. And those are for the splitting only, same function can be used maybe 90% of current tuner and demod drivers we have.
You should take a look at drivers/misc/eeprom/at24.c, it contains fairly complete transfer functions which cover the various EEPROM types. Non-EEPROM devices could behave differently, but this would still seem to be a good start for any I2C device using block transfers. It was once proposed that these functions could make their way into i2c-core or a generic i2c helper function. Both at24 and Antti's proposal share the idea of storing information about the device capabilities (max block read and write lengths, but we could also put there alignment requirements or support for repeated start condition.) in a private structure. If we generalize the functions then this information would have to be stored in struct i2c_client and possibly struct i2c_adapter (or struct i2c_algorithm) so that the function can automatically find out the right sequence of commands for the adapter/slave combination. Speaking of struct i2c_client, I seem to remember that the dvb subsystem doesn't use it much at the moment. This might be an issue if you intend to get the generic code into i2c-core, as most helper functions rely on a valid i2c_client structure by design.
I will check those later today, have to go back lecture. regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
