Re: [PATCH v3 10/13] iio: imu: inv_icm42600: add buffer support in iio devices

2020-06-14 Thread Jonathan Cameron
On Mon,  8 Jun 2020 22:42:47 +0200
Jean-Baptiste Maneyrol  wrote:

> Add all FIFO parsing and reading functions. Add accel and gyro
> kfifo buffer and FIFO data parsing. Use device interrupt for
> reading data FIFO and launching accel and gyro parsing.
> 
> Support hwfifo watermark by multiplexing gyro and accel settings.
> Support hwfifo flush.

Thanks for the added docs.  I'm not 'totally' convinced that
people only use watermarks to control expected latency.  Sometimes
they also do it because they are doing some batch based algorithm
and need a certain amount of data to feed into it.  Still latency
is probably more common, so your explanation is fine.

All patches so far look good to me.
> 
> Signed-off-by: Jean-Baptiste Maneyrol 
> ---
>  drivers/iio/imu/inv_icm42600/Kconfig  |   1 +
>  drivers/iio/imu/inv_icm42600/Makefile |   1 +
>  drivers/iio/imu/inv_icm42600/inv_icm42600.h   |   8 +
>  .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 164 -
>  .../imu/inv_icm42600/inv_icm42600_buffer.c| 573 ++
>  .../imu/inv_icm42600/inv_icm42600_buffer.h|  98 +++
>  .../iio/imu/inv_icm42600/inv_icm42600_core.c  |  30 +
>  .../iio/imu/inv_icm42600/inv_icm42600_gyro.c  | 164 -
>  8 files changed, 1037 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
>  create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h
> 
> diff --git a/drivers/iio/imu/inv_icm42600/Kconfig 
> b/drivers/iio/imu/inv_icm42600/Kconfig
> index 22390a72f0a3..50cbcfcb6cf1 100644
> --- a/drivers/iio/imu/inv_icm42600/Kconfig
> +++ b/drivers/iio/imu/inv_icm42600/Kconfig
> @@ -2,6 +2,7 @@
>  
>  config INV_ICM42600
>   tristate
> + select IIO_BUFFER
>  
>  config INV_ICM42600_I2C
>   tristate "InvenSense ICM-426xx I2C driver"
> diff --git a/drivers/iio/imu/inv_icm42600/Makefile 
> b/drivers/iio/imu/inv_icm42600/Makefile
> index 48965824f00c..0f49f6df3647 100644
> --- a/drivers/iio/imu/inv_icm42600/Makefile
> +++ b/drivers/iio/imu/inv_icm42600/Makefile
> @@ -5,6 +5,7 @@ inv-icm42600-y += inv_icm42600_core.o
>  inv-icm42600-y += inv_icm42600_gyro.o
>  inv-icm42600-y += inv_icm42600_accel.o
>  inv-icm42600-y += inv_icm42600_temp.o
> +inv-icm42600-y += inv_icm42600_buffer.o
>  
>  obj-$(CONFIG_INV_ICM42600_I2C) += inv-icm42600-i2c.o
>  inv-icm42600-i2c-y += inv_icm42600_i2c.o
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h 
> b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> index 148894c888cc..7b52d92739c3 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
> @@ -14,6 +14,8 @@
>  #include 
>  #include 
>  
> +#include "inv_icm42600_buffer.h"
> +
>  enum inv_icm42600_chip {
>   INV_CHIP_ICM42600,
>   INV_CHIP_ICM42602,
> @@ -123,6 +125,7 @@ struct inv_icm42600_suspended {
>   *  @indio_gyro: gyroscope IIO device.
>   *  @indio_accel:accelerometer IIO device.
>   *  @buffer: data transfer buffer aligned for DMA.
> + *  @fifo:   FIFO management structure.
>   */
>  struct inv_icm42600_state {
>   struct mutex lock;
> @@ -137,6 +140,7 @@ struct inv_icm42600_state {
>   struct iio_dev *indio_gyro;
>   struct iio_dev *indio_accel;
>   uint8_t buffer[2] cacheline_aligned;
> + struct inv_icm42600_fifo fifo;
>  };
>  
>  /* Virtual register addresses: @bank on MSB (4 upper bits), @address on LSB 
> */
> @@ -377,6 +381,10 @@ int inv_icm42600_core_probe(struct regmap *regmap, int 
> chip, int irq,
>  
>  struct iio_dev *inv_icm42600_gyro_init(struct inv_icm42600_state *st);
>  
> +int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev);
> +
>  struct iio_dev *inv_icm42600_accel_init(struct inv_icm42600_state *st);
>  
> +int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev);
> +
>  #endif
> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c 
> b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> index 3f214df44093..77cdad99de91 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
> @@ -11,9 +11,12 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  #include "inv_icm42600.h"
>  #include "inv_icm42600_temp.h"
> +#include "inv_icm42600_buffer.h"
>  
>  #define INV_ICM42600_ACCEL_CHAN(_modifier, _index, _ext_info)
> \
>   {   \
> @@ -64,6 +67,79 @@ static const struct iio_chan_spec 
> inv_icm42600_accel_channels[] = {
>   INV_ICM42600_TEMP_CHAN(INV_ICM42600_ACCEL_SCAN_TEMP),
>  };
>  
> +/*
> + * IIO buffer data: size must be a power of 2
> + * 8 bytes: 7 bytes data (accel 6 + temp 1) + 1 byte padding
> + */
> +struct inv_icm42600_accel_buffer {
> + struct inv_icm42600_fifo_sensor_data accel;
> + int8_t temp;
> + uint8_t padding;
> +};
> +
> +#define INV_ICM42600_SCAN_MASK_ACCEL_3AXIS   \
> +  

[PATCH v3 10/13] iio: imu: inv_icm42600: add buffer support in iio devices

2020-06-08 Thread Jean-Baptiste Maneyrol
Add all FIFO parsing and reading functions. Add accel and gyro
kfifo buffer and FIFO data parsing. Use device interrupt for
reading data FIFO and launching accel and gyro parsing.

Support hwfifo watermark by multiplexing gyro and accel settings.
Support hwfifo flush.

Signed-off-by: Jean-Baptiste Maneyrol 
---
 drivers/iio/imu/inv_icm42600/Kconfig  |   1 +
 drivers/iio/imu/inv_icm42600/Makefile |   1 +
 drivers/iio/imu/inv_icm42600/inv_icm42600.h   |   8 +
 .../iio/imu/inv_icm42600/inv_icm42600_accel.c | 164 -
 .../imu/inv_icm42600/inv_icm42600_buffer.c| 573 ++
 .../imu/inv_icm42600/inv_icm42600_buffer.h|  98 +++
 .../iio/imu/inv_icm42600/inv_icm42600_core.c  |  30 +
 .../iio/imu/inv_icm42600/inv_icm42600_gyro.c  | 164 -
 8 files changed, 1037 insertions(+), 2 deletions(-)
 create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.c
 create mode 100644 drivers/iio/imu/inv_icm42600/inv_icm42600_buffer.h

diff --git a/drivers/iio/imu/inv_icm42600/Kconfig 
b/drivers/iio/imu/inv_icm42600/Kconfig
index 22390a72f0a3..50cbcfcb6cf1 100644
--- a/drivers/iio/imu/inv_icm42600/Kconfig
+++ b/drivers/iio/imu/inv_icm42600/Kconfig
@@ -2,6 +2,7 @@
 
 config INV_ICM42600
tristate
+   select IIO_BUFFER
 
 config INV_ICM42600_I2C
tristate "InvenSense ICM-426xx I2C driver"
diff --git a/drivers/iio/imu/inv_icm42600/Makefile 
b/drivers/iio/imu/inv_icm42600/Makefile
index 48965824f00c..0f49f6df3647 100644
--- a/drivers/iio/imu/inv_icm42600/Makefile
+++ b/drivers/iio/imu/inv_icm42600/Makefile
@@ -5,6 +5,7 @@ inv-icm42600-y += inv_icm42600_core.o
 inv-icm42600-y += inv_icm42600_gyro.o
 inv-icm42600-y += inv_icm42600_accel.o
 inv-icm42600-y += inv_icm42600_temp.o
+inv-icm42600-y += inv_icm42600_buffer.o
 
 obj-$(CONFIG_INV_ICM42600_I2C) += inv-icm42600-i2c.o
 inv-icm42600-i2c-y += inv_icm42600_i2c.o
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600.h 
b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
index 148894c888cc..7b52d92739c3 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600.h
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600.h
@@ -14,6 +14,8 @@
 #include 
 #include 
 
+#include "inv_icm42600_buffer.h"
+
 enum inv_icm42600_chip {
INV_CHIP_ICM42600,
INV_CHIP_ICM42602,
@@ -123,6 +125,7 @@ struct inv_icm42600_suspended {
  *  @indio_gyro:   gyroscope IIO device.
  *  @indio_accel:  accelerometer IIO device.
  *  @buffer:   data transfer buffer aligned for DMA.
+ *  @fifo: FIFO management structure.
  */
 struct inv_icm42600_state {
struct mutex lock;
@@ -137,6 +140,7 @@ struct inv_icm42600_state {
struct iio_dev *indio_gyro;
struct iio_dev *indio_accel;
uint8_t buffer[2] cacheline_aligned;
+   struct inv_icm42600_fifo fifo;
 };
 
 /* Virtual register addresses: @bank on MSB (4 upper bits), @address on LSB */
@@ -377,6 +381,10 @@ int inv_icm42600_core_probe(struct regmap *regmap, int 
chip, int irq,
 
 struct iio_dev *inv_icm42600_gyro_init(struct inv_icm42600_state *st);
 
+int inv_icm42600_gyro_parse_fifo(struct iio_dev *indio_dev);
+
 struct iio_dev *inv_icm42600_accel_init(struct inv_icm42600_state *st);
 
+int inv_icm42600_accel_parse_fifo(struct iio_dev *indio_dev);
+
 #endif
diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c 
b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
index 3f214df44093..77cdad99de91 100644
--- a/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
+++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_accel.c
@@ -11,9 +11,12 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "inv_icm42600.h"
 #include "inv_icm42600_temp.h"
+#include "inv_icm42600_buffer.h"
 
 #define INV_ICM42600_ACCEL_CHAN(_modifier, _index, _ext_info)  \
{   \
@@ -64,6 +67,79 @@ static const struct iio_chan_spec 
inv_icm42600_accel_channels[] = {
INV_ICM42600_TEMP_CHAN(INV_ICM42600_ACCEL_SCAN_TEMP),
 };
 
+/*
+ * IIO buffer data: size must be a power of 2
+ * 8 bytes: 7 bytes data (accel 6 + temp 1) + 1 byte padding
+ */
+struct inv_icm42600_accel_buffer {
+   struct inv_icm42600_fifo_sensor_data accel;
+   int8_t temp;
+   uint8_t padding;
+};
+
+#define INV_ICM42600_SCAN_MASK_ACCEL_3AXIS \
+   (BIT(INV_ICM42600_ACCEL_SCAN_X) |   \
+   BIT(INV_ICM42600_ACCEL_SCAN_Y) |\
+   BIT(INV_ICM42600_ACCEL_SCAN_Z))
+
+#define INV_ICM42600_SCAN_MASK_TEMPBIT(INV_ICM42600_ACCEL_SCAN_TEMP)
+
+static const unsigned long inv_icm42600_accel_scan_masks[] = {
+   /* 3-axis accel + temperature */
+   INV_ICM42600_SCAN_MASK_ACCEL_3AXIS | INV_ICM42600_SCAN_MASK_TEMP,
+   0,
+};
+
+/* enable accelerometer sensor and FIFO write */
+static int inv_icm42600_accel_update_scan_mode(struct iio_dev *indio_dev,
+  const