[PATCH v3 0/8] Add support for best effort block read emulation

[PATCH v3 0/8] Add support for best effort block read emulation

[Irina Tirdea]

This is version 3 for adding i2c_smbus_read_i2c_block_data_or_emulated
to i2c-core. The i2c core part is the same as in v2, but I have added
3 more users for iio drivers.

Changes in v2:
 - changed bmc150-accel, kxcjk-1013 and bmg160 drivers to use
i2c_smbus_read_i2c_block_data_or_emulated

Changes in v1:
 - dropped the RFC tag
 - changed at24 to use i2c_smbus_read_i2c_block_data_or_emulated
 - when reading an odd number of bytes using word emulation, read an even
number of bytes and drop the last one
 - add a comment that this might not be suitable for all I2C slaves 

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

Irina Tirdea (6):
  i2c: core: Add support for best effort block read emulation
  eeprom: at24: use i2c_smbus_read_i2c_block_data_or_emulated
  iio: accel: bmc150: use available_scan_masks
  iio: accel: bmc150: optimize i2c transfers in trigger handler
  iio: gyro: bmg160: use available_scan_masks
  iio: gyro: bmg160: optimize i2c transfers in trigger handler

 drivers/i2c/i2c-core.c           | 60 ++++++++++++++++++++++++++++++++++++++++
 drivers/iio/accel/bmc150-accel.c | 23 +++++++--------
 drivers/iio/accel/kxcjk-1013.c   | 24 ++++++++--------
 drivers/iio/gyro/bmg160.c        | 23 +++++++--------
 drivers/misc/eeprom/at24.c       | 40 ++++++---------------------
 include/linux/i2c.h              |  3 ++
 6 files changed, 108 insertions(+), 65 deletions(-)

-- 
1.9.1

[PATCH v3 1/8] i2c: core: Add support for best effort block read emulation

[Irina Tirdea]

There are devices that need to handle block transactions
regardless of the capabilities exported by the adapter.
For performance reasons, they need to use i2c read blocks
if available, otherwise emulate the block transaction with word
or byte transactions.

Add support for a helper function that would read a data block
using the best transfer available: I2C_FUNC_SMBUS_READ_I2C_BLOCK,
I2C_FUNC_SMBUS_READ_WORD_DATA or I2C_FUNC_SMBUS_READ_BYTE_DATA.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/i2c/i2c-core.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h    |  3 +++
 2 files changed, 63 insertions(+)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 96771ea..55a3455 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -2914,6 +2914,66 @@ trace:
 }
 EXPORT_SYMBOL(i2c_smbus_xfer);
 
