Re: [PATCH v6 4/6] iio:adxl372: Add FIFO and interrupts support

2018-08-19 Thread Jonathan Cameron
On Sun, 19 Aug 2018 18:12:18 +0100
Jonathan Cameron  wrote:

> On Fri, 10 Aug 2018 11:46:21 +0300
> Stefan Popa  wrote:
> 
> > This patch adds support for the adxl372 FIFO. In order to accomplish this,
> > triggered buffers were used.
> > 
> > The number of FIFO samples which trigger the watermark interrupt can be
> > configured by using the buffer watermark. The FIFO format is determined by
> > configuring the scan elements for each axis. The FIFO data is pushed to the
> > IIO device's buffer.
> > 
> > Signed-off-by: Stefan Popa   
> 
> one minor item I'd missed previously.  I'll fix it whilst applying.
I was wrong on this, so no need to make that change.
> 
> Applied to the togreg branch of iio.git and pushed out as testing
> to see what we've all missed that the autobuilders might find.
Dropped for now because of issues in the other email I just sent.
I'll hold the rest of the series until those are sorted.

Jonathan

> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  drivers/iio/accel/adxl372.c | 357 
> > +++-
> >  1 file changed, 356 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> > index db9ecd2..1e2519a 100644
> > --- a/drivers/iio/accel/adxl372.c
> > +++ b/drivers/iio/accel/adxl372.c
> > @@ -6,12 +6,19 @@
> >   */
> >  
> >  #include 
> > +#include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> >  
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> >  
> >  /* ADXL372 registers definition */
> >  #define ADXL372_DEVID  0x00
> > @@ -123,6 +130,9 @@
> >  #define ADXL372_INT1_MAP_LOW_MSK   BIT(7)
> >  #define ADXL372_INT1_MAP_LOW_MODE(x)   (((x) & 0x1) << 7)
> >  
> > +/* The ADXL372 includes a deep, 512 sample FIFO buffer */
> > +#define ADXL372_FIFO_SIZE  512
> > +
> >  /*
> >   * At +/- 200g with 12-bit resolution, scale is computed as:
> >   * (200 + 200) * 9.81 / (2^12 - 1) = 0.958241
> > @@ -170,6 +180,43 @@ static const unsigned int adxl372_th_reg_high_addr[3] 
> > = {
> > [ADXL372_INACTIVITY] = ADXL372_X_THRESH_INACT_H,
> >  };
> >  
> > +enum adxl372_fifo_format {
> > +   ADXL372_XYZ_FIFO,
> > +   ADXL372_X_FIFO,
> > +   ADXL372_Y_FIFO,
> > +   ADXL372_XY_FIFO,
> > +   ADXL372_Z_FIFO,
> > +   ADXL372_XZ_FIFO,
> > +   ADXL372_YZ_FIFO,
> > +   ADXL372_XYZ_PEAK_FIFO,
> > +};
> > +
> > +enum adxl372_fifo_mode {
> > +   ADXL372_FIFO_BYPASSED,
> > +   ADXL372_FIFO_STREAMED,
> > +   ADXL372_FIFO_TRIGGERED,
> > +   ADXL372_FIFO_OLD_SAVED
> > +};
> > +
> > +static const int adxl372_samp_freq_tbl[5] = {
> > +   400, 800, 1600, 3200, 6400,
> > +};
> > +
> > +struct adxl372_axis_lookup {
> > +   unsigned int bits;
> > +   enum adxl372_fifo_format fifo_format;
> > +};
> > +
> > +static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
> > +   { BIT(0), ADXL372_X_FIFO },
> > +   { BIT(1), ADXL372_Y_FIFO },
> > +   { BIT(2), ADXL372_Z_FIFO },
> > +   { BIT(0) | BIT(1), ADXL372_XY_FIFO },
> > +   { BIT(0) | BIT(2), ADXL372_XZ_FIFO },
> > +   { BIT(1) | BIT(2), ADXL372_YZ_FIFO },
> > +   { BIT(0) | BIT(1) | BIT(2), ADXL372_XYZ_FIFO },
> > +};
> > +
> >  #define ADXL372_ACCEL_CHANNEL(index, reg, axis) {  \
> > .type = IIO_ACCEL,  \
> > .address = reg, \
> > @@ -177,6 +224,13 @@ static const unsigned int adxl372_th_reg_high_addr[3] 
> > = {
> > .channel2 = IIO_MOD_##axis, \
> > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
> > .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
> > +   .scan_index = index,\
> > +   .scan_type = {  \
> > +   .sign = 's',\
> > +   .realbits = 12, \
> > +   .storagebits = 16,  \
> > +   .shift = 4, \
> > +   },  \
> >  }
> >  
> >  static const struct iio_chan_spec adxl372_channels[] = {
> > @@ -188,12 +242,29 @@ static const struct iio_chan_spec adxl372_channels[] 
> > = {
> >  struct adxl372_state {
> > struct spi_device   *spi;
> > struct regmap   *regmap;
> > +   struct iio_trigger  *dready_trig;
> > +   enum adxl372_fifo_mode  fifo_mode;
> > +   enum adxl372_fifo_formatfifo_format;
> > enum adxl372_op_modeop_mode;
> > enum adxl372_act_proc_mode  act_proc_mode;
> > enum adxl372_odrodr;
> > enum adxl372_bandwidth  bw;
> > u32 act_time_ms;
> > u32  

Re: [PATCH v6 4/6] iio:adxl372: Add FIFO and interrupts support

2018-08-19 Thread Jonathan Cameron
On Fri, 10 Aug 2018 11:46:21 +0300
Stefan Popa  wrote:

> This patch adds support for the adxl372 FIFO. In order to accomplish this,
> triggered buffers were used.
> 
> The number of FIFO samples which trigger the watermark interrupt can be
> configured by using the buffer watermark. The FIFO format is determined by
> configuring the scan elements for each axis. The FIFO data is pushed to the
> IIO device's buffer.
> 
> Signed-off-by: Stefan Popa 

Hi Stefan,  I'm afraid I missed a few things on previous versions of this.

If we are going to expose a general purpose trigger or allow a driver
to use a general purpose trigger (this currently does both as neither validate
function is provided to limit it's usage), then the trigger 'must' correspond
to one interrupt per reading.  It's absolutely fine to have a device specific
trigger where that isn't true though normally we would only bother exposing it
as a trigger if there as an alternative.

A classic case would be that we have a fifo and use it if the trigger is the
devices only trigger, but in the event of a different trigger being selected
we are happy to use it with the fifo length set to 1 (or it just turned off).

That allows the use of say a sysfs of hrtimer trigger with us getting the 
expected
1 sample set per trigger, and the use of the fifo for this device alone.
To do that you have make sure you have a validate_device callback provided
for the trigger so it rejects other devices.

We can just forgo alternative triggers and have the buffer driven directly.
I don't really like that option though as it leads to abi changes if we ever
want to add alternatives in future though we can sort of get around that using
default triggers.

So the simple solution for now is to provide validate_trigger and 
validate_device
callbacks to fix it to use them only when they are the same device.

We can do clever stuff later if needed to enable other triggers.

Jonathan

> ---
>  drivers/iio/accel/adxl372.c | 357 
> +++-
>  1 file changed, 356 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index db9ecd2..1e2519a 100644
> --- a/drivers/iio/accel/adxl372.c
> +++ b/drivers/iio/accel/adxl372.c
> @@ -6,12 +6,19 @@
>   */
>  
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
>  
>  #include 
>  #include 
> +#include 
> +#include 

