RE: [PATCH] iio: gyro: bmg160: optimize i2c transfers in trigger handler

From: Tirdea, Irina
Date: Mon Feb 16 2015 - 12:59:07 EST




> -----Original Message-----
> From: Pandruvada, Srinivas
> Sent: 04 February, 2015 22:39
> To: jic23@xxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-iio@xxxxxxxxxxxxxxx; Tirdea, Irina; viorel.suman@xxxxxxxxx
> Subject: Re: [PATCH] iio: gyro: bmg160: optimize i2c transfers in trigger handler
>
> On Wed, 2015-02-04 at 17:27 +0000, Jonathan Cameron wrote:
> > On 30/01/15 04:03, Viorel Suman wrote:
> > > Hi,
> > >
> > > You might need more space in "values" buffer, for more details please check the
> > > "iio_push_to_buffers_with_timestamp" description and implementation.
> > Whilst that function is a little 'odd', this patch doesn't change the use
> > of buffer (simply how it if filled). The buffer is 16bytes long
> > (8 x s16) so should be fine (4 x 16 bit and 1 x 64 bit).
> >
> > More problematically, we have using a new smbus capability here
> > (to read it one transaction). It's one you can check for, so I'm guessing
> > we aren't guaranteed it is present.
> > I2C_FUNC_SMBUS_READ_I2C_BLOCK
> Good catch. We should add
> if (i2c_check_functionality(client->adapter,
> I2C_FUNC_SMBUS_READ_I2C_BLOCK) in probe and use this otherwise revert to
> old method.
>
Thanks for the review, Jonathan and Srinivas!
Will fix this in next version.
> Thanks,
> Srinivas
>
> >
> > Jonathan
> > >
> > > Regards,
> > > Viorel
> > >
> > >
> > > On Thu, Jan 29, 2015 at 3:15 PM, Irina Tirdea <irina.tirdea@xxxxxxxxx <mailto:irina.tirdea@xxxxxxxxx>> 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.
> > >
> > > Signed-off-by: Irina Tirdea <irina.tirdea@xxxxxxxxx <mailto:irina.tirdea@xxxxxxxxx>>
> > > Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx <mailto:srinivas.pandruvada@xxxxxxxxxxxxxxx>>
> > > ---
> > > drivers/iio/gyro/bmg160.c | 20 +++++++++++---------
> > > 1 file changed, 11 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/iio/gyro/bmg160.c b/drivers/iio/gyro/bmg160.c
> > > index 60451b3..814eb20 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 {
> > > @@ -820,20 +821,21 @@ static irqreturn_t bmg160_trigger_handler(int irq, void *p)
> > > struct iio_dev *indio_dev = pf->indio_dev;
> > > struct bmg160_data *data = iio_priv(indio_dev);
> > > int bit, ret, i = 0;
> > > + s16 values[AXIS_MAX];
> > >
> > > mutex_lock(&data->mutex);
> > > - for_each_set_bit(bit, indio_dev->buffer->scan_mask,
> > > - indio_dev->masklength) {
> > > - 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(data->client, BMG160_REG_XOUT_L,
> > > + sizeof(values), (u8 *) values);
> > > + if (ret < 0) {
> > > + mutex_unlock(&data->mutex);
> > > + goto err;
> > > }
> > > mutex_unlock(&data->mutex);
> > >
> > > + for_each_set_bit(bit, indio_dev->buffer->scan_mask,
> > > + indio_dev->masklength)
> > > + data->buffer[i++] = values[bit];
> > > +
> > > iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> > > data->timestamp);
> > > err:
> > > --
> > > 1.9.1
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> > > the body of a message to majordomo@xxxxxxxxxxxxxxx <mailto:majordomo@xxxxxxxxxxxxxxx>
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > >
> > >
> >

N‹§²æ¸›yú²X¬¶ÇvØ–)Þ{.nlj·¥Š{±‘êX§¶›¡Ü}©ž²ÆzÚj:+v‰¨¾«‘êZ+€Êzf£¢·hšˆ§~†­†Ûÿû®w¥¢¸?™¨è&¢)ßf”ùy§m…á«a¶Úÿ 0¶ìå