+/**
+ * i2c_smbus_read_i2c_block_data_or_emulated - read block or emulate
+ * @client: Handle to slave device
+ * @command: Byte interpreted by slave
+ * @length: Size of data block; SMBus allows at most 32 bytes
+ * @values: Byte array into which data will be read; big enough to hold
+ *	the data returned by the slave.  SMBus allows at most 32 bytes.
+ *
+ * This executes the SMBus "block read" protocol if supported by the adapter.
+ * If block read is not supported, it emulates it using either word or byte
+ * read protocols depending on availability.
+ *
+ * Before using this function you must double-check if the I2C slave does
+ * support exchanging a block transfer with a byte transfer.
+ */
+s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
+					      u8 command, u8 length, u8 *values)
+{
+	u8 i;
+	int status;
+
+	if (length > I2C_SMBUS_BLOCK_MAX)
+		length = I2C_SMBUS_BLOCK_MAX;
+
+	if (i2c_check_functionality(client->adapter,
+				    I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+		return i2c_smbus_read_i2c_block_data(client, command,
+						     length, values);
+	} else if (i2c_check_functionality(client->adapter,
+					   I2C_FUNC_SMBUS_READ_WORD_DATA)) {
+		for (i = 0; i < length; i += 2) {
+			status = i2c_smbus_read_word_data(client, command + i);
+			if (status < 0)
+				return status;
+			values[i] = status & 0xff;
+			if ((i + 1) < length)
+				values[i + 1] = status >> 8;
+		}
+		if (i > length)
+			return length;
+		return i;
+	} else if (i2c_check_functionality(client->adapter,
+					   I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
+		for (i = 0; i < length; i++) {
+			status = i2c_smbus_read_byte_data(client, command + i);
+			if (status < 0)
+				return status;
+			values[i] = status;
+		}
+		return i;
+	}
+
+	dev_err(&client->adapter->dev, "Unsupported transactions: %d,%d,%d\n",
+		I2C_SMBUS_I2C_BLOCK_DATA, I2C_SMBUS_WORD_DATA,
+		I2C_SMBUS_BYTE_DATA);
+
+	return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL(i2c_smbus_read_i2c_block_data_or_emulated);
+
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb)
 {
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index e83a738..faf518d 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -121,6 +121,9 @@ 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);
+extern s32
+i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
+					  u8 command, u8 length, u8 *values);
 #endif /* I2C */
 
 /**
-- 
1.9.1

[PATCH v3 2/8] eeprom: at24: use i2c_smbus_read_i2c_block_data_or_emulated

[Irina Tirdea]

For i2c busses that support only SMBUS extensions, the eeprom at24
driver reads data from the device using the SMBus block, word or byte
read protocols depending on availability.

Replace the block read emulation from the driver with the
i2c_smbus_read_i2c_block_data_or_emulated call from i2c core.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/misc/eeprom/at24.c | 40 +++++++++-------------------------------
 1 file changed, 9 insertions(+), 31 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 2d3db81..d13795a 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -186,19 +186,11 @@ static ssize_t at24_eeprom_read(struct at24_data *at24, char *buf,
 	if (count > io_limit)
 		count = io_limit;
 
-	switch (at24->use_smbus) {
-	case I2C_SMBUS_I2C_BLOCK_DATA:
+	if (at24->use_smbus) {
 		/* Smaller eeproms can work given some SMBus extension calls */
 		if (count > I2C_SMBUS_BLOCK_MAX)
 			count = I2C_SMBUS_BLOCK_MAX;
-		break;
-	case I2C_SMBUS_WORD_DATA:
-		count = 2;
-		break;
-	case I2C_SMBUS_BYTE_DATA:
-		count = 1;
-		break;
-	default:
+	} else {
 		/*
 		 * When we have a better choice than SMBus calls, use a
 		 * combined I2C message. Write address; then read up to
@@ -229,27 +221,13 @@ static ssize_t at24_eeprom_read(struct at24_data *at24, char *buf,
 	timeout = jiffies + msecs_to_jiffies(write_timeout);
 	do {
 		read_time = jiffies;
-		switch (at24->use_smbus) {
-		case I2C_SMBUS_I2C_BLOCK_DATA:
-			status = i2c_smbus_read_i2c_block_data(client, offset,
-					count, buf);
-			break;
-		case I2C_SMBUS_WORD_DATA:
-			status = i2c_smbus_read_word_data(client, offset);
-			if (status >= 0) {
-				buf[0] = status & 0xff;
-				buf[1] = status >> 8;
-				status = count;
-			}
-			break;
-		case I2C_SMBUS_BYTE_DATA:
-			status = i2c_smbus_read_byte_data(client, offset);
-			if (status >= 0) {
-				buf[0] = status;
-				status = count;
-			}
-			break;
-		default:
+		if (at24->use_smbus) {
+			status =
+			    i2c_smbus_read_i2c_block_data_or_emulated(client,
+								      offset,
+								      count,
+								      buf);
+		} else {
 			status = i2c_transfer(client->adapter, msg, 2);
 			if (status == 2)
 				status = count;
-- 
1.9.1

[PATCH v3 3/8] iio: accel: bmc150: use available_scan_masks

[Irina Tirdea]

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.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/iio/accel/bmc150-accel.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
index 73e8773..c6c8416 100644
--- a/drivers/iio/accel/bmc150-accel.c
+++ b/drivers/iio/accel/bmc150-accel.c
@@ -136,6 +136,7 @@ enum bmc150_accel_axis {
 	AXIS_X,
 	AXIS_Y,
 	AXIS_Z,
+	AXIS_MAX,
 };
 
 enum bmc150_power_modes {
@@ -1201,6 +1202,8 @@ static const struct iio_info bmc150_accel_info_fifo = {
 	.driver_module		= THIS_MODULE,
 };
 
+static const unsigned long bmc150_accel_scan_masks[] = {0x7, 0};
+
 static irqreturn_t bmc150_accel_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
@@ -1209,8 +1212,7 @@ static irqreturn_t bmc150_accel_trigger_handler(int irq, void *p)
 	int bit, ret, i = 0;
 
 	mutex_lock(&data->mutex);
-	for_each_set_bit(bit, indio_dev->active_scan_mask,
-			 indio_dev->masklength) {
+	for (bit = 0; bit < AXIS_MAX; bit++) {
 		ret = i2c_smbus_read_word_data(data->client,
 					       BMC150_ACCEL_AXIS_TO_REG(bit));
 		if (ret < 0) {
@@ -1632,6 +1634,7 @@ static int bmc150_accel_probe(struct i2c_client *client,
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->channels = data->chip_info->channels;
 	indio_dev->num_channels = data->chip_info->num_channels;
+	indio_dev->available_scan_masks = bmc150_accel_scan_masks;
 	indio_dev->name = name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &bmc150_accel_info;
-- 
1.9.1

[PATCH v3 4/8] iio: accel: bmc150: optimize i2c transfers in trigger handler

[Irina Tirdea]

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 bmc150 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. Uses i2c_smbus_read_i2c_block_data_or_emulated
that will fallback to reading each axis as a separate word in case i2c
block read is not supported.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/iio/accel/bmc150-accel.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
index c6c8416..e686add 100644
--- a/drivers/iio/accel/bmc150-accel.c
+++ b/drivers/iio/accel/bmc150-accel.c
@@ -1082,6 +1082,7 @@ static const struct iio_event_spec bmc150_accel_event = {
 		.realbits = (bits),					\
 		.storagebits = 16,					\
 		.shift = 16 - (bits),					\
+		.endianness = IIO_LE,					\
 	},								\
 	.event_spec = &bmc150_accel_event,				\
 	.num_event_specs = 1						\
@@ -1209,19 +1210,16 @@ static irqreturn_t bmc150_accel_trigger_handler(int irq, void *p)
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct bmc150_accel_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 = i2c_smbus_read_word_data(data->client,
-					       BMC150_ACCEL_AXIS_TO_REG(bit));
-		if (ret < 0) {
-			mutex_unlock(&data->mutex);
-			goto err_read;
-		}
-		data->buffer[i++] = ret;
-	}
+	ret = i2c_smbus_read_i2c_block_data_or_emulated(data->client,
+							BMC150_ACCEL_REG_XOUT_L,
+							AXIS_MAX * 2,
+							(u8 *)data->buffer);
 	mutex_unlock(&data->mutex);
+	if (ret < 0)
+		goto err_read;
 
 	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
 					   data->timestamp);
-- 
1.9.1

[PATCH v3 5/8] iio: gyro: bmg160: use available_scan_masks

[Irina Tirdea]

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.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/iio/gyro/bmg160.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/gyro/bmg160.c b/drivers/iio/gyro/bmg160.c
index 4415f55..4b423f2 100644
--- a/drivers/iio/gyro/bmg160.c
+++ b/drivers/iio/gyro/bmg160.c
@@ -115,6 +115,7 @@ enum bmg160_axis {
 	AXIS_X,
 	AXIS_Y,
 	AXIS_Z,
+	AXIS_MAX,
 };
 
 static const struct {
@@ -814,6 +815,8 @@ static const struct iio_info bmg160_info = {
 	.driver_module		= THIS_MODULE,
 };
 
+static const unsigned long bmg160_scan_masks[] = {0x7, 0};
+
 static irqreturn_t bmg160_trigger_handler(int irq, void *p)
 {
 	struct iio_poll_func *pf = p;
@@ -822,8 +825,7 @@ static irqreturn_t bmg160_trigger_handler(int irq, void *p)
 	int bit, ret, i = 0;
 
 	mutex_lock(&data->mutex);
-	for_each_set_bit(bit, indio_dev->active_scan_mask,
-			 indio_dev->masklength) {
+	for (bit = 0; bit < AXIS_MAX; bit++) {
 		ret = i2c_smbus_read_word_data(data->client,
 					       BMG160_AXIS_TO_REG(bit));
 		if (ret < 0) {
@@ -1056,6 +1058,7 @@ static int bmg160_probe(struct i2c_client *client,
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->channels = bmg160_channels;
 	indio_dev->num_channels = ARRAY_SIZE(bmg160_channels);
+	indio_dev->available_scan_masks = bmg160_scan_masks;
 	indio_dev->name = name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->info = &bmg160_info;
-- 
1.9.1

[PATCH v3 6/8] iio: gyro: bmg160: optimize i2c transfers in trigger handler

[Irina Tirdea]

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 bmg160 driver does
one i2c transfer for each axis. This has an impact on the frequency
of the gyroscope 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. Uses i2c_smbus_read_i2c_block_data_or_emulated
that will fallback to reading each axis as a separate word in case i2c
block read is not supported.

Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/iio/gyro/bmg160.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/gyro/bmg160.c b/drivers/iio/gyro/bmg160.c
index 4b423f2..04e15e9 100644
--- a/drivers/iio/gyro/bmg160.c
+++ b/drivers/iio/gyro/bmg160.c
@@ -784,6 +784,7 @@ static const struct iio_event_spec bmg160_event = {
 		.sign = 's',						\
 		.realbits = 16,					\
 		.storagebits = 16,					\
+		.endianness = IIO_LE,					\
 	},								\
 	.event_spec = &bmg160_event,					\
 	.num_event_specs = 1						\
@@ -822,19 +823,16 @@ static irqreturn_t bmg160_trigger_handler(int irq, void *p)
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct bmg160_data *data = iio_priv(indio_dev);
-	int bit, ret, i = 0;
+	int ret = 0;
 
 	mutex_lock(&data->mutex);
-	for (bit = 0; bit < AXIS_MAX; bit++) {
-		ret = i2c_smbus_read_word_data(data->client,
-					       BMG160_AXIS_TO_REG(bit));
-		if (ret < 0) {
-			mutex_unlock(&data->mutex);
-			goto err;
-		}
-		data->buffer[i++] = ret;
-	}
+	ret = i2c_smbus_read_i2c_block_data_or_emulated(data->client,
+							BMG160_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);
-- 
1.9.1

[PATCH v3 7/8] 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.

Signed-off-by: Adriana Reus <adriana.reus@intel.com>
Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 51da369..4960397 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;
@@ -956,8 +959,7 @@ static irqreturn_t kxcjk1013_trigger_handler(int irq, void *p)
 
 	mutex_lock(&data->mutex);
 
-	for_each_set_bit(bit, indio_dev->active_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);
@@ -1224,6 +1226,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 v3 8/8] 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. Uses i2c_smbus_read_i2c_block_data_or_emulated
that will fallback to reading each axis as a separate word in case i2c
block read is not supported.

Signed-off-by: Adriana Reus <adriana.reus@intel.com>
Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 4960397..8f401c2 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -923,7 +923,7 @@ static const struct iio_event_spec kxcjk1013_event = {
 		.realbits = 12,						\
 		.storagebits = 16,					\
 		.shift = 4,						\
-		.endianness = IIO_CPU,					\
+		.endianness = IIO_LE,					\
 	},								\
 	.event_spec = &kxcjk1013_event,				\
 	.num_event_specs = 1						\
@@ -955,19 +955,16 @@ 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 = i2c_smbus_read_i2c_block_data_or_emulated(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);
-- 
1.9.1

Re: [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation

[Jonathan Cameron]

On 03/07/15 10:33, Irina Tirdea wrote:
> There are devices that need to handle block transactions
> regardless of the capabilities exported by the adapter.
> For performance reasons, they need to use i2c read blocks
> if available, otherwise emulate the block transaction with word
> or byte transactions.
> 
> Add support for a helper function that would read a data block
> using the best transfer available: I2C_FUNC_SMBUS_READ_I2C_BLOCK,
> I2C_FUNC_SMBUS_READ_WORD_DATA or I2C_FUNC_SMBUS_READ_BYTE_DATA.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Looks good to me - I vaguely wondered if it would make sense to use
an endian conversion in the word case, but as we have possible odd
numbers of bytes that gets fiddly.

I wonder what devices do if you do a word read beyond their end address?
Perhaps in odd cases we should always fall back to byte reads?

> ---
>  drivers/i2c/i2c-core.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/i2c.h    |  3 +++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 96771ea..55a3455 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -2914,6 +2914,66 @@ trace:
>  }
>  EXPORT_SYMBOL(i2c_smbus_xfer);
>  
> +/**
> + * i2c_smbus_read_i2c_block_data_or_emulated - read block or emulate
> + * @client: Handle to slave device
> + * @command: Byte interpreted by slave
> + * @length: Size of data block; SMBus allows at most 32 bytes
> + * @values: Byte array into which data will be read; big enough to hold
> + *	the data returned by the slave.  SMBus allows at most 32 bytes.
> + *
> + * This executes the SMBus "block read" protocol if supported by the adapter.
> + * If block read is not supported, it emulates it using either word or byte
> + * read protocols depending on availability.
> + *
> + * Before using this function you must double-check if the I2C slave does
> + * support exchanging a block transfer with a byte transfer.
Add something here about addressing assumptions.  You get odd devices which
will give bulk reads of addresses not mapped to a nice linear region when
you do byte reads.
> + */
> +s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
> +					      u8 command, u8 length, u8 *values)
> +{
> +	u8 i;
> +	int status;
> +
> +	if (length > I2C_SMBUS_BLOCK_MAX)
> +		length = I2C_SMBUS_BLOCK_MAX;
> +
> +	if (i2c_check_functionality(client->adapter,
> +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
> +		return i2c_smbus_read_i2c_block_data(client, command,
> +						     length, values);
> +	} else if (i2c_check_functionality(client->adapter,
> +					   I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> +		for (i = 0; i < length; i += 2) {
> +			status = i2c_smbus_read_word_data(client, command + i);
> +			if (status < 0)
> +				return status;
> +			values[i] = status & 0xff;
> +			if ((i + 1) < length)
> +				values[i + 1] = status >> 8;
> +		}
> +		if (i > length)
> +			return length;
> +		return i;
> +	} else if (i2c_check_functionality(client->adapter,
> +					   I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
> +		for (i = 0; i < length; i++) {
> +			status = i2c_smbus_read_byte_data(client, command + i);
> +			if (status < 0)
> +				return status;
> +			values[i] = status;
> +		}
> +		return i;
> +	}
> +
> +	dev_err(&client->adapter->dev, "Unsupported transactions: %d,%d,%d\n",
> +		I2C_SMBUS_I2C_BLOCK_DATA, I2C_SMBUS_WORD_DATA,
> +		I2C_SMBUS_BYTE_DATA);
> +
> +	return -EOPNOTSUPP;
> +}
> +EXPORT_SYMBOL(i2c_smbus_read_i2c_block_data_or_emulated);
> +
>  #if IS_ENABLED(CONFIG_I2C_SLAVE)
>  int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb)
>  {
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index e83a738..faf518d 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -121,6 +121,9 @@ 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);
> +extern s32
> +i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
> +					  u8 command, u8 length, u8 *values);
>  #endif /* I2C */
>  
>  /**
> 

Re: [PATCH v3 4/8] iio: accel: bmc150: optimize i2c transfers in trigger handler

[Jonathan Cameron]

On 03/07/15 10:33, Irina Tirdea wrote:
> 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 bmc150 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. Uses i2c_smbus_read_i2c_block_data_or_emulated
> that will fallback to reading each axis as a separate word in case i2c
> block read is not supported.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Very nice.  There is an effective userspace ABI change, but if people
are using the ABI wrong, then it's kind of their own fault!
I'll assume Wolfram will pick these up if / when he is happy with the
new function. 

Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/iio/accel/bmc150-accel.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
> index c6c8416..e686add 100644
> --- a/drivers/iio/accel/bmc150-accel.c
> +++ b/drivers/iio/accel/bmc150-accel.c
> @@ -1082,6 +1082,7 @@ static const struct iio_event_spec bmc150_accel_event = {
>  		.realbits = (bits),					\
>  		.storagebits = 16,					\
>  		.shift = 16 - (bits),					\
> +		.endianness = IIO_LE,					\
Hmm. ABI change of a sort.  Should be fine if userspace is doing things right, but
never a whole lot of certainty about that.  Lets see if anyone screams...
>  	},								\
>  	.event_spec = &bmc150_accel_event,				\
>  	.num_event_specs = 1						\
> @@ -1209,19 +1210,16 @@ static irqreturn_t bmc150_accel_trigger_handler(int irq, void *p)
>  	struct iio_poll_func *pf = p;
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct bmc150_accel_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 = i2c_smbus_read_word_data(data->client,
> -					       BMC150_ACCEL_AXIS_TO_REG(bit));
> -		if (ret < 0) {
> -			mutex_unlock(&data->mutex);
> -			goto err_read;
> -		}
> -		data->buffer[i++] = ret;
> -	}
> +	ret = i2c_smbus_read_i2c_block_data_or_emulated(data->client,
> +							BMC150_ACCEL_REG_XOUT_L,
> +							AXIS_MAX * 2,
> +							(u8 *)data->buffer);
>  	mutex_unlock(&data->mutex);
> +	if (ret < 0)
> +		goto err_read;
>  
>  	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
>  					   data->timestamp);
> 

Re: [PATCH v3 6/8] iio: gyro: bmg160: optimize i2c transfers in trigger handler

[Jonathan Cameron]

On 03/07/15 10:33, Irina Tirdea wrote:
> 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 bmg160 driver does
> one i2c transfer for each axis. This has an impact on the frequency
> of the gyroscope 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. Uses i2c_smbus_read_i2c_block_data_or_emulated
> that will fallback to reading each axis as a separate word in case i2c
> block read is not supported.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>

Obviously you'll ideally pick up Ack's from the driver authors /maintainers
as well.

Jonathan
> ---
>  drivers/iio/gyro/bmg160.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/gyro/bmg160.c b/drivers/iio/gyro/bmg160.c
> index 4b423f2..04e15e9 100644
> --- a/drivers/iio/gyro/bmg160.c
> +++ b/drivers/iio/gyro/bmg160.c
> @@ -784,6 +784,7 @@ static const struct iio_event_spec bmg160_event = {
>  		.sign = 's',						\
>  		.realbits = 16,					\
>  		.storagebits = 16,					\
> +		.endianness = IIO_LE,					\
>  	},								\
>  	.event_spec = &bmg160_event,					\
>  	.num_event_specs = 1						\
> @@ -822,19 +823,16 @@ static irqreturn_t bmg160_trigger_handler(int irq, void *p)
>  	struct iio_poll_func *pf = p;
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct bmg160_data *data = iio_priv(indio_dev);
> -	int bit, ret, i = 0;
> +	int ret = 0;
>  
>  	mutex_lock(&data->mutex);
> -	for (bit = 0; bit < AXIS_MAX; bit++) {
> -		ret = i2c_smbus_read_word_data(data->client,
> -					       BMG160_AXIS_TO_REG(bit));
> -		if (ret < 0) {
> -			mutex_unlock(&data->mutex);
> -			goto err;
> -		}
> -		data->buffer[i++] = ret;
> -	}
> +	ret = i2c_smbus_read_i2c_block_data_or_emulated(data->client,
> +							BMG160_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);
> 

Re: [PATCH v3 3/8] iio: accel: bmc150: use available_scan_masks

[Jonathan Cameron]

On 03/07/15 10:33, Irina Tirdea wrote:
> 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.
> 
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
I guess people almost always want all 3 axes of an accelerometer
so a burst read like this probably makes sense for the majority.

Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/iio/accel/bmc150-accel.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/bmc150-accel.c b/drivers/iio/accel/bmc150-accel.c
> index 73e8773..c6c8416 100644
> --- a/drivers/iio/accel/bmc150-accel.c
> +++ b/drivers/iio/accel/bmc150-accel.c
> @@ -136,6 +136,7 @@ enum bmc150_accel_axis {
>  	AXIS_X,
>  	AXIS_Y,
>  	AXIS_Z,
> +	AXIS_MAX,
>  };
>  
>  enum bmc150_power_modes {
> @@ -1201,6 +1202,8 @@ static const struct iio_info bmc150_accel_info_fifo = {
>  	.driver_module		= THIS_MODULE,
>  };
>  
> +static const unsigned long bmc150_accel_scan_masks[] = {0x7, 0};
> +
>  static irqreturn_t bmc150_accel_trigger_handler(int irq, void *p)
>  {
>  	struct iio_poll_func *pf = p;
> @@ -1209,8 +1212,7 @@ static irqreturn_t bmc150_accel_trigger_handler(int irq, void *p)
>  	int bit, ret, i = 0;
>  
>  	mutex_lock(&data->mutex);
> -	for_each_set_bit(bit, indio_dev->active_scan_mask,
> -			 indio_dev->masklength) {
> +	for (bit = 0; bit < AXIS_MAX; bit++) {
>  		ret = i2c_smbus_read_word_data(data->client,
>  					       BMC150_ACCEL_AXIS_TO_REG(bit));
>  		if (ret < 0) {
> @@ -1632,6 +1634,7 @@ static int bmc150_accel_probe(struct i2c_client *client,
>  	indio_dev->dev.parent = &client->dev;
>  	indio_dev->channels = data->chip_info->channels;
>  	indio_dev->num_channels = data->chip_info->num_channels;
> +	indio_dev->available_scan_masks = bmc150_accel_scan_masks;
>  	indio_dev->name = name;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->info = &bmc150_accel_info;
> 

Re: [PATCH v3 7/8] iio: accel: kxcjk-1013: use available_scan_masks

[Jonathan Cameron]

On 03/07/15 10:33, Irina Tirdea wrote:
> 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.
> 
> Signed-off-by: Adriana Reus <adriana.reus@intel.com>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/iio/accel/kxcjk-1013.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 51da369..4960397 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;
> @@ -956,8 +959,7 @@ static irqreturn_t kxcjk1013_trigger_handler(int irq, void *p)
>  
>  	mutex_lock(&data->mutex);
>  
> -	for_each_set_bit(bit, indio_dev->active_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);
> @@ -1224,6 +1226,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;
> 

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

[Jonathan Cameron]

On 03/07/15 10:33, Irina Tirdea wrote:
> 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. Uses i2c_smbus_read_i2c_block_data_or_emulated
> that will fallback to reading each axis as a separate word in case i2c
> block read is not supported.
> 
> Signed-off-by: Adriana Reus <adriana.reus@intel.com>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
Same comments as previously.

Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  drivers/iio/accel/kxcjk-1013.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 4960397..8f401c2 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -923,7 +923,7 @@ static const struct iio_event_spec kxcjk1013_event = {
>  		.realbits = 12,						\
>  		.storagebits = 16,					\
>  		.shift = 4,						\
> -		.endianness = IIO_CPU,					\
> +		.endianness = IIO_LE,					\
>  	},								\
>  	.event_spec = &kxcjk1013_event,				\
>  	.num_event_specs = 1						\
> @@ -955,19 +955,16 @@ 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 = i2c_smbus_read_i2c_block_data_or_emulated(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);
> 

RE: [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation

[Tirdea, Irina]

> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: 05 July, 2015 14:59
> To: Tirdea, Irina; Wolfram Sang; linux-iio@vger.kernel.org; linux-i2c@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Peter Meerwald
> Subject: Re: [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation
> 
> On 03/07/15 10:33, Irina Tirdea wrote:
> > There are devices that need to handle block transactions
> > regardless of the capabilities exported by the adapter.
> > For performance reasons, they need to use i2c read blocks
> > if available, otherwise emulate the block transaction with word
> > or byte transactions.
> >
> > Add support for a helper function that would read a data block
> > using the best transfer available: I2C_FUNC_SMBUS_READ_I2C_BLOCK,
> > I2C_FUNC_SMBUS_READ_WORD_DATA or I2C_FUNC_SMBUS_READ_BYTE_DATA.
> >
> > Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Looks good to me - I vaguely wondered if it would make sense to use
> an endian conversion in the word case, but as we have possible odd
> numbers of bytes that gets fiddly.
> 

Thanks for the review, Jonathan!

> I wonder what devices do if you do a word read beyond their end address?
> Perhaps in odd cases we should always fall back to byte reads?

In my tests I can read beyond the end address, but I cannot be sure if this is OK for all
devices. This was actually a suggestion from Wolfram for v1, but maybe I'm missing
something.

Wolfram, is it safe to read one byte beyond the end address or should I better use
only byte reads for odd lengths?

> 
> > ---
> >  drivers/i2c/i2c-core.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/i2c.h    |  3 +++
> >  2 files changed, 63 insertions(+)
> >
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index 96771ea..55a3455 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -2914,6 +2914,66 @@ trace:
> >  }
> >  EXPORT_SYMBOL(i2c_smbus_xfer);
> >
> > +/**
> > + * i2c_smbus_read_i2c_block_data_or_emulated - read block or emulate
> > + * @client: Handle to slave device
> > + * @command: Byte interpreted by slave
> > + * @length: Size of data block; SMBus allows at most 32 bytes
> > + * @values: Byte array into which data will be read; big enough to hold
> > + *	the data returned by the slave.  SMBus allows at most 32 bytes.
> > + *
> > + * This executes the SMBus "block read" protocol if supported by the adapter.
> > + * If block read is not supported, it emulates it using either word or byte
> > + * read protocols depending on availability.
> > + *
> > + * Before using this function you must double-check if the I2C slave does
> > + * support exchanging a block transfer with a byte transfer.
> Add something here about addressing assumptions.  You get odd devices which
> will give bulk reads of addresses not mapped to a nice linear region when
> you do byte reads.

OK, I'll add this to the comment above:
"The addresses of the I2C slave device that are accessed with this function
 must be mapped to a linear region, so that a block read will have the same
 effect as a byte read."

Thanks,
Irina

> > + */
> > +s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
> > +					      u8 command, u8 length, u8 *values)
> > +{
> > +	u8 i;
> > +	int status;
> > +
> > +	if (length > I2C_SMBUS_BLOCK_MAX)
> > +		length = I2C_SMBUS_BLOCK_MAX;
> > +
> > +	if (i2c_check_functionality(client->adapter,
> > +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
> > +		return i2c_smbus_read_i2c_block_data(client, command,
> > +						     length, values);
> > +	} else if (i2c_check_functionality(client->adapter,
> > +					   I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> > +		for (i = 0; i < length; i += 2) {
> > +			status = i2c_smbus_read_word_data(client, command + i);
> > +			if (status < 0)
> > +				return status;
> > +			values[i] = status & 0xff;
> > +			if ((i + 1) < length)
> > +				values[i + 1] = status >> 8;
> > +		}
> > +		if (i > length)
> > +			return length;
> > +		return i;
> > +	} else if (i2c_check_functionality(client->adapter,
> > +					   I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
> > +		for (i = 0; i < length; i++) {
> > +			status = i2c_smbus_read_byte_data(client, command + i);
> > +			if (status < 0)
> > +				return status;
> > +			values[i] = status;
> > +		}
> > +		return i;
> > +	}
> > +
> > +	dev_err(&client->adapter->dev, "Unsupported transactions: %d,%d,%d\n",
> > +		I2C_SMBUS_I2C_BLOCK_DATA, I2C_SMBUS_WORD_DATA,
> > +		I2C_SMBUS_BYTE_DATA);
> > +
> > +	return -EOPNOTSUPP;
> > +}
> > +EXPORT_SYMBOL(i2c_smbus_read_i2c_block_data_or_emulated);
> > +
> >  #if IS_ENABLED(CONFIG_I2C_SLAVE)
> >  int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb)
> >  {
> > diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> > index e83a738..faf518d 100644
> > --- a/include/linux/i2c.h
> > +++ b/include/linux/i2c.h
> > @@ -121,6 +121,9 @@ 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);
> > +extern s32
> > +i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
> > +					  u8 command, u8 length, u8 *values);
> >  #endif /* I2C */
> >
> >  /**
> >

RE: [PATCH v3 6/8] iio: gyro: bmg160: optimize i2c transfers in trigger handler

[Tirdea, Irina]

> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: 05 July, 2015 15:08
> To: Tirdea, Irina; Wolfram Sang; linux-iio@vger.kernel.org; linux-i2c@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Peter Meerwald
> Subject: Re: [PATCH v3 6/8] iio: gyro: bmg160: optimize i2c transfers in trigger handler
> 
> On 03/07/15 10:33, Irina Tirdea wrote:
> > 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 bmg160 driver does
> > one i2c transfer for each axis. This has an impact on the frequency
> > of the gyroscope 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. Uses i2c_smbus_read_i2c_block_data_or_emulated
> > that will fallback to reading each axis as a separate word in case i2c
> > block read is not supported.
> >
> > Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Acked-by: Jonathan Cameron <jic23@kernel.org>
> 

Thanks Jonathan!

> Obviously you'll ideally pick up Ack's from the driver authors /maintainers
> as well.
> 

Srinivas, are you OK with these changes for bmc150, bmg160 and kxcjk-1013?

Thanks,
Irina

> Jonathan
> > ---
> >  drivers/iio/gyro/bmg160.c | 18 ++++++++----------
> >  1 file changed, 8 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/iio/gyro/bmg160.c b/drivers/iio/gyro/bmg160.c
> > index 4b423f2..04e15e9 100644
> > --- a/drivers/iio/gyro/bmg160.c
> > +++ b/drivers/iio/gyro/bmg160.c
> > @@ -784,6 +784,7 @@ static const struct iio_event_spec bmg160_event = {
> >  		.sign = 's',						\
> >  		.realbits = 16,					\
> >  		.storagebits = 16,					\
> > +		.endianness = IIO_LE,					\
> >  	},								\
> >  	.event_spec = &bmg160_event,					\
> >  	.num_event_specs = 1						\
> > @@ -822,19 +823,16 @@ static irqreturn_t bmg160_trigger_handler(int irq, void *p)
> >  	struct iio_poll_func *pf = p;
> >  	struct iio_dev *indio_dev = pf->indio_dev;
> >  	struct bmg160_data *data = iio_priv(indio_dev);
> > -	int bit, ret, i = 0;
> > +	int ret = 0;
> >
> >  	mutex_lock(&data->mutex);
> > -	for (bit = 0; bit < AXIS_MAX; bit++) {
> > -		ret = i2c_smbus_read_word_data(data->client,
> > -					       BMG160_AXIS_TO_REG(bit));
> > -		if (ret < 0) {
> > -			mutex_unlock(&data->mutex);
> > -			goto err;
> > -		}
> > -		data->buffer[i++] = ret;
> > -	}
> > +	ret = i2c_smbus_read_i2c_block_data_or_emulated(data->client,
> > +							BMG160_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);
> >

Re: [PATCH v3 6/8] iio: gyro: bmg160: 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: 3396 bytes --]

On Fri, 2015-07-10 at 17:31 +0000, Tirdea, Irina wrote:
> 
> > -----Original Message-----
> > From: Jonathan Cameron [mailto:jic23@kernel.org]
> > Sent: 05 July, 2015 15:08
> > To: Tirdea, Irina; Wolfram Sang; linux-iio@vger.kernel.org; linux-i2c@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Peter Meerwald
> > Subject: Re: [PATCH v3 6/8] iio: gyro: bmg160: optimize i2c transfers in trigger handler
> > 
> > On 03/07/15 10:33, Irina Tirdea wrote:
> > > 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 bmg160 driver does
> > > one i2c transfer for each axis. This has an impact on the frequency
> > > of the gyroscope 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. Uses i2c_smbus_read_i2c_block_data_or_emulated
> > > that will fallback to reading each axis as a separate word in case i2c
> > > block read is not supported.
> > >
> > > Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> > Acked-by: Jonathan Cameron <jic23@kernel.org>
> > 
> 
> Thanks Jonathan!
> 
> > Obviously you'll ideally pick up Ack's from the driver authors /maintainers
> > as well.
> > 
> 
> Srinivas, are you OK with these changes for bmc150, bmg160 and kxcjk-1013?
> 
Yes.

Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

> Thanks,
> Irina
> 
> > Jonathan
> > > ---
> > >  drivers/iio/gyro/bmg160.c | 18 ++++++++----------
> > >  1 file changed, 8 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/iio/gyro/bmg160.c b/drivers/iio/gyro/bmg160.c
> > > index 4b423f2..04e15e9 100644
> > > --- a/drivers/iio/gyro/bmg160.c
> > > +++ b/drivers/iio/gyro/bmg160.c
> > > @@ -784,6 +784,7 @@ static const struct iio_event_spec bmg160_event = {
> > >  		.sign = 's',						\
> > >  		.realbits = 16,					\
> > >  		.storagebits = 16,					\
> > > +		.endianness = IIO_LE,					\
> > >  	},								\
> > >  	.event_spec = &bmg160_event,					\
> > >  	.num_event_specs = 1						\
> > > @@ -822,19 +823,16 @@ static irqreturn_t bmg160_trigger_handler(int irq, void *p)
> > >  	struct iio_poll_func *pf = p;
> > >  	struct iio_dev *indio_dev = pf->indio_dev;
> > >  	struct bmg160_data *data = iio_priv(indio_dev);
> > > -	int bit, ret, i = 0;
> > > +	int ret = 0;
> > >
> > >  	mutex_lock(&data->mutex);
> > > -	for (bit = 0; bit < AXIS_MAX; bit++) {
> > > -		ret = i2c_smbus_read_word_data(data->client,
> > > -					       BMG160_AXIS_TO_REG(bit));
> > > -		if (ret < 0) {
> > > -			mutex_unlock(&data->mutex);
> > > -			goto err;
> > > -		}
> > > -		data->buffer[i++] = ret;
> > > -	}
> > > +	ret = i2c_smbus_read_i2c_block_data_or_emulated(data->client,
> > > +							BMG160_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);
> > >
> 

ÿôèº{.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 v3 1/8] i2c: core: Add support for best effort block read emulation

[Jonathan Cameron]

On 10/07/15 18:14, Tirdea, Irina wrote:
> 
> 
>> -----Original Message-----
>> From: Jonathan Cameron [mailto:jic23@kernel.org]
>> Sent: 05 July, 2015 14:59
>> To: Tirdea, Irina; Wolfram Sang; linux-iio@vger.kernel.org; linux-i2c@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Peter Meerwald
>> Subject: Re: [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation
>>
>> On 03/07/15 10:33, Irina Tirdea wrote:
>>> There are devices that need to handle block transactions
>>> regardless of the capabilities exported by the adapter.
>>> For performance reasons, they need to use i2c read blocks
>>> if available, otherwise emulate the block transaction with word
>>> or byte transactions.
>>>
>>> Add support for a helper function that would read a data block
>>> using the best transfer available: I2C_FUNC_SMBUS_READ_I2C_BLOCK,
>>> I2C_FUNC_SMBUS_READ_WORD_DATA or I2C_FUNC_SMBUS_READ_BYTE_DATA.
>>>
>>> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
>> Looks good to me - I vaguely wondered if it would make sense to use
>> an endian conversion in the word case, but as we have possible odd
>> numbers of bytes that gets fiddly.
>>
> 
> Thanks for the review, Jonathan!
> 
>> I wonder what devices do if you do a word read beyond their end address?
>> Perhaps in odd cases we should always fall back to byte reads?
> 
> In my tests I can read beyond the end address, but I cannot be sure if this is OK for all
> devices. This was actually a suggestion from Wolfram for v1, but maybe I'm missing
> something.
> 
> Wolfram, is it safe to read one byte beyond the end address or should I better use
> only byte reads for odd lengths?
> 
>>
>>> ---
>>>  drivers/i2c/i2c-core.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  include/linux/i2c.h    |  3 +++
>>>  2 files changed, 63 insertions(+)
>>>
>>> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>>> index 96771ea..55a3455 100644
>>> --- a/drivers/i2c/i2c-core.c
>>> +++ b/drivers/i2c/i2c-core.c
>>> @@ -2914,6 +2914,66 @@ trace:
>>>  }
>>>  EXPORT_SYMBOL(i2c_smbus_xfer);
>>>
>>> +/**
>>> + * i2c_smbus_read_i2c_block_data_or_emulated - read block or emulate
>>> + * @client: Handle to slave device
>>> + * @command: Byte interpreted by slave
>>> + * @length: Size of data block; SMBus allows at most 32 bytes
>>> + * @values: Byte array into which data will be read; big enough to hold
>>> + *	the data returned by the slave.  SMBus allows at most 32 bytes.
>>> + *
>>> + * This executes the SMBus "block read" protocol if supported by the adapter.
>>> + * If block read is not supported, it emulates it using either word or byte
>>> + * read protocols depending on availability.
>>> + *
>>> + * Before using this function you must double-check if the I2C slave does
>>> + * support exchanging a block transfer with a byte transfer.
>> Add something here about addressing assumptions.  You get odd devices which
>> will give bulk reads of addresses not mapped to a nice linear region when
>> you do byte reads.
> 
> OK, I'll add this to the comment above:
> "The addresses of the I2C slave device that are accessed with this function
>  must be mapped to a linear region, so that a block read will have the same
>  effect as a byte read."
> 
Works for me.
> Thanks,
> Irina
> 
>>> + */
>>> +s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
>>> +					      u8 command, u8 length, u8 *values)
>>> +{
>>> +	u8 i;
>>> +	int status;
>>> +
>>> +	if (length > I2C_SMBUS_BLOCK_MAX)
>>> +		length = I2C_SMBUS_BLOCK_MAX;
>>> +
>>> +	if (i2c_check_functionality(client->adapter,
>>> +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
>>> +		return i2c_smbus_read_i2c_block_data(client, command,
>>> +						     length, values);
>>> +	} else if (i2c_check_functionality(client->adapter,
>>> +					   I2C_FUNC_SMBUS_READ_WORD_DATA)) {
>>> +		for (i = 0; i < length; i += 2) {
>>> +			status = i2c_smbus_read_word_data(client, command + i);
>>> +			if (status < 0)
>>> +				return status;
>>> +			values[i] = status & 0xff;
>>> +			if ((i + 1) < length)
>>> +				values[i + 1] = status >> 8;
>>> +		}
>>> +		if (i > length)
>>> +			return length;
>>> +		return i;
>>> +	} else if (i2c_check_functionality(client->adapter,
>>> +					   I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
>>> +		for (i = 0; i < length; i++) {
>>> +			status = i2c_smbus_read_byte_data(client, command + i);
>>> +			if (status < 0)
>>> +				return status;
>>> +			values[i] = status;
>>> +		}
>>> +		return i;
>>> +	}
>>> +
>>> +	dev_err(&client->adapter->dev, "Unsupported transactions: %d,%d,%d\n",
>>> +		I2C_SMBUS_I2C_BLOCK_DATA, I2C_SMBUS_WORD_DATA,
>>> +		I2C_SMBUS_BYTE_DATA);
>>> +
>>> +	return -EOPNOTSUPP;
>>> +}
>>> +EXPORT_SYMBOL(i2c_smbus_read_i2c_block_data_or_emulated);
>>> +
>>>  #if IS_ENABLED(CONFIG_I2C_SLAVE)
>>>  int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb)
>>>  {
>>> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
>>> index e83a738..faf518d 100644
>>> --- a/include/linux/i2c.h
>>> +++ b/include/linux/i2c.h
>>> @@ -121,6 +121,9 @@ 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);
>>> +extern s32
>>> +i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
>>> +					  u8 command, u8 length, u8 *values);
>>>  #endif /* I2C */
>>>
>>>  /**
>>>
> 

Re: [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2726 bytes --]


> > I wonder what devices do if you do a word read beyond their end address?
> > Perhaps in odd cases we should always fall back to byte reads?
> 
> In my tests I can read beyond the end address, but I cannot be sure if this is OK for all
> devices. This was actually a suggestion from Wolfram for v1, but maybe I'm missing
> something.
> 
> Wolfram, is it safe to read one byte beyond the end address or should I better use
> only byte reads for odd lengths?

Well, from a device perspective, it is probably better to be safe than
sorry and fall back to a single byte read. That means, however, that we
need READ_WORD_DATA and READ_BYTE_DATA to be supported by the adapter.
Should be OK, I don't think there are adapters which can only read words.

> > > + */
> > > +s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
> > > +					      u8 command, u8 length, u8 *values)
> > > +{
> > > +	u8 i;
> > > +	int status;
> > > +
> > > +	if (length > I2C_SMBUS_BLOCK_MAX)
> > > +		length = I2C_SMBUS_BLOCK_MAX;
> > > +
> > > +	if (i2c_check_functionality(client->adapter,
> > > +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
> > > +		return i2c_smbus_read_i2c_block_data(client, command,
> > > +						     length, values);

I am not very strict with the 80 char limit. I'd think the code is more
readable if the two statements above would be oneliners. And for some
other lines later as well.

> > > +	} else if (i2c_check_functionality(client->adapter,

No need for else since we return in the above if anyhow.

> > > +					   I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> > > +		for (i = 0; i < length; i += 2) {
> > > +			status = i2c_smbus_read_word_data(client, command + i);
> > > +			if (status < 0)
> > > +				return status;
> > > +			values[i] = status & 0xff;
> > > +			if ((i + 1) < length)
> > > +				values[i + 1] = status >> 8;
> > > +		}
> > > +		if (i > length)
> > > +			return length;
> > > +		return i;
> > > +	} else if (i2c_check_functionality(client->adapter,
> > > +					   I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
> > > +		for (i = 0; i < length; i++) {
> > > +			status = i2c_smbus_read_byte_data(client, command + i);
> > > +			if (status < 0)
> > > +				return status;
> > > +			values[i] = status;
> > > +		}
> > > +		return i;
> > > +	}
> > > +
> > > +	dev_err(&client->adapter->dev, "Unsupported transactions: %d,%d,%d\n",
> > > +		I2C_SMBUS_I2C_BLOCK_DATA, I2C_SMBUS_WORD_DATA,
> > > +		I2C_SMBUS_BYTE_DATA);

I don't think the %d printouts are useful. Just say that the adapter
lacks support for needed transactions. And I think the device which
reports the error should be the client device, no?

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v3 2/8] eeprom: at24: use i2c_smbus_read_i2c_block_data_or_emulated

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 285 bytes --]

> +		if (at24->use_smbus) {
> +			status =
> +			    i2c_smbus_read_i2c_block_data_or_emulated(client,
> +								      offset,
> +								      count,
> +								      buf);

Please ignore checkpatch warnings for this one and find a nice readable
way to squeeze that into two lines.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation

[Tirdea, Irina]

> -----Original Message-----
> From: linux-iio-owner@vger.kernel.org [mailto:linux-iio-owner@vger.kernel.org] On Behalf Of Wolfram Sang
> Sent: 01 August, 2015 22:53
> To: Tirdea, Irina
> Cc: Jonathan Cameron; linux-kernel@vger.kernel.org; Pandruvada, Srinivas; Peter Meerwald; linux-iio@vger.kernel.org; linux-
> i2c@vger.kernel.org
> Subject: Re: [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation
> 
> 
> > > I wonder what devices do if you do a word read beyond their end address?
> > > Perhaps in odd cases we should always fall back to byte reads?
> >
> > In my tests I can read beyond the end address, but I cannot be sure if this is OK for all
> > devices. This was actually a suggestion from Wolfram for v1, but maybe I'm missing
> > something.
> >
> > Wolfram, is it safe to read one byte beyond the end address or should I better use
> > only byte reads for odd lengths?
> 
> Well, from a device perspective, it is probably better to be safe than
> sorry and fall back to a single byte read. That means, however, that we
> need READ_WORD_DATA and READ_BYTE_DATA to be supported by the adapter.
> Should be OK, I don't think there are adapters which can only read words.
> 

OK. In this case, I would rather go back to how reading odd lengths was handled in
the first version of this patch: use word reads up to the last byte and read that last
byte using a byte read. In this way, odd and even lengths are both handled as
expected (by first falling back to word reads and then to byte reads).
Also, some adapters have performance issues when using more i2c transactions,
so using word instead of byte reads where is possible could help.
I'll send v4 like this and wait for your feedback.

> > > > + */
> > > > +s32 i2c_smbus_read_i2c_block_data_or_emulated(const struct i2c_client *client,
> > > > +					      u8 command, u8 length, u8 *values)
> > > > +{
> > > > +	u8 i;
> > > > +	int status;
> > > > +
> > > > +	if (length > I2C_SMBUS_BLOCK_MAX)
> > > > +		length = I2C_SMBUS_BLOCK_MAX;
> > > > +
> > > > +	if (i2c_check_functionality(client->adapter,
> > > > +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
> > > > +		return i2c_smbus_read_i2c_block_data(client, command,
> > > > +						     length, values);
> 
> I am not very strict with the 80 char limit. I'd think the code is more
> readable if the two statements above would be oneliners. And for some
> other lines later as well.
> 
Ok. 
> > > > +	} else if (i2c_check_functionality(client->adapter,
> 
> No need for else since we return in the above if anyhow.

Ok. Will fix this.
> 
> > > > +					   I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> > > > +		for (i = 0; i < length; i += 2) {
> > > > +			status = i2c_smbus_read_word_data(client, command + i);
> > > > +			if (status < 0)
> > > > +				return status;
> > > > +			values[i] = status & 0xff;
> > > > +			if ((i + 1) < length)
> > > > +				values[i + 1] = status >> 8;
> > > > +		}
> > > > +		if (i > length)
> > > > +			return length;
> > > > +		return i;
> > > > +	} else if (i2c_check_functionality(client->adapter,
> > > > +					   I2C_FUNC_SMBUS_READ_BYTE_DATA)) {
> > > > +		for (i = 0; i < length; i++) {
> > > > +			status = i2c_smbus_read_byte_data(client, command + i);
> > > > +			if (status < 0)
> > > > +				return status;
> > > > +			values[i] = status;
> > > > +		}
> > > > +		return i;
> > > > +	}
> > > > +
> > > > +	dev_err(&client->adapter->dev, "Unsupported transactions: %d,%d,%d\n",
> > > > +		I2C_SMBUS_I2C_BLOCK_DATA, I2C_SMBUS_WORD_DATA,
> > > > +		I2C_SMBUS_BYTE_DATA);
> 
> I don't think the %d printouts are useful. Just say that the adapter
> lacks support for needed transactions. And I think the device which
> reports the error should be the client device, no?
> 

I actually looked at i2c_smbus_xfer_emulated that is printing a similar message along
with the unsupported transaction size. Probably the client should print these kind of
messages anyway, so I will remove the %d printouts. I can remove the dev_err entirely
if you think that is better.

Thanks,
Irina

> Thanks,
> 
>    Wolfram

RE: [PATCH v3 2/8] eeprom: at24: use i2c_smbus_read_i2c_block_data_or_emulated

[Tirdea, Irina]

> -----Original Message-----
> From: Wolfram Sang [mailto:wsa@the-dreams.de]
> Sent: 01 August, 2015 22:58
> To: Tirdea, Irina
> Cc: Jonathan Cameron; linux-iio@vger.kernel.org; linux-i2c@vger.kernel.org; linux-kernel@vger.kernel.org; Pandruvada, Srinivas;
> Peter Meerwald
> Subject: Re: [PATCH v3 2/8] eeprom: at24: use i2c_smbus_read_i2c_block_data_or_emulated
> 
> > +		if (at24->use_smbus) {
> > +			status =
> > +			    i2c_smbus_read_i2c_block_data_or_emulated(client,
> > +								      offset,
> > +								      count,
> > +								      buf);
> 
> Please ignore checkpatch warnings for this one and find a nice readable
> way to squeeze that into two lines.

Sure, I will fix this.

Thanks,
Irina