Why events?

> +#include 
> +#include 
> +#include 
>  
>  /* ADXL372 registers definition */
>  #define ADXL372_DEVID0x00
> @@ -123,6 +130,9 @@
>  #define ADXL372_INT1_MAP_LOW_MSK BIT(7)
>  #define ADXL372_INT1_MAP_LOW_MODE(x) (((x) & 0x1) << 7)
>  
> +/* The ADXL372 includes a deep, 512 sample FIFO buffer */
> +#define ADXL372_FIFO_SIZE512
> +
>  /*
>   * At +/- 200g with 12-bit resolution, scale is computed as:
>   * (200 + 200) * 9.81 / (2^12 - 1) = 0.958241
> @@ -170,6 +180,43 @@ static const unsigned int adxl372_th_reg_high_addr[3] = {
>   [ADXL372_INACTIVITY] = ADXL372_X_THRESH_INACT_H,
>  };
>  
> +enum adxl372_fifo_format {
> + ADXL372_XYZ_FIFO,
> + ADXL372_X_FIFO,
> + ADXL372_Y_FIFO,
> + ADXL372_XY_FIFO,
> + ADXL372_Z_FIFO,
> + ADXL372_XZ_FIFO,
> + ADXL372_YZ_FIFO,
> + ADXL372_XYZ_PEAK_FIFO,
> +};
> +
> +enum adxl372_fifo_mode {
> + ADXL372_FIFO_BYPASSED,
> + ADXL372_FIFO_STREAMED,
> + ADXL372_FIFO_TRIGGERED,
> + ADXL372_FIFO_OLD_SAVED
> +};
> +
> +static const int adxl372_samp_freq_tbl[5] = {
> + 400, 800, 1600, 3200, 6400,
> +};
> +
> +struct adxl372_axis_lookup {
> + unsigned int bits;
> + enum adxl372_fifo_format fifo_format;
> +};
> +
> +static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
> + { BIT(0), ADXL372_X_FIFO },
> + { BIT(1), ADXL372_Y_FIFO },
> + { BIT(2), ADXL372_Z_FIFO },
> + { BIT(0) | BIT(1), ADXL372_XY_FIFO },
> + { BIT(0) | BIT(2), ADXL372_XZ_FIFO },
> + { BIT(1) | BIT(2), ADXL372_YZ_FIFO },
> + { BIT(0) | BIT(1) | BIT(2), ADXL372_XYZ_FIFO },
> +};
> +
>  #define ADXL372_ACCEL_CHANNEL(index, reg, axis) {\
>   .type = IIO_ACCEL,  \
>   .address = reg, \
> @@ -177,6 +224,13 @@ static const unsigned int adxl372_th_reg_high_addr[3] = {
>   .channel2 = IIO_MOD_##axis, \
>   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
>   .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
> + .scan_index = index,\
> + .scan_type = {  \
> + .sign = 's',\
> + .realbits = 12, \
> + .storagebits = 16,   

Re: [PATCH v6 4/6] iio:adxl372: Add FIFO and interrupts support

2018-08-19 Thread Jonathan Cameron
On Fri, 10 Aug 2018 11:46:21 +0300
Stefan Popa  wrote:

> This patch adds support for the adxl372 FIFO. In order to accomplish this,
> triggered buffers were used.
> 
> The number of FIFO samples which trigger the watermark interrupt can be
> configured by using the buffer watermark. The FIFO format is determined by
> configuring the scan elements for each axis. The FIFO data is pushed to the
> IIO device's buffer.
> 
> Signed-off-by: Stefan Popa 

one minor item I'd missed previously.  I'll fix it whilst applying.

Applied to the togreg branch of iio.git and pushed out as testing
to see what we've all missed that the autobuilders might find.

Thanks,

Jonathan

> ---
>  drivers/iio/accel/adxl372.c | 357 
> +++-
>  1 file changed, 356 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index db9ecd2..1e2519a 100644
> --- a/drivers/iio/accel/adxl372.c
> +++ b/drivers/iio/accel/adxl372.c
> @@ -6,12 +6,19 @@
>   */
>  
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
>  
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
>  
>  /* ADXL372 registers definition */
>  #define ADXL372_DEVID0x00
> @@ -123,6 +130,9 @@
>  #define ADXL372_INT1_MAP_LOW_MSK BIT(7)
>  #define ADXL372_INT1_MAP_LOW_MODE(x) (((x) & 0x1) << 7)
>  
> +/* The ADXL372 includes a deep, 512 sample FIFO buffer */
> +#define ADXL372_FIFO_SIZE512
> +
>  /*
>   * At +/- 200g with 12-bit resolution, scale is computed as:
>   * (200 + 200) * 9.81 / (2^12 - 1) = 0.958241
> @@ -170,6 +180,43 @@ static const unsigned int adxl372_th_reg_high_addr[3] = {
>   [ADXL372_INACTIVITY] = ADXL372_X_THRESH_INACT_H,
>  };
>  
> +enum adxl372_fifo_format {
> + ADXL372_XYZ_FIFO,
> + ADXL372_X_FIFO,
> + ADXL372_Y_FIFO,
> + ADXL372_XY_FIFO,
> + ADXL372_Z_FIFO,
> + ADXL372_XZ_FIFO,
> + ADXL372_YZ_FIFO,
> + ADXL372_XYZ_PEAK_FIFO,
> +};
> +
> +enum adxl372_fifo_mode {
> + ADXL372_FIFO_BYPASSED,
> + ADXL372_FIFO_STREAMED,
> + ADXL372_FIFO_TRIGGERED,
> + ADXL372_FIFO_OLD_SAVED
> +};
> +
> +static const int adxl372_samp_freq_tbl[5] = {
> + 400, 800, 1600, 3200, 6400,
> +};
> +
> +struct adxl372_axis_lookup {
> + unsigned int bits;
> + enum adxl372_fifo_format fifo_format;
> +};
> +
> +static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
> + { BIT(0), ADXL372_X_FIFO },
> + { BIT(1), ADXL372_Y_FIFO },
> + { BIT(2), ADXL372_Z_FIFO },
> + { BIT(0) | BIT(1), ADXL372_XY_FIFO },
> + { BIT(0) | BIT(2), ADXL372_XZ_FIFO },
> + { BIT(1) | BIT(2), ADXL372_YZ_FIFO },
> + { BIT(0) | BIT(1) | BIT(2), ADXL372_XYZ_FIFO },
> +};
> +
>  #define ADXL372_ACCEL_CHANNEL(index, reg, axis) {\
>   .type = IIO_ACCEL,  \
>   .address = reg, \
> @@ -177,6 +224,13 @@ static const unsigned int adxl372_th_reg_high_addr[3] = {
>   .channel2 = IIO_MOD_##axis, \
>   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
>   .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
> + .scan_index = index,\
> + .scan_type = {  \
> + .sign = 's',\
> + .realbits = 12, \
> + .storagebits = 16,  \
> + .shift = 4, \
> + },  \
>  }
>  
>  static const struct iio_chan_spec adxl372_channels[] = {
> @@ -188,12 +242,29 @@ static const struct iio_chan_spec adxl372_channels[] = {
>  struct adxl372_state {
>   struct spi_device   *spi;
>   struct regmap   *regmap;
> + struct iio_trigger  *dready_trig;
> + enum adxl372_fifo_mode  fifo_mode;
> + enum adxl372_fifo_formatfifo_format;
>   enum adxl372_op_modeop_mode;
>   enum adxl372_act_proc_mode  act_proc_mode;
>   enum adxl372_odrodr;
>   enum adxl372_bandwidth  bw;
>   u32 act_time_ms;
>   u32 inact_time_ms;
> + u8  fifo_set_size;
> + u8  int1_bitmask;
> + u8  int2_bitmask;
> + u16 watermark;
> + __be16  fifo_buf[ADXL372_FIFO_SIZE];
> +};
> +
> +static const unsigned long adxl372_channel_masks[] = {
> + BIT(0), BIT(1), BIT(2),
> + BIT(0)

[PATCH v6 4/6] iio:adxl372: Add FIFO and interrupts support

2018-08-10 Thread Stefan Popa
This patch adds support for the adxl372 FIFO. In order to accomplish this,
triggered buffers were used.

The number of FIFO samples which trigger the watermark interrupt can be
configured by using the buffer watermark. The FIFO format is determined by
configuring the scan elements for each axis. The FIFO data is pushed to the
IIO device's buffer.

Signed-off-by: Stefan Popa 
---
 drivers/iio/accel/adxl372.c | 357 +++-
 1 file changed, 356 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index db9ecd2..1e2519a 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -6,12 +6,19 @@
  */
 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 /* ADXL372 registers definition */
 #define ADXL372_DEVID  0x00
