[PATCH 0/2] kxcjk-1013 driver optimizations

[PATCH 0/2] kxcjk-1013 driver optimizations

[Irina Tirdea]

Adds optimization of i2c transactions in trigger handler and
usage of available_scan_masks for kxcjk-1013.

Adriana Reus (2):
  iio: accel: kxcjk-1013: use available_scan_masks
  iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler

 drivers/iio/accel/kxcjk-1013.c | 50 ++++++++++++++++++++++++++++++++----------
 1 file changed, 39 insertions(+), 11 deletions(-)

-- 
1.9.1

[PATCH 1/2] iio: accel: kxcjk-1013: use available_scan_masks

[Irina Tirdea]

From: Adriana Reus <adriana.reus@intel.com>

Use available_scan_masks to allow the iio core to select
the data to send to userspace depending on which axes are
enabled, instead of doing this in the driver's interrupt
handler.

This also fixes the issue of accessing the buffer scan_mask
instead of active_scan_mask, since these might not be the
same due to client devices.

Signed-off-by: Adriana Reus <adriana.reus@intel.com>
Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 567de26..5f27787 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -115,6 +115,7 @@ enum kxcjk1013_axis {
 	AXIS_X,
 	AXIS_Y,
 	AXIS_Z,
+	AXIS_MAX,
 };
 
 enum kxcjk1013_mode {
@@ -947,6 +948,8 @@ static const struct iio_info kxcjk1013_info = {
 	.driver_module		= THIS_MODULE,
 };
 
+static const unsigned long kxcjk1013_scan_masks[] = {0x7, 0};
+
 static irqreturn_t kxcjk1013_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
@@ -955,9 +958,7 @@ static irqreturn_t kxcjk1013_trigger_handler(int irq, void *p)
 	int bit, ret, i = 0;
 
 	mutex_lock(&data->mutex);
-
-	for_each_set_bit(bit, indio_dev->buffer->scan_mask,
-			 indio_dev->masklength) {
+	for (bit = 0; bit < AXIS_MAX; bit++) {
 		ret = kxcjk1013_get_acc_reg(data, bit);
 		if (ret < 0) {
 			mutex_unlock(&data->mutex);
@@ -1228,6 +1229,7 @@ static int kxcjk1013_probe(struct i2c_client *client,
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->channels = kxcjk1013_channels;
 	indio_dev->num_channels = ARRAY_SIZE(kxcjk1013_channels);
+	indio_dev->available_scan_masks = kxcjk1013_scan_masks;
 	indio_dev->name = name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &kxcjk1013_info;
-- 
1.9.1

[PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler

[Irina Tirdea]

From: Adriana Reus <adriana.reus@intel.com>

Some i2c busses (e.g.: Synopsys DesignWare I2C adapter) need to
enable/disable the bus at each i2c transfer and must wait for
the enable/disable to happen before sending the data.

When reading data in the trigger handler, the kxcjk-1013 accel driver
does one i2c transfer for each axis. This has an impact on the
frequency of the accelerometer at high sample rates due to additional
delays introduced by the i2c bus at each transfer.

Reading all axis values in one i2c transfer reduces the delays
introduced by the i2c bus. In case i2c block read is not supported,
fallback to reading each axis as a separate word.

Signed-off-by: Adriana Reus <adriana.reus@intel.com>
Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 44 +++++++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 5f27787..bfa2899 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -109,6 +109,8 @@ struct kxcjk1013_data {
 	int64_t timestamp;
 	enum kx_chipset chipset;
 	bool is_smo8500_device;
+	s32 (*read_block_data)(const struct i2c_client *client, u8 command,
+			       u8 length, u8 *values);
 };
 
 enum kxcjk1013_axis {
@@ -216,6 +218,23 @@ static const struct {
 				 {800, 0, 0x06},
 				 {1600, 0, 0x06} };
 
+static s32 kxcjk1013_read_block_data(const struct i2c_client *client,
+				     u8 command, u8 length, u8 *values)
+{
+	s32 data;
+	u8 i;
+
+	for (i = 0; i < length; i += 2) {
+		data = i2c_smbus_read_word_data(client, command + i);
+		if (data < 0)
+			return data;
+
+		values[i] = data & 0xFF;
+		values[i+1] = data >> 8;
+	}
+	return i;
+}
+
 static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 			      enum kxcjk1013_mode mode)
 {
@@ -955,18 +974,14 @@ static irqreturn_t kxcjk1013_trigger_handler(int irq, void *p)
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
-	int bit, ret, i = 0;
+	int ret;
 
 	mutex_lock(&data->mutex);
-	for (bit = 0; bit < AXIS_MAX; bit++) {
-		ret = kxcjk1013_get_acc_reg(data, bit);
-		if (ret < 0) {
-			mutex_unlock(&data->mutex);
-			goto err;
-		}
-		data->buffer[i++] = ret;
-	}
+	ret = data->read_block_data(data->client, KXCJK1013_REG_XOUT_L,
+				    AXIS_MAX * 2, (u8 *) data->buffer);
 	mutex_unlock(&data->mutex);
+	if (ret < 0)
+		goto err;
 
 	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
 					   data->timestamp);
@@ -1196,6 +1211,11 @@ static int kxcjk1013_probe(struct i2c_client *client,
 	const char *name;
 	int ret;
 
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_BYTE_DATA |
+				     I2C_FUNC_SMBUS_READ_WORD_DATA))
+		return -ENODEV;
+
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
@@ -1204,6 +1224,12 @@ static int kxcjk1013_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, indio_dev);
 	data->client = client;
 
+	if (i2c_check_functionality(client->adapter,
+				    I2C_FUNC_SMBUS_READ_I2C_BLOCK))
+		data->read_block_data = i2c_smbus_read_i2c_block_data;
+	else
+		data->read_block_data = kxcjk1013_read_block_data;
+
 	pdata = dev_get_platdata(&client->dev);
 	if (pdata)
 		data->active_high_intr = pdata->active_high_intr;
-- 
1.9.1

Re: [PATCH 0/2] kxcjk-1013 driver optimizations

[Peter Meerwald]

> Adds optimization of i2c transactions in trigger handler and
> usage of available_scan_masks for kxcjk-1013.

the 'optimization' changes the capabilities of the driver, reading 
individual channel is not possible anymore

> Adriana Reus (2):
>   iio: accel: kxcjk-1013: use available_scan_masks
>   iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler
> 
>  drivers/iio/accel/kxcjk-1013.c | 50 ++++++++++++++++++++++++++++++++----------
>  1 file changed, 39 insertions(+), 11 deletions(-)
> 
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler

[Peter Meerwald]

> Reading all axis values in one i2c transfer reduces the delays
> introduced by the i2c bus. In case i2c block read is not supported,
> fallback to reading each axis as a separate word.

see comments inline below
 
> Signed-off-by: Adriana Reus <adriana.reus@intel.com>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  drivers/iio/accel/kxcjk-1013.c | 44 +++++++++++++++++++++++++++++++++---------
>  1 file changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 5f27787..bfa2899 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -109,6 +109,8 @@ struct kxcjk1013_data {
>  	int64_t timestamp;
>  	enum kx_chipset chipset;
>  	bool is_smo8500_device;
> +	s32 (*read_block_data)(const struct i2c_client *client, u8 command,
> +			       u8 length, u8 *values);

probably this could/should be done in the i2c layer or we end up adding a 
similar function in every driver?

>  };
>  
>  enum kxcjk1013_axis {
> @@ -216,6 +218,23 @@ static const struct {
>  				 {800, 0, 0x06},
>  				 {1600, 0, 0x06} };
>  
> +static s32 kxcjk1013_read_block_data(const struct i2c_client *client,
> +				     u8 command, u8 length, u8 *values)
> +{
> +	s32 data;
> +	u8 i;
> +
> +	for (i = 0; i < length; i += 2) {
> +		data = i2c_smbus_read_word_data(client, command + i);
> +		if (data < 0)
> +			return data;
> +
> +		values[i] = data & 0xFF;
> +		values[i+1] = data >> 8;

this is incorrect; it forces the data to be little endian, however, the 
endianness (as specified in the driver's .scan_type) is IIO_CPU -- the 
code breaks for big-endian CPUs

since _read_i2c_block_data() can't do endianness conversion (and the chip 
does i2c endianness, i.e. little-endian), the .scan_type should become 
IIO_LE and above code is correct again but still ugly :)

bottom line: change .scan_type to IIO_LE

> +	}
> +	return i;
> +}
> +
>  static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
>  			      enum kxcjk1013_mode mode)
>  {
> @@ -955,18 +974,14 @@ static irqreturn_t kxcjk1013_trigger_handler(int irq, void *p)
>  	struct iio_poll_func *pf = p;
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct kxcjk1013_data *data = iio_priv(indio_dev);
> -	int bit, ret, i = 0;
> +	int ret;
>  
>  	mutex_lock(&data->mutex);
> -	for (bit = 0; bit < AXIS_MAX; bit++) {
> -		ret = kxcjk1013_get_acc_reg(data, bit);
> -		if (ret < 0) {
> -			mutex_unlock(&data->mutex);
> -			goto err;
> -		}
> -		data->buffer[i++] = ret;
> -	}
> +	ret = data->read_block_data(data->client, KXCJK1013_REG_XOUT_L,
> +				    AXIS_MAX * 2, (u8 *) data->buffer);
>  	mutex_unlock(&data->mutex);
> +	if (ret < 0)
> +		goto err;
>  
>  	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
>  					   data->timestamp);
> @@ -1196,6 +1211,11 @@ static int kxcjk1013_probe(struct i2c_client *client,
>  	const char *name;
>  	int ret;
>  
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_BYTE_DATA |
> +				     I2C_FUNC_SMBUS_READ_WORD_DATA))
> +		return -ENODEV;
> +
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>  	if (!indio_dev)
>  		return -ENOMEM;
> @@ -1204,6 +1224,12 @@ static int kxcjk1013_probe(struct i2c_client *client,
>  	i2c_set_clientdata(client, indio_dev);
>  	data->client = client;
>  
> +	if (i2c_check_functionality(client->adapter,
> +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK))
> +		data->read_block_data = i2c_smbus_read_i2c_block_data;
> +	else
> +		data->read_block_data = kxcjk1013_read_block_data;
> +
>  	pdata = dev_get_platdata(&client->dev);
>  	if (pdata)
>  		data->active_high_intr = pdata->active_high_intr;
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

RE: [PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler

[Tirdea, Irina]

> -----Original Message-----
> From: Peter Meerwald [mailto:pmeerw@pmeerw.net]
> Sent: 16 February, 2015 21:26
> To: Tirdea, Irina
> Cc: Jonathan Cameron; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Reus, Adriana
> Subject: Re: [PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler
> 
> 
> > Reading all axis values in one i2c transfer reduces the delays
> > introduced by the i2c bus. In case i2c block read is not supported,
> > fallback to reading each axis as a separate word.
> 
> see comments inline below
> 
Thanks for the review, Peter!

> > Signed-off-by: Adriana Reus <adriana.reus@intel.com>
> > Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> > Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > ---
> >  drivers/iio/accel/kxcjk-1013.c | 44 +++++++++++++++++++++++++++++++++---------
> >  1 file changed, 35 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> > index 5f27787..bfa2899 100644
> > --- a/drivers/iio/accel/kxcjk-1013.c
> > +++ b/drivers/iio/accel/kxcjk-1013.c
> > @@ -109,6 +109,8 @@ struct kxcjk1013_data {
> >  	int64_t timestamp;
> >  	enum kx_chipset chipset;
> >  	bool is_smo8500_device;
> > +	s32 (*read_block_data)(const struct i2c_client *client, u8 command,
> > +			       u8 length, u8 *values);
> 
> probably this could/should be done in the i2c layer or we end up adding a
> similar function in every driver?
> 
Actually this is exactly what I did: adding this code in a couple of drivers :)
You are right, this belongs to the i2c core. Will move it there.

> >  };
> >
> >  enum kxcjk1013_axis {
> > @@ -216,6 +218,23 @@ static const struct {
> >  				 {800, 0, 0x06},
> >  				 {1600, 0, 0x06} };
> >
> > +static s32 kxcjk1013_read_block_data(const struct i2c_client *client,
> > +				     u8 command, u8 length, u8 *values)
> > +{
> > +	s32 data;
> > +	u8 i;
> > +
> > +	for (i = 0; i < length; i += 2) {
> > +		data = i2c_smbus_read_word_data(client, command + i);
> > +		if (data < 0)
> > +			return data;
> > +
> > +		values[i] = data & 0xFF;
> > +		values[i+1] = data >> 8;
> 
> this is incorrect; it forces the data to be little endian, however, the
> endianness (as specified in the driver's .scan_type) is IIO_CPU -- the
> code breaks for big-endian CPUs
> 
> since _read_i2c_block_data() can't do endianness conversion (and the chip
> does i2c endianness, i.e. little-endian), the .scan_type should become
> IIO_LE and above code is correct again but still ugly :)
> 
> bottom line: change .scan_type to IIO_LE
> 
Good point. Changing the endianess to IIO_LE is correct for either kxcjk1013_read_block_data or i2c_smbus_read_i2c_block_data.
Will fix this in the next version. Thanks for catching this!

> > +	}
> > +	return i;
> > +}
> > +
> >  static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
> >  			      enum kxcjk1013_mode mode)
> >  {
> > @@ -955,18 +974,14 @@ static irqreturn_t kxcjk1013_trigger_handler(int irq, void *p)
> >  	struct iio_poll_func *pf = p;
> >  	struct iio_dev *indio_dev = pf->indio_dev;
> >  	struct kxcjk1013_data *data = iio_priv(indio_dev);
> > -	int bit, ret, i = 0;
> > +	int ret;
> >
> >  	mutex_lock(&data->mutex);
> > -	for (bit = 0; bit < AXIS_MAX; bit++) {
> > -		ret = kxcjk1013_get_acc_reg(data, bit);
> > -		if (ret < 0) {
> > -			mutex_unlock(&data->mutex);
> > -			goto err;
> > -		}
> > -		data->buffer[i++] = ret;
> > -	}
> > +	ret = data->read_block_data(data->client, KXCJK1013_REG_XOUT_L,
> > +				    AXIS_MAX * 2, (u8 *) data->buffer);
> >  	mutex_unlock(&data->mutex);
> > +	if (ret < 0)
> > +		goto err;
> >
> >  	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> >  					   data->timestamp);
> > @@ -1196,6 +1211,11 @@ static int kxcjk1013_probe(struct i2c_client *client,
> >  	const char *name;
> >  	int ret;
> >
> > +	if (!i2c_check_functionality(client->adapter,
> > +				     I2C_FUNC_SMBUS_BYTE_DATA |
> > +				     I2C_FUNC_SMBUS_READ_WORD_DATA))
> > +		return -ENODEV;
> > +
> >  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> >  	if (!indio_dev)
> >  		return -ENOMEM;
> > @@ -1204,6 +1224,12 @@ static int kxcjk1013_probe(struct i2c_client *client,
> >  	i2c_set_clientdata(client, indio_dev);
> >  	data->client = client;
> >
> > +	if (i2c_check_functionality(client->adapter,
> > +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK))
> > +		data->read_block_data = i2c_smbus_read_i2c_block_data;
> > +	else
> > +		data->read_block_data = kxcjk1013_read_block_data;
> > +
> >  	pdata = dev_get_platdata(&client->dev);
> >  	if (pdata)
> >  		data->active_high_intr = pdata->active_high_intr;
> >
> 
> --
> 
> Peter Meerwald
> +43-664-2444418 (mobile)

RE: [PATCH 0/2] kxcjk-1013 driver optimizations

[Tirdea, Irina]

> -----Original Message-----
> From: Peter Meerwald [mailto:pmeerw@pmeerw.net]
> Sent: 16 February, 2015 21:14
> To: Tirdea, Irina
> Cc: Jonathan Cameron; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Reus, Adriana
> Subject: Re: [PATCH 0/2] kxcjk-1013 driver optimizations
> 
> 
> > Adds optimization of i2c transactions in trigger handler and
> > usage of available_scan_masks for kxcjk-1013.
> 
> the 'optimization' changes the capabilities of the driver, reading
> individual channel is not possible anymore
> 
We can still read individual channels, just that the driver does not see that anymore and the demux is handled by the iio core.
available_scan_masks contains the bitmap superset for all channels we can enable (I will remove the 0 available_scan_mask as it does not makes sense).
When a new channel is enabled, the iio core checks if the new mask is a subset of any entry in available_scan_masks. When the buffer is pushed from the driver, the iio core will select only the enabled channel values.

> > Adriana Reus (2):
> >   iio: accel: kxcjk-1013: use available_scan_masks
> >   iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler
> >
> >  drivers/iio/accel/kxcjk-1013.c | 50 ++++++++++++++++++++++++++++++++----------
> >  1 file changed, 39 insertions(+), 11 deletions(-)
> >
> >
> 
> --
> 
> Peter Meerwald
> +43-664-2444418 (mobile)

Re: [PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler

[Pandruvada, Srinivas]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 5391 bytes --]

On Fri, 2015-02-20 at 12:02 +0000, Tirdea, Irina wrote:
> 
> > -----Original Message-----
> > From: Peter Meerwald [mailto:pmeerw@pmeerw.net]
> > Sent: 16 February, 2015 21:26
> > To: Tirdea, Irina
> > Cc: Jonathan Cameron; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Reus, Adriana
> > Subject: Re: [PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler
> > 
> > 
> > > Reading all axis values in one i2c transfer reduces the delays
> > > introduced by the i2c bus. In case i2c block read is not supported,
> > > fallback to reading each axis as a separate word.
> > 
> > see comments inline below
> > 
> Thanks for the review, Peter!
> 
> > > Signed-off-by: Adriana Reus <adriana.reus@intel.com>
> > > Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> > > Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > > ---
> > >  drivers/iio/accel/kxcjk-1013.c | 44 +++++++++++++++++++++++++++++++++---------
> > >  1 file changed, 35 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> > > index 5f27787..bfa2899 100644
> > > --- a/drivers/iio/accel/kxcjk-1013.c
> > > +++ b/drivers/iio/accel/kxcjk-1013.c
> > > @@ -109,6 +109,8 @@ struct kxcjk1013_data {
> > >  	int64_t timestamp;
> > >  	enum kx_chipset chipset;
> > >  	bool is_smo8500_device;
> > > +	s32 (*read_block_data)(const struct i2c_client *client, u8 command,
> > > +			       u8 length, u8 *values);
> > 
> > probably this could/should be done in the i2c layer or we end up adding a
> > similar function in every driver?
> > 
> Actually this is exactly what I did: adding this code in a couple of drivers :)
> You are right, this belongs to the i2c core. Will move it there.
> 
> > >  };
> > >
> > >  enum kxcjk1013_axis {
> > > @@ -216,6 +218,23 @@ static const struct {
> > >  				 {800, 0, 0x06},
> > >  				 {1600, 0, 0x06} };
> > >
> > > +static s32 kxcjk1013_read_block_data(const struct i2c_client *client,
> > > +				     u8 command, u8 length, u8 *values)
> > > +{
> > > +	s32 data;
> > > +	u8 i;
> > > +
> > > +	for (i = 0; i < length; i += 2) {
> > > +		data = i2c_smbus_read_word_data(client, command + i);
> > > +		if (data < 0)
> > > +			return data;
> > > +
> > > +		values[i] = data & 0xFF;
> > > +		values[i+1] = data >> 8;
> > 
> > this is incorrect; it forces the data to be little endian, however, the
> > endianness (as specified in the driver's .scan_type) is IIO_CPU -- the
> > code breaks for big-endian CPUs
> > 
> > since _read_i2c_block_data() can't do endianness conversion (and the chip
> > does i2c endianness, i.e. little-endian), the .scan_type should become
> > IIO_LE and above code is correct again but still ugly :)
> > 
> > bottom line: change .scan_type to IIO_LE
> > 
> Good point. Changing the endianess to IIO_LE is correct for either kxcjk1013_read_block_data or i2c_smbus_read_i2c_block_data.
> Will fix this in the next version. Thanks for catching this!
> 
I don't think changing to IIO_LE is good idea as when i2c_read_bock..
then the scan type will be CPU. So better to fix endianness in this
function.

> > > +	}
> > > +	return i;
> > > +}
> > > +
> > >  static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
> > >  			      enum kxcjk1013_mode mode)
> > >  {
> > > @@ -955,18 +974,14 @@ static irqreturn_t kxcjk1013_trigger_handler(int irq, void *p)
> > >  	struct iio_poll_func *pf = p;
> > >  	struct iio_dev *indio_dev = pf->indio_dev;
> > >  	struct kxcjk1013_data *data = iio_priv(indio_dev);
> > > -	int bit, ret, i = 0;
> > > +	int ret;
> > >
> > >  	mutex_lock(&data->mutex);
> > > -	for (bit = 0; bit < AXIS_MAX; bit++) {
> > > -		ret = kxcjk1013_get_acc_reg(data, bit);
> > > -		if (ret < 0) {
> > > -			mutex_unlock(&data->mutex);
> > > -			goto err;
> > > -		}
> > > -		data->buffer[i++] = ret;
> > > -	}
> > > +	ret = data->read_block_data(data->client, KXCJK1013_REG_XOUT_L,
> > > +				    AXIS_MAX * 2, (u8 *) data->buffer);
> > >  	mutex_unlock(&data->mutex);
> > > +	if (ret < 0)
> > > +		goto err;
> > >
> > >  	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> > >  					   data->timestamp);
> > > @@ -1196,6 +1211,11 @@ static int kxcjk1013_probe(struct i2c_client *client,
> > >  	const char *name;
> > >  	int ret;
> > >
> > > +	if (!i2c_check_functionality(client->adapter,
> > > +				     I2C_FUNC_SMBUS_BYTE_DATA |
> > > +				     I2C_FUNC_SMBUS_READ_WORD_DATA))
> > > +		return -ENODEV;
> > > +
> > >  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> > >  	if (!indio_dev)
> > >  		return -ENOMEM;
> > > @@ -1204,6 +1224,12 @@ static int kxcjk1013_probe(struct i2c_client *client,
> > >  	i2c_set_clientdata(client, indio_dev);
> > >  	data->client = client;
> > >
> > > +	if (i2c_check_functionality(client->adapter,
> > > +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK))
> > > +		data->read_block_data = i2c_smbus_read_i2c_block_data;
> > > +	else
> > > +		data->read_block_data = kxcjk1013_read_block_data;
> > > +
> > >  	pdata = dev_get_platdata(&client->dev);
> > >  	if (pdata)
> > >  		data->active_high_intr = pdata->active_high_intr;
> > >
> > 
> > --
> > 
> > Peter Meerwald
> > +43-664-2444418 (mobile)

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler

[Peter Meerwald]

> > > > +
> > > > +		values[i] = data & 0xFF;
> > > > +		values[i+1] = data >> 8;
> > > 
> > > this is incorrect; it forces the data to be little endian, however, the
> > > endianness (as specified in the driver's .scan_type) is IIO_CPU -- the
> > > code breaks for big-endian CPUs
> > > 
> > > since _read_i2c_block_data() can't do endianness conversion (and the chip
> > > does i2c endianness, i.e. little-endian), the .scan_type should become
> > > IIO_LE and above code is correct again but still ugly :)
> > > 
> > > bottom line: change .scan_type to IIO_LE
> > > 
> > Good point. Changing the endianess to IIO_LE is correct for either kxcjk1013_read_block_data or i2c_smbus_read_i2c_block_data.
> > Will fix this in the next version. Thanks for catching this!
> > 
> I don't think changing to IIO_LE is good idea as when i2c_read_bock..
> then the scan type will be CPU. So better to fix endianness in this
> function.

the chip has little-endian data registers; i2c_read_block() just transfers 
the data (no endianness conversion), so the data will still be 
little-endian

p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler

[Pandruvada, Srinivas]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1372 bytes --]

On Fri, 2015-02-20 at 16:23 +0100, Peter Meerwald wrote:
> > > > > +
> > > > > +		values[i] = data & 0xFF;
> > > > > +		values[i+1] = data >> 8;
> > > > 
> > > > this is incorrect; it forces the data to be little endian, however, the
> > > > endianness (as specified in the driver's .scan_type) is IIO_CPU -- the
> > > > code breaks for big-endian CPUs
> > > > 
> > > > since _read_i2c_block_data() can't do endianness conversion (and the chip
> > > > does i2c endianness, i.e. little-endian), the .scan_type should become
> > > > IIO_LE and above code is correct again but still ugly :)
> > > > 
> > > > bottom line: change .scan_type to IIO_LE
> > > > 
> > > Good point. Changing the endianess to IIO_LE is correct for either kxcjk1013_read_block_data or i2c_smbus_read_i2c_block_data.
> > > Will fix this in the next version. Thanks for catching this!
> > > 
> > I don't think changing to IIO_LE is good idea as when i2c_read_bock..
> > then the scan type will be CPU. So better to fix endianness in this
> > function.
> 
> the chip has little-endian data registers; i2c_read_block() just transfers 
> the data (no endianness conversion), so the data will still be 
> little-endian
You are right.
> 
> p.
> 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH 0/2] kxcjk-1013 driver optimizations

[Jonathan Cameron]

On 20/02/15 12:03, Tirdea, Irina wrote:
> 
> 
>> -----Original Message-----
>> From: Peter Meerwald [mailto:pmeerw@pmeerw.net]
>> Sent: 16 February, 2015 21:14
>> To: Tirdea, Irina
>> Cc: Jonathan Cameron; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Reus, Adriana
>> Subject: Re: [PATCH 0/2] kxcjk-1013 driver optimizations
>>
>>
>>> Adds optimization of i2c transactions in trigger handler and
>>> usage of available_scan_masks for kxcjk-1013.
>>
>> the 'optimization' changes the capabilities of the driver, reading
>> individual channel is not possible anymore
>>
> We can still read individual channels, just that the driver does not see that anymore and the demux is handled by the iio core.
> available_scan_masks contains the bitmap superset for all channels we can enable (I will remove the 0 available_scan_mask as it does not makes sense).
Careful. That's there as a null terminator for this list of available scan masks.
> When a new channel is enabled, the iio core checks if the new mask is a subset of any entry in available_scan_masks. 
> When the buffer is pushed from the driver, the iio core will select only the enabled channel values.

Pretty much.  IIRC This originally went in as part of handling multiple clients on these
data streams, but it was rapidly pointed out that lots of drivers spun their own
versions to break out particular elements from a multichannel read.
As such, using the generic form saved on repetition in these cases as well.
A good, complex example is drivers/iio/adc/max1363 in which really weird combinations
of channels (but far from all) are available for bulk capture and reading.
> 
>>> Adriana Reus (2):
>>>   iio: accel: kxcjk-1013: use available_scan_masks
>>>   iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler
>>>
>>>  drivers/iio/accel/kxcjk-1013.c | 50 ++++++++++++++++++++++++++++++++----------
>>>  1 file changed, 39 insertions(+), 11 deletions(-)
>>>
>>>
>>
>> --
>>
>> Peter Meerwald
>> +43-664-2444418 (mobile)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler

[Jonathan Cameron]

On 20/02/15 12:02, Tirdea, Irina wrote:
> 
> 
>> -----Original Message-----
>> From: Peter Meerwald [mailto:pmeerw@pmeerw.net]
>> Sent: 16 February, 2015 21:26
>> To: Tirdea, Irina
>> Cc: Jonathan Cameron; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Reus, Adriana
>> Subject: Re: [PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler
>>
>>
>>> Reading all axis values in one i2c transfer reduces the delays
>>> introduced by the i2c bus. In case i2c block read is not supported,
>>> fallback to reading each axis as a separate word.
>>
>> see comments inline below
>>
> Thanks for the review, Peter!
> 
>>> Signed-off-by: Adriana Reus <adriana.reus@intel.com>
>>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
>>> Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>>> ---
>>>  drivers/iio/accel/kxcjk-1013.c | 44 +++++++++++++++++++++++++++++++++---------
>>>  1 file changed, 35 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
>>> index 5f27787..bfa2899 100644
>>> --- a/drivers/iio/accel/kxcjk-1013.c
>>> +++ b/drivers/iio/accel/kxcjk-1013.c
>>> @@ -109,6 +109,8 @@ struct kxcjk1013_data {
>>>  	int64_t timestamp;
>>>  	enum kx_chipset chipset;
>>>  	bool is_smo8500_device;
>>> +	s32 (*read_block_data)(const struct i2c_client *client, u8 command,
>>> +			       u8 length, u8 *values);
>>
>> probably this could/should be done in the i2c layer or we end up adding a
>> similar function in every driver?
>>
> Actually this is exactly what I did: adding this code in a couple of drivers :)
> You are right, this belongs to the i2c core. Will move it there.
A good idea.  Might be possible to make this a transparent fallback so no
special handling is needed in drivers at all (e.g. emulate the block read using
the biggest read that is available) - taking care about the endian fun and
games that results.

Propose it to Wolfram and the i2c list and see what people think of it.

Jonathan
> 
>>>  };
>>>
>>>  enum kxcjk1013_axis {
>>> @@ -216,6 +218,23 @@ static const struct {
>>>  				 {800, 0, 0x06},
>>>  				 {1600, 0, 0x06} };
>>>
>>> +static s32 kxcjk1013_read_block_data(const struct i2c_client *client,
>>> +				     u8 command, u8 length, u8 *values)
>>> +{
>>> +	s32 data;
>>> +	u8 i;
>>> +
>>> +	for (i = 0; i < length; i += 2) {
>>> +		data = i2c_smbus_read_word_data(client, command + i);
>>> +		if (data < 0)
>>> +			return data;
>>> +
>>> +		values[i] = data & 0xFF;
>>> +		values[i+1] = data >> 8;
>>
>> this is incorrect; it forces the data to be little endian, however, the
>> endianness (as specified in the driver's .scan_type) is IIO_CPU -- the
>> code breaks for big-endian CPUs
>>
>> since _read_i2c_block_data() can't do endianness conversion (and the chip
>> does i2c endianness, i.e. little-endian), the .scan_type should become
>> IIO_LE and above code is correct again but still ugly :)
>>
>> bottom line: change .scan_type to IIO_LE
>>
> Good point. Changing the endianess to IIO_LE is correct for either kxcjk1013_read_block_data or i2c_smbus_read_i2c_block_data.
> Will fix this in the next version. Thanks for catching this!
> 
>>> +	}
>>> +	return i;
>>> +}
>>> +
>>>  static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
>>>  			      enum kxcjk1013_mode mode)
>>>  {
>>> @@ -955,18 +974,14 @@ static irqreturn_t kxcjk1013_trigger_handler(int irq, void *p)
>>>  	struct iio_poll_func *pf = p;
>>>  	struct iio_dev *indio_dev = pf->indio_dev;
>>>  	struct kxcjk1013_data *data = iio_priv(indio_dev);
>>> -	int bit, ret, i = 0;
>>> +	int ret;
>>>
>>>  	mutex_lock(&data->mutex);
>>> -	for (bit = 0; bit < AXIS_MAX; bit++) {
>>> -		ret = kxcjk1013_get_acc_reg(data, bit);
>>> -		if (ret < 0) {
>>> -			mutex_unlock(&data->mutex);
>>> -			goto err;
>>> -		}
>>> -		data->buffer[i++] = ret;
>>> -	}
>>> +	ret = data->read_block_data(data->client, KXCJK1013_REG_XOUT_L,
>>> +				    AXIS_MAX * 2, (u8 *) data->buffer);
>>>  	mutex_unlock(&data->mutex);
>>> +	if (ret < 0)
>>> +		goto err;
>>>
>>>  	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
>>>  					   data->timestamp);
>>> @@ -1196,6 +1211,11 @@ static int kxcjk1013_probe(struct i2c_client *client,
>>>  	const char *name;
>>>  	int ret;
>>>
>>> +	if (!i2c_check_functionality(client->adapter,
>>> +				     I2C_FUNC_SMBUS_BYTE_DATA |
>>> +				     I2C_FUNC_SMBUS_READ_WORD_DATA))
>>> +		return -ENODEV;
>>> +
>>>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>>>  	if (!indio_dev)
>>>  		return -ENOMEM;
>>> @@ -1204,6 +1224,12 @@ static int kxcjk1013_probe(struct i2c_client *client,
>>>  	i2c_set_clientdata(client, indio_dev);
>>>  	data->client = client;
>>>
>>> +	if (i2c_check_functionality(client->adapter,
>>> +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK))
>>> +		data->read_block_data = i2c_smbus_read_i2c_block_data;
>>> +	else
>>> +		data->read_block_data = kxcjk1013_read_block_data;
>>> +
>>>  	pdata = dev_get_platdata(&client->dev);
>>>  	if (pdata)
>>>  		data->active_high_intr = pdata->active_high_intr;
>>>
>>
>> --
>>
>> Peter Meerwald
>> +43-664-2444418 (mobile)

RE: [PATCH 0/2] kxcjk-1013 driver optimizations

[Tirdea, Irina]

> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: 21 February, 2015 20:27
> To: Tirdea, Irina; Peter Meerwald
> Cc: linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Reus, Adriana
> Subject: Re: [PATCH 0/2] kxcjk-1013 driver optimizations
> 
> On 20/02/15 12:03, Tirdea, Irina wrote:
> >
> >
> >> -----Original Message-----
> >> From: Peter Meerwald [mailto:pmeerw@pmeerw.net]
> >> Sent: 16 February, 2015 21:14
> >> To: Tirdea, Irina
> >> Cc: Jonathan Cameron; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Reus, Adriana
> >> Subject: Re: [PATCH 0/2] kxcjk-1013 driver optimizations
> >>
> >>
> >>> Adds optimization of i2c transactions in trigger handler and
> >>> usage of available_scan_masks for kxcjk-1013.
> >>
> >> the 'optimization' changes the capabilities of the driver, reading
> >> individual channel is not possible anymore
> >>
> > We can still read individual channels, just that the driver does not see that anymore and the demux is handled by the iio core.
> > available_scan_masks contains the bitmap superset for all channels we can enable (I will remove the 0 available_scan_mask as it
> does not makes sense).
> Careful. That's there as a null terminator for this list of available scan masks.
Missed this. You are right, thanks! Seems it's good as is.

> > When a new channel is enabled, the iio core checks if the new mask is a subset of any entry in available_scan_masks.
> > When the buffer is pushed from the driver, the iio core will select only the enabled channel values.
> 
> Pretty much.  IIRC This originally went in as part of handling multiple clients on these
> data streams, but it was rapidly pointed out that lots of drivers spun their own
> versions to break out particular elements from a multichannel read.
> As such, using the generic form saved on repetition in these cases as well.
> A good, complex example is drivers/iio/adc/max1363 in which really weird combinations
> of channels (but far from all) are available for bulk capture and reading.
> >
> >>> Adriana Reus (2):
> >>>   iio: accel: kxcjk-1013: use available_scan_masks
> >>>   iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler
> >>>
> >>>  drivers/iio/accel/kxcjk-1013.c | 50 ++++++++++++++++++++++++++++++++----------
> >>>  1 file changed, 39 insertions(+), 11 deletions(-)
> >>>
> >>>
> >>
> >> --
> >>
> >> Peter Meerwald
> >> +43-664-2444418 (mobile)
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >

RE: [PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler

[Tirdea, Irina]

> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: 21 February, 2015 20:30
> To: Tirdea, Irina; Peter Meerwald
> Cc: linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Reus, Adriana
> Subject: Re: [PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler
> 
> On 20/02/15 12:02, Tirdea, Irina wrote:
> >
> >
> >> -----Original Message-----
> >> From: Peter Meerwald [mailto:pmeerw@pmeerw.net]
> >> Sent: 16 February, 2015 21:26
> >> To: Tirdea, Irina
> >> Cc: Jonathan Cameron; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Reus, Adriana
> >> Subject: Re: [PATCH 2/2] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler
> >>
> >>
> >>> Reading all axis values in one i2c transfer reduces the delays
> >>> introduced by the i2c bus. In case i2c block read is not supported,
> >>> fallback to reading each axis as a separate word.
> >>
> >> see comments inline below
> >>
> > Thanks for the review, Peter!
> >
> >>> Signed-off-by: Adriana Reus <adriana.reus@intel.com>
> >>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> >>> Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> >>> ---
> >>>  drivers/iio/accel/kxcjk-1013.c | 44 +++++++++++++++++++++++++++++++++---------
> >>>  1 file changed, 35 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> >>> index 5f27787..bfa2899 100644
> >>> --- a/drivers/iio/accel/kxcjk-1013.c
> >>> +++ b/drivers/iio/accel/kxcjk-1013.c
> >>> @@ -109,6 +109,8 @@ struct kxcjk1013_data {
> >>>  	int64_t timestamp;
> >>>  	enum kx_chipset chipset;
> >>>  	bool is_smo8500_device;
> >>> +	s32 (*read_block_data)(const struct i2c_client *client, u8 command,
> >>> +			       u8 length, u8 *values);
> >>
> >> probably this could/should be done in the i2c layer or we end up adding a
> >> similar function in every driver?
> >>
> > Actually this is exactly what I did: adding this code in a couple of drivers :)
> > You are right, this belongs to the i2c core. Will move it there.
> A good idea.  Might be possible to make this a transparent fallback so no
> special handling is needed in drivers at all (e.g. emulate the block read using
> the biggest read that is available) - taking care about the endian fun and
> games that results.
> 
> Propose it to Wolfram and the i2c list and see what people think of it.
> 
I'll do the block emulation code first and send it on the i2c list (still checking some endianness issues). 
Once I'll have a response for the i2c part, I'll resend these patchsets with the required fixes.

Thanks,
Irina

> Jonathan
> >
> >>>  };
> >>>
> >>>  enum kxcjk1013_axis {
> >>> @@ -216,6 +218,23 @@ static const struct {
> >>>  				 {800, 0, 0x06},
> >>>  				 {1600, 0, 0x06} };
> >>>
> >>> +static s32 kxcjk1013_read_block_data(const struct i2c_client *client,
> >>> +				     u8 command, u8 length, u8 *values)
> >>> +{
> >>> +	s32 data;
> >>> +	u8 i;
> >>> +
> >>> +	for (i = 0; i < length; i += 2) {
> >>> +		data = i2c_smbus_read_word_data(client, command + i);
> >>> +		if (data < 0)
> >>> +			return data;
> >>> +
> >>> +		values[i] = data & 0xFF;
> >>> +		values[i+1] = data >> 8;
> >>
> >> this is incorrect; it forces the data to be little endian, however, the
> >> endianness (as specified in the driver's .scan_type) is IIO_CPU -- the
> >> code breaks for big-endian CPUs
> >>
> >> since _read_i2c_block_data() can't do endianness conversion (and the chip
> >> does i2c endianness, i.e. little-endian), the .scan_type should become
> >> IIO_LE and above code is correct again but still ugly :)
> >>
> >> bottom line: change .scan_type to IIO_LE
> >>
> > Good point. Changing the endianess to IIO_LE is correct for either kxcjk1013_read_block_data or i2c_smbus_read_i2c_block_data.
> > Will fix this in the next version. Thanks for catching this!
> >
> >>> +	}
> >>> +	return i;
> >>> +}
> >>> +
> >>>  static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
> >>>  			      enum kxcjk1013_mode mode)
> >>>  {
> >>> @@ -955,18 +974,14 @@ static irqreturn_t kxcjk1013_trigger_handler(int irq, void *p)
> >>>  	struct iio_poll_func *pf = p;
> >>>  	struct iio_dev *indio_dev = pf->indio_dev;
> >>>  	struct kxcjk1013_data *data = iio_priv(indio_dev);
> >>> -	int bit, ret, i = 0;
> >>> +	int ret;
> >>>
> >>>  	mutex_lock(&data->mutex);
> >>> -	for (bit = 0; bit < AXIS_MAX; bit++) {
> >>> -		ret = kxcjk1013_get_acc_reg(data, bit);
> >>> -		if (ret < 0) {
> >>> -			mutex_unlock(&data->mutex);
> >>> -			goto err;
> >>> -		}
> >>> -		data->buffer[i++] = ret;
> >>> -	}
> >>> +	ret = data->read_block_data(data->client, KXCJK1013_REG_XOUT_L,
> >>> +				    AXIS_MAX * 2, (u8 *) data->buffer);
> >>>  	mutex_unlock(&data->mutex);
> >>> +	if (ret < 0)
> >>> +		goto err;
> >>>
> >>>  	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> >>>  					   data->timestamp);
> >>> @@ -1196,6 +1211,11 @@ static int kxcjk1013_probe(struct i2c_client *client,
> >>>  	const char *name;
> >>>  	int ret;
> >>>
> >>> +	if (!i2c_check_functionality(client->adapter,
> >>> +				     I2C_FUNC_SMBUS_BYTE_DATA |
> >>> +				     I2C_FUNC_SMBUS_READ_WORD_DATA))
> >>> +		return -ENODEV;
> >>> +
> >>>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> >>>  	if (!indio_dev)
> >>>  		return -ENOMEM;
> >>> @@ -1204,6 +1224,12 @@ static int kxcjk1013_probe(struct i2c_client *client,
> >>>  	i2c_set_clientdata(client, indio_dev);
> >>>  	data->client = client;
> >>>
> >>> +	if (i2c_check_functionality(client->adapter,
> >>> +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK))
> >>> +		data->read_block_data = i2c_smbus_read_i2c_block_data;
> >>> +	else
> >>> +		data->read_block_data = kxcjk1013_read_block_data;
> >>> +
> >>>  	pdata = dev_get_platdata(&client->dev);
> >>>  	if (pdata)
> >>>  		data->active_high_intr = pdata->active_high_intr;
> >>>
> >>
> >> --
> >>
> >> Peter Meerwald
> >> +43-664-2444418 (mobile)