On 18/11/15 06:11, Matt Ranostay wrote:
> Optimize device tranactions using i2c transfers versus multiple
> possibly racey i2c_smbus_* function calls, and only one tranaction
> for distance measurement.
>
> Signed-off-by: Matt Ranostay <mranostay@xxxxxxxxx>
This is in principle fine, but...
It does require use of more generic I2C whereas previously we just needed
smbus. As such you need to check said support is available and should
have a fallback to smbus. Note you are correct in thinking this might
need some locking to be entirely safe.
However, you do hold the mlock (for other reasons) during the read so
that might effectively do the job. I haven't checked thorougly.
Jonathan
> ---
> drivers/iio/proximity/pulsedlight-lidar-lite-v2.c | 54 ++++++++++++-----------
> 1 file changed, 29 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/iio/proximity/pulsedlight-lidar-lite-v2.c b/drivers/iio/proximity/pulsedlight-lidar-lite-v2.c
> index e015a89..1df1a2f 100644
> --- a/drivers/iio/proximity/pulsedlight-lidar-lite-v2.c
> +++ b/drivers/iio/proximity/pulsedlight-lidar-lite-v2.c
> @@ -36,8 +36,7 @@
> #define LIDAR_REG_STATUS_INVALID BIT(3)
> #define LIDAR_REG_STATUS_READY BIT(0)
>
> -#define LIDAR_REG_DATA_HBYTE 0x0f
> -#define LIDAR_REG_DATA_LBYTE 0x10
> +#define LIDAR_REG_DATA 0x8f
> #define LIDAR_REG_PWR_CONTROL 0x65
>
> #define LIDAR_DRV_NAME "lidar"
> @@ -64,9 +63,10 @@ static const struct iio_chan_spec lidar_channels[] = {
> IIO_CHAN_SOFT_TIMESTAMP(1),
> };
>
> -static int lidar_read_byte(struct lidar_data *data, int reg)
> +static int lidar_i2c_xfer(struct lidar_data *data, u8 reg, u8 *val, int len)
> {
> struct i2c_client *client = data->client;
> + struct i2c_msg msg[2];
> int ret;
>
> /*
> @@ -74,17 +74,31 @@ static int lidar_read_byte(struct lidar_data *data, int reg)
> * so in turn i2c_smbus_read_byte_data cannot be used
> */
>
> - ret = i2c_smbus_write_byte(client, reg);
> - if (ret < 0) {
> - dev_err(&client->dev, "cannot write addr value");
> - return ret;
> - }
> + msg[0].addr = client->addr;
> + msg[0].flags = client->flags | I2C_M_STOP;
> + msg[0].len = 1;
> + msg[0].buf = (char *) ®
>
> - ret = i2c_smbus_read_byte(client);
> + msg[1].addr = client->addr;
> + msg[1].flags = client->flags | I2C_M_RD;
> + msg[1].len = len;
> + msg[1].buf = (char *) val;
> +
> + ret = i2c_transfer(client->adapter, msg, 2);
> +
> + return (ret == len) ? 0 : ret;
> +}
> +
> +static int lidar_read_byte(struct lidar_data *data, u8 reg)
> +{
> + int ret;
> + u8 val;
> +
> + ret = lidar_i2c_xfer(data, reg, &val, 1);
> if (ret < 0)
> - dev_err(&client->dev, "cannot read data value");
> + return ret;
>
> - return ret;
> + return val;
> }
>
> static inline int lidar_write_control(struct lidar_data *data, int val)
> @@ -100,22 +114,12 @@ static inline int lidar_write_power(struct lidar_data *data, int val)
>
> static int lidar_read_measurement(struct lidar_data *data, u16 *reg)
> {
> - int ret;
> - int val;
> -
> - ret = lidar_read_byte(data, LIDAR_REG_DATA_HBYTE);
> - if (ret < 0)
> - return ret;
> - val = ret << 8;
> + int ret = lidar_i2c_xfer(data, LIDAR_REG_DATA, (u8 *) reg, 2);
>
> - ret = lidar_read_byte(data, LIDAR_REG_DATA_LBYTE);
> - if (ret < 0)
> - return ret;
> -
> - val |= ret;
> - *reg = val;
> + if (!ret)
> + *reg = be16_to_cpu(*reg);
>
> - return 0;
> + return ret;
> }
>
> static int lidar_get_measurement(struct lidar_data *data, u16 *reg)
>
--
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