@@ -123,6 +130,9 @@
 #define ADXL372_INT1_MAP_LOW_MSK   BIT(7)
 #define ADXL372_INT1_MAP_LOW_MODE(x)   (((x) & 0x1) << 7)
 
+/* The ADXL372 includes a deep, 512 sample FIFO buffer */
+#define ADXL372_FIFO_SIZE  512
+
 /*
  * At +/- 200g with 12-bit resolution, scale is computed as:
  * (200 + 200) * 9.81 / (2^12 - 1) = 0.958241
@@ -170,6 +180,43 @@ static const unsigned int adxl372_th_reg_high_addr[3] = {
[ADXL372_INACTIVITY] = ADXL372_X_THRESH_INACT_H,
 };
 
+enum adxl372_fifo_format {
+   ADXL372_XYZ_FIFO,
+   ADXL372_X_FIFO,
+   ADXL372_Y_FIFO,
+   ADXL372_XY_FIFO,
+   ADXL372_Z_FIFO,
+   ADXL372_XZ_FIFO,
+   ADXL372_YZ_FIFO,
+   ADXL372_XYZ_PEAK_FIFO,
+};
+
+enum adxl372_fifo_mode {
+   ADXL372_FIFO_BYPASSED,
+   ADXL372_FIFO_STREAMED,
+   ADXL372_FIFO_TRIGGERED,
+   ADXL372_FIFO_OLD_SAVED
+};
+
+static const int adxl372_samp_freq_tbl[5] = {
+   400, 800, 1600, 3200, 6400,
+};
+
+struct adxl372_axis_lookup {
+   unsigned int bits;
+   enum adxl372_fifo_format fifo_format;
+};
+
+static const struct adxl372_axis_lookup adxl372_axis_lookup_table[] = {
+   { BIT(0), ADXL372_X_FIFO },
+   { BIT(1), ADXL372_Y_FIFO },
+   { BIT(2), ADXL372_Z_FIFO },
+   { BIT(0) | BIT(1), ADXL372_XY_FIFO },
+   { BIT(0) | BIT(2), ADXL372_XZ_FIFO },
+   { BIT(1) | BIT(2), ADXL372_YZ_FIFO },
+   { BIT(0) | BIT(1) | BIT(2), ADXL372_XYZ_FIFO },
+};
+
 #define ADXL372_ACCEL_CHANNEL(index, reg, axis) {  \
.type = IIO_ACCEL,  \
.address = reg, \
@@ -177,6 +224,13 @@ static const unsigned int adxl372_th_reg_high_addr[3] = {
.channel2 = IIO_MOD_##axis, \
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),   \
+   .scan_index = index,\
+   .scan_type = {  \
+   .sign = 's',\
+   .realbits = 12, \
+   .storagebits = 16,  \
+   .shift = 4, \
+   },  \
 }
 
 static const struct iio_chan_spec adxl372_channels[] = {
@@ -188,12 +242,29 @@ static const struct iio_chan_spec adxl372_channels[] = {
 struct adxl372_state {
struct spi_device   *spi;
struct regmap   *regmap;
+   struct iio_trigger  *dready_trig;
+   enum adxl372_fifo_mode  fifo_mode;
+   enum adxl372_fifo_formatfifo_format;
enum adxl372_op_modeop_mode;
enum adxl372_act_proc_mode  act_proc_mode;
enum adxl372_odrodr;
enum adxl372_bandwidth  bw;
u32 act_time_ms;
u32 inact_time_ms;
+   u8  fifo_set_size;
+   u8  int1_bitmask;
+   u8  int2_bitmask;
+   u16 watermark;
+   __be16  fifo_buf[ADXL372_FIFO_SIZE];
+};
+
+static const unsigned long adxl372_channel_masks[] = {
+   BIT(0), BIT(1), BIT(2),
+   BIT(0) | BIT(1),
+   BIT(0) | BIT(2),
+   BIT(1) | BIT(2),
+   BIT(0) | BIT(1) | BIT(2),
+   0
 };
 
 static int adxl372_read_axis(struct adxl372_state *st, u8 addr)
@@ -359,6 +430,112 @@ static int adxl372_set_inactivity_time_ms(struct 
adxl372_state *st,
return ret;
 }
 
+static int adxl372_set_interrupts(struct adxl372_state *st,
+ unsigned char int1_bitmask,
+