Ok will note in the future.
Thanks,
Matt
On Wed, Dec 2, 2015 at 10:44 AM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On 02/12/15 04:59, Matt Ranostay wrote:
>> I just noticed... " return (ret == VZ89X_REG_MEASUREMENT_SIZE) ? 0 :
>> ret;" which is wrong it should be the count of the messages
>> executed....
>>
>> I need to see why this passed my testing :/
> As I'd only rebased a few mins ago and I doubt anyone pulled my tree
> in the meantime I did a bit of history editing and removed the original
> patch. Generally however, once a patch has been applied, I'd
> be looking for follow up additional patches rather than a new
> version of the original patch.
>
> Will deal with the new version at the weekend as this was just
> a flying visit to deal with merge issues.
>
> Jonathan
>>
>> On Sun, Nov 29, 2015 at 6:27 AM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>>> On 26/11/15 17:11, Matt Ranostay wrote:
>>>> Add an optimized i2c transfer reading function, and fallback
>>>> to racey smbus transfers if client->adapter doesn't support this.
>>>>
>>>> Signed-off-by: Matt Ranostay <mranostay@xxxxxxxxx>
>>> Few comments inline but nothing that stops me applying this.
>>>
>>> Applied to the togreg branch of iio.git - initially pushed out as testing.
>>> Note this branch will rebase soonish so don't base anything much on it!
>>>
>>> Jonathan
>>>> ---
>>>> drivers/iio/chemical/vz89x.c | 66 ++++++++++++++++++++++++++++++++++----------
>>>> 1 file changed, 52 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/chemical/vz89x.c b/drivers/iio/chemical/vz89x.c
>>>> index 11e59a5..c3367aa 100644
>>>> --- a/drivers/iio/chemical/vz89x.c
>>>> +++ b/drivers/iio/chemical/vz89x.c
>>>> @@ -34,8 +34,9 @@
>>>> struct vz89x_data {
>>>> struct i2c_client *client;
>>>> struct mutex lock;
>>>> - unsigned long last_update;
>>>> + int (*xfer)(struct vz89x_data *data, u8 cmd);
>>>>
>>>> + unsigned long last_update;
>>>> u8 buffer[VZ89X_REG_MEASUREMENT_SIZE];
>>>> };
>>>>
>>>> @@ -100,27 +101,60 @@ static int vz89x_measurement_is_valid(struct vz89x_data *data)
>>>> return !!(data->buffer[VZ89X_REG_MEASUREMENT_SIZE - 1] > 0);
>>>> }
>>>>
>>>> -static int vz89x_get_measurement(struct vz89x_data *data)
>>>> +static int vz89x_i2c_xfer(struct vz89x_data *data, u8 cmd)
>>>> {
>>>> + struct i2c_client *client = data->client;
>>>> + struct i2c_msg msg[2];
>>>> int ret;
>>>> - int i;
>>>> + u8 buf[3] = { cmd, 0, 0};
>>>>
>>>> - /* sensor can only be polled once a second max per datasheet */
>>>> - if (!time_after(jiffies, data->last_update + HZ))
>>>> - return 0;
>>>> + msg[0].addr = client->addr;
>>>> + msg[0].flags = client->flags;
>>>> + msg[0].len = 3;
>>>> + msg[0].buf = (char *) &buf;
>>>> +
>>>> + msg[1].addr = client->addr;
>>>> + msg[1].flags = client->flags | I2C_M_RD;
>>>> + msg[1].len = VZ89X_REG_MEASUREMENT_SIZE;
>>>> + msg[1].buf = (char *) &data->buffer;
>>>> +
>>>> + ret = i2c_transfer(client->adapter, msg, 2);
>>>>
>>>> - ret = i2c_smbus_write_word_data(data->client,
>>>> - VZ89X_REG_MEASUREMENT, 0);
>>>> + return (ret == VZ89X_REG_MEASUREMENT_SIZE) ? 0 : ret;
>>>> +}
>>>> +
>>>> +static int vz89x_smbus_xfer(struct vz89x_data *data, u8 cmd)
>>>> +{
>>>> + struct i2c_client *client = data->client;
>>> I wouldn't have bothered with the local variable, but it's just
>>> a matter of personal taste so lets leave it be!
>>>
>>>> + int ret;
>>>> + int i;
>>>> +
>>>> + ret = i2c_smbus_write_word_data(client, cmd, 0);
>>>> if (ret < 0)
>>>> return ret;
>>>>
>>>> for (i = 0; i < VZ89X_REG_MEASUREMENT_SIZE; i++) {
>>>> - ret = i2c_smbus_read_byte(data->client);
>>>> + ret = i2c_smbus_read_byte(client);
>>>> if (ret < 0)
>>>> return ret;
>>>> data->buffer[i] = ret;
>>>> }
>>>>
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int vz89x_get_measurement(struct vz89x_data *data)
>>>> +{
>>>> + int ret;
>>>> +
>>>> + /* sensor can only be polled once a second max per datasheet */
>>>> + if (!time_after(jiffies, data->last_update + HZ))
>>>> + return 0;
>>>> +
>>>> + ret = data->xfer(data, VZ89X_REG_MEASUREMENT);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> +
>>>> ret = vz89x_measurement_is_valid(data);
>>>> if (ret)
>>>> return -EAGAIN;
>>>> @@ -204,15 +238,19 @@ static int vz89x_probe(struct i2c_client *client,
>>>> struct iio_dev *indio_dev;
>>>> struct vz89x_data *data;
>>>>
>>>> - if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
>>>> - I2C_FUNC_SMBUS_BYTE))
>>>> - return -ENODEV;
>>>> -
>>>> indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>>>> if (!indio_dev)
>>>> return -ENOMEM;
>>>> -
>>> THis is a random white space change that snuck in - hardly crucial but would
>>> have been cleaner if this hadn't been in the patch.
>>>> data = iio_priv(indio_dev);
>>>> +
>>>> + if (i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
>>>> + data->xfer = vz89x_i2c_xfer;
>>>> + else if (i2c_check_functionality(client->adapter,
>>>> + I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BYTE))
>>>> + data->xfer = vz89x_smbus_xfer;
>>>> + else
>>>> + return -ENOTSUPP;
>>>> +
>>>> i2c_set_clientdata(client, indio_dev);
>>>> data->client = client;
>>>> data->last_update = jiffies - HZ;
>>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
