Re: [PATCH] i2c: core: add support for read commands longer than byte

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9 August 2015 at 09:40, Wolfram Sang <wsa@xxxxxxxxxxxxx> wrote:
> On Fri, Jul 24, 2015 at 03:20:42PM +0200, Crt Mori wrote:
>> If you want to send more than 1 byte as command to slave you need to go away
>> from the smbus (which has a 8 bit command defined in specification). i2c has
>> no such limitation in specification and since we want to have a longer than
>> 1 byte command, we need additional function to handle it. With outside
>> buffers we have also avoided endianness problems (if we would just define a
>> u16 command instead of u8). Also there are now no limitations (as per i2c
>> specification) of command length.
>> Addressed read function is when we have a slave device which accepts commands
>> longer than byte or some subcommands in same write string before issuing a
>> repeat start before read cycle to read more than byte (or byte) from slave
>> device.
>>
>> Signed-off-by: Crt Mori <cmo@xxxxxxxxxxx>
>
> I don't see much gain over using i2c_transfer directly to be honest. Do
> you see much use of that in the kernel? Any other gain I missed? And if
> at all, the function should be probably named i2c_write_then_read or
> something.
>
This debate is what I wanted to have. The new Melexis sensor will be
using it and
since I think some others might also be using it in future I would
rather put it to
i2c. It is more i2c command than sensor specific so I think it fits into i2c.

Another gain is that when number of functions each device does increase, as
well as ram/rom addressing read commands with longer than 8-bit register read
commands will be needed. Especially since most sensors have digital design of
i2c interface (not software) which means native access to registers is required,
therefor 8bit read addressing (smbus) might not be suitable.

I can fix the function name, but in core this is a master read command.

>> ---
>>  drivers/i2c/i2c-core.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>  include/linux/i2c.h    |  4 ++++
>>  2 files changed, 44 insertions(+)
>>
>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>> index 069a41f..bccf83f 100644
>> --- a/drivers/i2c/i2c-core.c
>> +++ b/drivers/i2c/i2c-core.c
>> @@ -2980,6 +2980,46 @@ int i2c_slave_unregister(struct i2c_client *client)
>>  EXPORT_SYMBOL_GPL(i2c_slave_unregister);
>>  #endif
>>
>> +/**
>> + * i2c_addressed_read - writes read commands with desired length and read
>> + * desired length of data.
>> + * @client: Handle to slave device
>> + * @command: Pointer to command location
>> + * @cmd_len: Number of command bytes on command location
>> + * @data: Pointer to read data location
>> + * @data_len: Expected number of read bytes
>> + *
>> + * Addressed read on slave device when command is longer than byte. Returns
>> + * negative errno on error and 0 on success.
>> + */
>> +s32 i2c_addressed_read(const struct i2c_client *client, u8 *command,
>> +                    u16 cmd_len, u8 *data, u16 data_len)
>> +{
>> +     s32 status;
>> +     struct i2c_msg msg[2] = {
>> +             {
>> +                     .addr = client->addr,
>> +                     .flags = client->flags,
>> +                     .len = cmd_len,
>> +                     .buf = command,
>> +             },
>> +             {
>> +                     .addr = client->addr,
>> +                     .flags = client->flags | I2C_M_RD,
>> +                     .len = data_len,
>> +                     .buf = data,
>> +             },
>> +     };
>> +
>> +     status = i2c_transfer(client->adapter, msg, 2);
>> +
>> +     if (status < 0)
>> +             return status;
>> +
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL(i2c_addressed_read);
>> +
>>  MODULE_AUTHOR("Simon G. Vogl <simon@xxxxxxxxxxxxxxxxx>");
>>  MODULE_DESCRIPTION("I2C-Bus main module");
>>  MODULE_LICENSE("GPL");
>> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
>> index e83a738..d3cc1af 100644
>> --- a/include/linux/i2c.h
>> +++ b/include/linux/i2c.h
>> @@ -121,6 +121,10 @@ extern s32 i2c_smbus_read_i2c_block_data(const struct i2c_client *client,
>>  extern s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client,
>>                                         u8 command, u8 length,
>>                                         const u8 *values);
>> +
>> +s32 i2c_addressed_read(const struct i2c_client *client, u8 *command,
>> +                    u16 cmd_len, u8 *data, u16 data_len);
>> +
>>  #endif /* I2C */
>>
>>  /**
>> --
>> 2.1.4
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux