On 18/05/16 16:00, Crestez Dan Leonard wrote:
> Using regmap_read_bulk is wrong because it assumes that a range of
> registers is being read. In our case reading from the fifo register will
> return multiple values but this is *not* auto-increment.
>
> This currently works by accident.
>
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@xxxxxxxxx>
I'd really prefer to see this in the regmap core. It really doesn't look like
it will be hard to do. Basically it's just an cut and paste job from
regmap_bulk_read.
1. Check the register in question is volatile - anything else would be bonkers.
2. The emulation of bulk reads relies on volatile or no caching anyway.
However, that will probably take a while, so I suppose we might want to take this
in the meantime as a fix - be it not a terribly urgent one as you observe it
works by accident at the moment.
Clearly it will be needed for your regcache support though.
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c | 33 ++++++++++++++++++++++++++----
> 1 file changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> index d070062..8455af0 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_ring.c
> @@ -21,6 +21,7 @@
> #include <linux/interrupt.h>
> #include <linux/kfifo.h>
> #include <linux/poll.h>
> +#include <linux/spi/spi.h>
> #include "inv_mpu_iio.h"
>
> static void inv_clear_kfifo(struct inv_mpu6050_state *st)
> @@ -128,6 +129,13 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
> u16 fifo_count;
> s64 timestamp;
>
> + struct device *regmap_dev = regmap_get_device(st->map);
> + struct i2c_client *i2c;
> + struct spi_device *spi = NULL;
> +
> + i2c = i2c_verify_client(regmap_dev);
> + spi = i2c ? NULL: to_spi_device(regmap_dev);
> +
> mutex_lock(&indio_dev->mlock);
> if (!(st->chip_config.accl_fifo_enable |
> st->chip_config.gyro_fifo_enable))
> @@ -160,10 +168,27 @@ irqreturn_t inv_mpu6050_read_fifo(int irq, void *p)
> fifo_count / bytes_per_datum + INV_MPU6050_TIME_STAMP_TOR)
> goto flush_fifo;
> while (fifo_count >= bytes_per_datum) {
> - result = regmap_bulk_read(st->map, st->reg->fifo_r_w,
> - data, bytes_per_datum);
> - if (result)
> - goto flush_fifo;
> + /*
> + * We need to do a large burst read from a single register.
> + *
> + * regmap_read_bulk assumes that multiple registers are
> + * involved but in our case st->reg->fifo_r_w + 1 is something
> + * completely unrelated.
> + */
> + if (spi) {
> + u8 cmd = st->reg->fifo_r_w | 0x80;
> + result = spi_write_then_read(spi,
> + &cmd, 1,
> + data, bytes_per_datum);
> + if (result)
> + goto flush_fifo;
> + } else {
> + result = i2c_smbus_read_i2c_block_data(i2c,
> + st->reg->fifo_r_w,
> + bytes_per_datum, data);
> + if (result != bytes_per_datum)
> + goto flush_fifo;
> + }
>
> result = kfifo_out(&st->timestamps, ×tamp, 1);
> /* when there is no timestamp, put timestamp as 0 */
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
