* [PATCH v3 0/8] Add support for best effort block read emulation @ 2015-07-03 9:33 Irina Tirdea 2015-07-03 9:33 ` [PATCH v3 5/8] iio: gyro: bmg160: use available_scan_masks Irina Tirdea ` (2 more replies) 0 siblings, 3 replies; 23+ messages in thread From: Irina Tirdea @ 2015-07-03 9:33 UTC (permalink / raw) To: Wolfram Sang, Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald, 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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 5/8] iio: gyro: bmg160: use available_scan_masks 2015-07-03 9:33 [PATCH v3 0/8] Add support for best effort block read emulation Irina Tirdea @ 2015-07-03 9:33 ` Irina Tirdea [not found] ` <1435916017-12859-1-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-03 9:33 ` [PATCH v3 7/8] iio: accel: kxcjk-1013: use available_scan_masks Irina Tirdea 2 siblings, 0 replies; 23+ messages in thread From: Irina Tirdea @ 2015-07-03 9:33 UTC (permalink / raw) To: Wolfram Sang, Jonathan Cameron, linux-iio, linux-i2c Cc: linux-kernel, Srinivas Pandruvada, Peter Meerwald, 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <1435916017-12859-1-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation [not found] ` <1435916017-12859-1-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2015-07-03 9:33 ` Irina Tirdea [not found] ` <1435916017-12859-2-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-03 9:33 ` [PATCH v3 2/8] eeprom: at24: use i2c_smbus_read_i2c_block_data_or_emulated Irina Tirdea ` (4 subsequent siblings) 5 siblings, 1 reply; 23+ messages in thread From: Irina Tirdea @ 2015-07-03 9:33 UTC (permalink / raw) To: Wolfram Sang, Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald, 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <1435916017-12859-2-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation [not found] ` <1435916017-12859-2-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2015-07-05 11:58 ` Jonathan Cameron [not found] ` <55991BE7.6050705-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Jonathan Cameron @ 2015-07-05 11:58 UTC (permalink / raw) To: Irina Tirdea, Wolfram Sang, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 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 */ > > /** > ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <55991BE7.6050705-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]
* RE: [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation [not found] ` <55991BE7.6050705-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> @ 2015-07-10 17:14 ` Tirdea, Irina [not found] ` <1F3AC3675D538145B1661F571FE1805F2F067FA4-pww93C2UFcwu0RiL9chJVbfspsVTdybXVpNB7YpNyf8@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Tirdea, Irina @ 2015-07-10 17:14 UTC (permalink / raw) To: Jonathan Cameron, Wolfram Sang Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Pandruvada, Srinivas, Peter Meerwald, linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > -----Original Message----- > From: Jonathan Cameron [mailto:jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org] > Sent: 05 July, 2015 14:59 > To: Tirdea, Irina; Wolfram Sang; linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > 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 */ > > > > /** > > ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <1F3AC3675D538145B1661F571FE1805F2F067FA4-pww93C2UFcwu0RiL9chJVbfspsVTdybXVpNB7YpNyf8@public.gmane.org>]
* Re: [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation [not found] ` <1F3AC3675D538145B1661F571FE1805F2F067FA4-pww93C2UFcwu0RiL9chJVbfspsVTdybXVpNB7YpNyf8@public.gmane.org> @ 2015-07-11 17:40 ` Jonathan Cameron 2015-08-01 19:53 ` Wolfram Sang 1 sibling, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2015-07-11 17:40 UTC (permalink / raw) To: Tirdea, Irina, Wolfram Sang Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Pandruvada, Srinivas, Peter Meerwald, linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On 10/07/15 18:14, Tirdea, Irina wrote: > > >> -----Original Message----- >> From: Jonathan Cameron [mailto:jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org] >> Sent: 05 July, 2015 14:59 >> To: Tirdea, Irina; Wolfram Sang; linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org >> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> >> 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 */ >>> >>> /** >>> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation [not found] ` <1F3AC3675D538145B1661F571FE1805F2F067FA4-pww93C2UFcwu0RiL9chJVbfspsVTdybXVpNB7YpNyf8@public.gmane.org> 2015-07-11 17:40 ` Jonathan Cameron @ 2015-08-01 19:53 ` Wolfram Sang 2015-08-04 13:51 ` Tirdea, Irina 1 sibling, 1 reply; 23+ messages in thread From: Wolfram Sang @ 2015-08-01 19:53 UTC (permalink / raw) To: Tirdea, Irina Cc: Jonathan Cameron, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Pandruvada, Srinivas, Peter Meerwald, linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [-- 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 --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation 2015-08-01 19:53 ` Wolfram Sang @ 2015-08-04 13:51 ` Tirdea, Irina 0 siblings, 0 replies; 23+ messages in thread From: Tirdea, Irina @ 2015-08-04 13:51 UTC (permalink / raw) To: Wolfram Sang Cc: Jonathan Cameron, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Pandruvada, Srinivas, Peter Meerwald, linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > -----Original Message----- > From: linux-iio-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-iio-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Wolfram Sang > Sent: 01 August, 2015 22:53 > To: Tirdea, Irina > Cc: Jonathan Cameron; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Pandruvada, Srinivas; Peter Meerwald; linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux- > i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 2/8] eeprom: at24: use i2c_smbus_read_i2c_block_data_or_emulated [not found] ` <1435916017-12859-1-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-03 9:33 ` [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation Irina Tirdea @ 2015-07-03 9:33 ` Irina Tirdea [not found] ` <1435916017-12859-3-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-03 9:33 ` [PATCH v3 3/8] iio: accel: bmc150: use available_scan_masks Irina Tirdea ` (3 subsequent siblings) 5 siblings, 1 reply; 23+ messages in thread From: Irina Tirdea @ 2015-07-03 9:33 UTC (permalink / raw) To: Wolfram Sang, Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald, 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <1435916017-12859-3-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 2/8] eeprom: at24: use i2c_smbus_read_i2c_block_data_or_emulated [not found] ` <1435916017-12859-3-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2015-08-01 19:57 ` Wolfram Sang 2015-08-04 13:52 ` Tirdea, Irina 0 siblings, 1 reply; 23+ messages in thread From: Wolfram Sang @ 2015-08-01 19:57 UTC (permalink / raw) To: Irina Tirdea Cc: Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald [-- 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 --] ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v3 2/8] eeprom: at24: use i2c_smbus_read_i2c_block_data_or_emulated 2015-08-01 19:57 ` Wolfram Sang @ 2015-08-04 13:52 ` Tirdea, Irina 0 siblings, 0 replies; 23+ messages in thread From: Tirdea, Irina @ 2015-08-04 13:52 UTC (permalink / raw) To: Wolfram Sang Cc: Jonathan Cameron, linux-iio@vger.kernel.org, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, Pandruvada, Srinivas, Peter Meerwald > -----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 ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 3/8] iio: accel: bmc150: use available_scan_masks [not found] ` <1435916017-12859-1-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-03 9:33 ` [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation Irina Tirdea 2015-07-03 9:33 ` [PATCH v3 2/8] eeprom: at24: use i2c_smbus_read_i2c_block_data_or_emulated Irina Tirdea @ 2015-07-03 9:33 ` Irina Tirdea [not found] ` <1435916017-12859-4-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-03 9:33 ` [PATCH v3 4/8] iio: accel: bmc150: optimize i2c transfers in trigger handler Irina Tirdea ` (2 subsequent siblings) 5 siblings, 1 reply; 23+ messages in thread From: Irina Tirdea @ 2015-07-03 9:33 UTC (permalink / raw) To: Wolfram Sang, Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald, 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.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; -- 1.9.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <1435916017-12859-4-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 3/8] iio: accel: bmc150: use available_scan_masks [not found] ` <1435916017-12859-4-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2015-07-05 12:10 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2015-07-05 12:10 UTC (permalink / raw) To: Irina Tirdea, Wolfram Sang, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 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-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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; > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 4/8] iio: accel: bmc150: optimize i2c transfers in trigger handler [not found] ` <1435916017-12859-1-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> ` (2 preceding siblings ...) 2015-07-03 9:33 ` [PATCH v3 3/8] iio: accel: bmc150: use available_scan_masks Irina Tirdea @ 2015-07-03 9:33 ` Irina Tirdea 2015-07-05 12:06 ` Jonathan Cameron 2015-07-03 9:33 ` [PATCH v3 6/8] iio: gyro: bmg160: " Irina Tirdea 2015-07-03 9:33 ` [PATCH v3 8/8] iio: accel: kxcjk-1013: " Irina Tirdea 5 siblings, 1 reply; 23+ messages in thread From: Irina Tirdea @ 2015-07-03 9:33 UTC (permalink / raw) To: Wolfram Sang, Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald, 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.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, \ }, \ .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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 4/8] iio: accel: bmc150: optimize i2c transfers in trigger handler 2015-07-03 9:33 ` [PATCH v3 4/8] iio: accel: bmc150: optimize i2c transfers in trigger handler Irina Tirdea @ 2015-07-05 12:06 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2015-07-05 12:06 UTC (permalink / raw) To: Irina Tirdea, Wolfram Sang, linux-iio, linux-i2c Cc: linux-kernel, Srinivas Pandruvada, Peter Meerwald 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); > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 6/8] iio: gyro: bmg160: optimize i2c transfers in trigger handler [not found] ` <1435916017-12859-1-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> ` (3 preceding siblings ...) 2015-07-03 9:33 ` [PATCH v3 4/8] iio: accel: bmc150: optimize i2c transfers in trigger handler Irina Tirdea @ 2015-07-03 9:33 ` Irina Tirdea [not found] ` <1435916017-12859-7-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-03 9:33 ` [PATCH v3 8/8] iio: accel: kxcjk-1013: " Irina Tirdea 5 siblings, 1 reply; 23+ messages in thread From: Irina Tirdea @ 2015-07-03 9:33 UTC (permalink / raw) To: Wolfram Sang, Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald, 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> --- 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <1435916017-12859-7-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 6/8] iio: gyro: bmg160: optimize i2c transfers in trigger handler [not found] ` <1435916017-12859-7-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2015-07-05 12:08 ` Jonathan Cameron 2015-07-10 17:31 ` Tirdea, Irina 0 siblings, 1 reply; 23+ messages in thread From: Jonathan Cameron @ 2015-07-05 12:08 UTC (permalink / raw) To: Irina Tirdea, Wolfram Sang, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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); > ^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [PATCH v3 6/8] iio: gyro: bmg160: optimize i2c transfers in trigger handler 2015-07-05 12:08 ` Jonathan Cameron @ 2015-07-10 17:31 ` Tirdea, Irina [not found] ` <1F3AC3675D538145B1661F571FE1805F2F068001-pww93C2UFcwu0RiL9chJVbfspsVTdybXVpNB7YpNyf8@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Tirdea, Irina @ 2015-07-10 17:31 UTC (permalink / raw) To: Pandruvada, Srinivas, Jonathan Cameron Cc: linux-kernel@vger.kernel.org, Peter Meerwald, Wolfram Sang, linux-iio@vger.kernel.org, linux-i2c@vger.kernel.org > -----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); > > ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <1F3AC3675D538145B1661F571FE1805F2F068001-pww93C2UFcwu0RiL9chJVbfspsVTdybXVpNB7YpNyf8@public.gmane.org>]
* Re: [PATCH v3 6/8] iio: gyro: bmg160: optimize i2c transfers in trigger handler [not found] ` <1F3AC3675D538145B1661F571FE1805F2F068001-pww93C2UFcwu0RiL9chJVbfspsVTdybXVpNB7YpNyf8@public.gmane.org> @ 2015-07-10 17:46 ` Pandruvada, Srinivas 0 siblings, 0 replies; 23+ messages in thread From: Pandruvada, Srinivas @ 2015-07-10 17:46 UTC (permalink / raw) To: Tirdea, Irina Cc: wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org, jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 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); > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 8/8] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler [not found] ` <1435916017-12859-1-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> ` (4 preceding siblings ...) 2015-07-03 9:33 ` [PATCH v3 6/8] iio: gyro: bmg160: " Irina Tirdea @ 2015-07-03 9:33 ` Irina Tirdea [not found] ` <1435916017-12859-9-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 5 siblings, 1 reply; 23+ messages in thread From: Irina Tirdea @ 2015-07-03 9:33 UTC (permalink / raw) To: Wolfram Sang, Jonathan Cameron, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald, Adriana Reus, Irina Tirdea From: Adriana Reus <adriana.reus-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> Signed-off-by: Irina Tirdea <irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.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); -- 1.9.1 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <1435916017-12859-9-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 8/8] iio: accel: kxcjk-1013: optimize i2c transfers in trigger handler [not found] ` <1435916017-12859-9-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2015-07-05 12:11 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2015-07-05 12:11 UTC (permalink / raw) To: Irina Tirdea, Wolfram Sang, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald, Adriana Reus On 03/07/15 10:33, Irina Tirdea wrote: > From: Adriana Reus <adriana.reus-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > > 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > Signed-off-by: Irina Tirdea <irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> Same comments as previously. Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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); > ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 7/8] iio: accel: kxcjk-1013: use available_scan_masks 2015-07-03 9:33 [PATCH v3 0/8] Add support for best effort block read emulation Irina Tirdea 2015-07-03 9:33 ` [PATCH v3 5/8] iio: gyro: bmg160: use available_scan_masks Irina Tirdea [not found] ` <1435916017-12859-1-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2015-07-03 9:33 ` Irina Tirdea [not found] ` <1435916017-12859-8-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2 siblings, 1 reply; 23+ messages in thread From: Irina Tirdea @ 2015-07-03 9:33 UTC (permalink / raw) To: Wolfram Sang, Jonathan Cameron, linux-iio, linux-i2c Cc: linux-kernel, Srinivas Pandruvada, Peter Meerwald, Adriana Reus, 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 ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <1435916017-12859-8-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 7/8] iio: accel: kxcjk-1013: use available_scan_masks [not found] ` <1435916017-12859-8-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> @ 2015-07-05 12:10 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2015-07-05 12:10 UTC (permalink / raw) To: Irina Tirdea, Wolfram Sang, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Srinivas Pandruvada, Peter Meerwald, Adriana Reus On 03/07/15 10:33, Irina Tirdea wrote: > From: Adriana Reus <adriana.reus-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > > 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-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> > Signed-off-by: Irina Tirdea <irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.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; > ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2015-08-04 13:52 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-07-03 9:33 [PATCH v3 0/8] Add support for best effort block read emulation Irina Tirdea 2015-07-03 9:33 ` [PATCH v3 5/8] iio: gyro: bmg160: use available_scan_masks Irina Tirdea [not found] ` <1435916017-12859-1-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-03 9:33 ` [PATCH v3 1/8] i2c: core: Add support for best effort block read emulation Irina Tirdea [not found] ` <1435916017-12859-2-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-05 11:58 ` Jonathan Cameron [not found] ` <55991BE7.6050705-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> 2015-07-10 17:14 ` Tirdea, Irina [not found] ` <1F3AC3675D538145B1661F571FE1805F2F067FA4-pww93C2UFcwu0RiL9chJVbfspsVTdybXVpNB7YpNyf8@public.gmane.org> 2015-07-11 17:40 ` Jonathan Cameron 2015-08-01 19:53 ` Wolfram Sang 2015-08-04 13:51 ` Tirdea, Irina 2015-07-03 9:33 ` [PATCH v3 2/8] eeprom: at24: use i2c_smbus_read_i2c_block_data_or_emulated Irina Tirdea [not found] ` <1435916017-12859-3-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-08-01 19:57 ` Wolfram Sang 2015-08-04 13:52 ` Tirdea, Irina 2015-07-03 9:33 ` [PATCH v3 3/8] iio: accel: bmc150: use available_scan_masks Irina Tirdea [not found] ` <1435916017-12859-4-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-05 12:10 ` Jonathan Cameron 2015-07-03 9:33 ` [PATCH v3 4/8] iio: accel: bmc150: optimize i2c transfers in trigger handler Irina Tirdea 2015-07-05 12:06 ` Jonathan Cameron 2015-07-03 9:33 ` [PATCH v3 6/8] iio: gyro: bmg160: " Irina Tirdea [not found] ` <1435916017-12859-7-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-05 12:08 ` Jonathan Cameron 2015-07-10 17:31 ` Tirdea, Irina [not found] ` <1F3AC3675D538145B1661F571FE1805F2F068001-pww93C2UFcwu0RiL9chJVbfspsVTdybXVpNB7YpNyf8@public.gmane.org> 2015-07-10 17:46 ` Pandruvada, Srinivas 2015-07-03 9:33 ` [PATCH v3 8/8] iio: accel: kxcjk-1013: " Irina Tirdea [not found] ` <1435916017-12859-9-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-05 12:11 ` Jonathan Cameron 2015-07-03 9:33 ` [PATCH v3 7/8] iio: accel: kxcjk-1013: use available_scan_masks Irina Tirdea [not found] ` <1435916017-12859-8-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2015-07-05 12:10 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).