Re: [PATCH 3/5] iio:adxl372: Add FIFO and interrupts support
On Thu, Jul 12, 2018 at 06:36:00PM +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, while the format depends on the > selected channels.The FIFO data along with the timestamp is pushed to the > IIO device's buffer. > > Signed-off-by: Stefan Popa > --- > .../devicetree/bindings/iio/accel/adxl372.txt | 7 + This belongs in patch 2. Bindings should reflect the h/w, not driver features. > drivers/iio/accel/adxl372.c| 346 > - > 2 files changed, 352 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/iio/accel/adxl372.txt > b/Documentation/devicetree/bindings/iio/accel/adxl372.txt > index fea4baf..73d7e03 100644 > --- a/Documentation/devicetree/bindings/iio/accel/adxl372.txt > +++ b/Documentation/devicetree/bindings/iio/accel/adxl372.txt > @@ -7,10 +7,17 @@ Required properties: > - reg: SPI chip select number for the device > - spi-max-frequency: Max SPI frequency to use > > +Optional properties: > + - interrupt-parent: phandle to the parent interrupt controller Don't need to put this in the binding. > + - interrupts: interrupt mapping for GPIO IRQ, it should by configured with > + flag IRQ_TYPE_EDGE_FALLING > + > Example: > > accelerometer@0 { > compatible = "adi,adxl372"; > reg = <0>; > spi-max-frequency = <100>; > + interrupt-parent = <>; > + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; > };
Re: [PATCH 3/5] iio:adxl372: Add FIFO and interrupts support
On Thu, Jul 12, 2018 at 06:36:00PM +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, while the format depends on the > selected channels.The FIFO data along with the timestamp is pushed to the > IIO device's buffer. > > Signed-off-by: Stefan Popa > --- > .../devicetree/bindings/iio/accel/adxl372.txt | 7 + This belongs in patch 2. Bindings should reflect the h/w, not driver features. > drivers/iio/accel/adxl372.c| 346 > - > 2 files changed, 352 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/iio/accel/adxl372.txt > b/Documentation/devicetree/bindings/iio/accel/adxl372.txt > index fea4baf..73d7e03 100644 > --- a/Documentation/devicetree/bindings/iio/accel/adxl372.txt > +++ b/Documentation/devicetree/bindings/iio/accel/adxl372.txt > @@ -7,10 +7,17 @@ Required properties: > - reg: SPI chip select number for the device > - spi-max-frequency: Max SPI frequency to use > > +Optional properties: > + - interrupt-parent: phandle to the parent interrupt controller Don't need to put this in the binding. > + - interrupts: interrupt mapping for GPIO IRQ, it should by configured with > + flag IRQ_TYPE_EDGE_FALLING > + > Example: > > accelerometer@0 { > compatible = "adi,adxl372"; > reg = <0>; > spi-max-frequency = <100>; > + interrupt-parent = <>; > + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; > };
Re: [PATCH 3/5] iio:adxl372: Add FIFO and interrupts support
On Thu, 12 Jul 2018 18:36:00 +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, while the format depends on the > selected channels.The FIFO data along with the timestamp is pushed to the > IIO device's buffer. > > Signed-off-by: Stefan Popa > --- > .../devicetree/bindings/iio/accel/adxl372.txt | 7 + > drivers/iio/accel/adxl372.c| 346 > - > 2 files changed, 352 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/iio/accel/adxl372.txt > b/Documentation/devicetree/bindings/iio/accel/adxl372.txt > index fea4baf..73d7e03 100644 > --- a/Documentation/devicetree/bindings/iio/accel/adxl372.txt > +++ b/Documentation/devicetree/bindings/iio/accel/adxl372.txt > @@ -7,10 +7,17 @@ Required properties: > - reg: SPI chip select number for the device > - spi-max-frequency: Max SPI frequency to use > > +Optional properties: > + - interrupt-parent: phandle to the parent interrupt controller > + - interrupts: interrupt mapping for GPIO IRQ, it should by configured with > + flag IRQ_TYPE_EDGE_FALLING Absolutely no reason I can see why this should be a gpio. Plenty of chips out there have specific irq lines that aren't completely useless as gpios. As a general rule, a DT binding is not meant to represent what Linux can currently do with a device, but rather everything about how it might be connected up. So it would have been better to have a single patch introducing all the DT bindings rather than having it in scattered like this. Note this is unlike other forms of documentation where doing it like this would have been entirely correct! > + > Example: > > accelerometer@0 { > compatible = "adi,adxl372"; > reg = <0>; > spi-max-frequency = <100>; > + interrupt-parent = <>; > + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; > }; > diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c > index 62ce238..645902d 100644 > --- a/drivers/iio/accel/adxl372.c > +++ b/drivers/iio/accel/adxl372.c > @@ -6,12 +6,20 @@ > */ > > #include > +#include > +#include > #include > #include > #include > +#include That worries me. No way a device should know about GPIOs to add interrupt support. Thankfully seems to just be the header. Drop it - you aren't a gpio consumer, you are an irq consumer which might be a gpio. > > #include > #include > +#include > +#include > +#include > +#include > +#include > > /* ADXL372 registers definition */ > #define ADXL372_DEVID0x00 > @@ -126,6 +134,8 @@ > #define ADXL372_INT1_MAP_LOW_MSK BIT(7) > #define ADXL372_INT1_MAP_LOW_MODE(x) (((x) & 0x1) << 7) > > +#define ADXL372_FIFO_SIZE512 > + > /* > * At +/- 200g with 12-bit resolution, scale is computed as: > * (200 + 200) * 9.81 / (2^12 - 1) = 0.958241 > @@ -167,6 +177,28 @@ enum adxl372_bandwidth { > ADXL372_BW_3200HZ, > }; > > +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, Hmm. This one is interesting ;) > +}; > + > +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, > +}; > + > #define ADXL372_ACCEL_CHANNEL(index, reg, axis) {\ > .type = IIO_ACCEL, \ > .address = reg, \ > @@ -193,12 +225,27 @@ 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; > + u8 fifo_set_size; > + u8 int1_bitmask; > + u8 int2_bitmask; > + u16 watermark; > + __be16 fifo_buf[512]; > }; > > +static int adxl372_read_fifo(struct adxl372_state *st, u16 fifo_entries) > +{ > + return regmap_bulk_read(st->regmap, > +
Re: [PATCH 3/5] iio:adxl372: Add FIFO and interrupts support
On Thu, 12 Jul 2018 18:36:00 +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, while the format depends on the > selected channels.The FIFO data along with the timestamp is pushed to the > IIO device's buffer. > > Signed-off-by: Stefan Popa > --- > .../devicetree/bindings/iio/accel/adxl372.txt | 7 + > drivers/iio/accel/adxl372.c| 346 > - > 2 files changed, 352 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/iio/accel/adxl372.txt > b/Documentation/devicetree/bindings/iio/accel/adxl372.txt > index fea4baf..73d7e03 100644 > --- a/Documentation/devicetree/bindings/iio/accel/adxl372.txt > +++ b/Documentation/devicetree/bindings/iio/accel/adxl372.txt > @@ -7,10 +7,17 @@ Required properties: > - reg: SPI chip select number for the device > - spi-max-frequency: Max SPI frequency to use > > +Optional properties: > + - interrupt-parent: phandle to the parent interrupt controller > + - interrupts: interrupt mapping for GPIO IRQ, it should by configured with > + flag IRQ_TYPE_EDGE_FALLING Absolutely no reason I can see why this should be a gpio. Plenty of chips out there have specific irq lines that aren't completely useless as gpios. As a general rule, a DT binding is not meant to represent what Linux can currently do with a device, but rather everything about how it might be connected up. So it would have been better to have a single patch introducing all the DT bindings rather than having it in scattered like this. Note this is unlike other forms of documentation where doing it like this would have been entirely correct! > + > Example: > > accelerometer@0 { > compatible = "adi,adxl372"; > reg = <0>; > spi-max-frequency = <100>; > + interrupt-parent = <>; > + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; > }; > diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c > index 62ce238..645902d 100644 > --- a/drivers/iio/accel/adxl372.c > +++ b/drivers/iio/accel/adxl372.c > @@ -6,12 +6,20 @@ > */ > > #include > +#include > +#include > #include > #include > #include > +#include That worries me. No way a device should know about GPIOs to add interrupt support. Thankfully seems to just be the header. Drop it - you aren't a gpio consumer, you are an irq consumer which might be a gpio. > > #include > #include > +#include > +#include > +#include > +#include > +#include > > /* ADXL372 registers definition */ > #define ADXL372_DEVID0x00 > @@ -126,6 +134,8 @@ > #define ADXL372_INT1_MAP_LOW_MSK BIT(7) > #define ADXL372_INT1_MAP_LOW_MODE(x) (((x) & 0x1) << 7) > > +#define ADXL372_FIFO_SIZE512 > + > /* > * At +/- 200g with 12-bit resolution, scale is computed as: > * (200 + 200) * 9.81 / (2^12 - 1) = 0.958241 > @@ -167,6 +177,28 @@ enum adxl372_bandwidth { > ADXL372_BW_3200HZ, > }; > > +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, Hmm. This one is interesting ;) > +}; > + > +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, > +}; > + > #define ADXL372_ACCEL_CHANNEL(index, reg, axis) {\ > .type = IIO_ACCEL, \ > .address = reg, \ > @@ -193,12 +225,27 @@ 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; > + u8 fifo_set_size; > + u8 int1_bitmask; > + u8 int2_bitmask; > + u16 watermark; > + __be16 fifo_buf[512]; > }; > > +static int adxl372_read_fifo(struct adxl372_state *st, u16 fifo_entries) > +{ > + return regmap_bulk_read(st->regmap, > +
[PATCH 3/5] iio:adxl372: Add FIFO and interrupts support
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, while the format depends on the selected channels.The FIFO data along with the timestamp is pushed to the IIO device's buffer. Signed-off-by: Stefan Popa --- .../devicetree/bindings/iio/accel/adxl372.txt | 7 + drivers/iio/accel/adxl372.c| 346 - 2 files changed, 352 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/iio/accel/adxl372.txt b/Documentation/devicetree/bindings/iio/accel/adxl372.txt index fea4baf..73d7e03 100644 --- a/Documentation/devicetree/bindings/iio/accel/adxl372.txt +++ b/Documentation/devicetree/bindings/iio/accel/adxl372.txt @@ -7,10 +7,17 @@ Required properties: - reg: SPI chip select number for the device - spi-max-frequency: Max SPI frequency to use +Optional properties: + - interrupt-parent: phandle to the parent interrupt controller + - interrupts: interrupt mapping for GPIO IRQ, it should by configured with + flag IRQ_TYPE_EDGE_FALLING + Example: accelerometer@0 { compatible = "adi,adxl372"; reg = <0>; spi-max-frequency = <100>; + interrupt-parent = <>; + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; }; diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c index 62ce238..645902d 100644 --- a/drivers/iio/accel/adxl372.c +++ b/drivers/iio/accel/adxl372.c @@ -6,12 +6,20 @@ */ #include +#include +#include #include #include #include +#include #include #include +#include +#include +#include +#include +#include /* ADXL372 registers definition */ #define ADXL372_DEVID 0x00 @@ -126,6 +134,8 @@ #define ADXL372_INT1_MAP_LOW_MSK BIT(7) #define ADXL372_INT1_MAP_LOW_MODE(x) (((x) & 0x1) << 7) +#define ADXL372_FIFO_SIZE 512 + /* * At +/- 200g with 12-bit resolution, scale is computed as: * (200 + 200) * 9.81 / (2^12 - 1) = 0.958241 @@ -167,6 +177,28 @@ enum adxl372_bandwidth { ADXL372_BW_3200HZ, }; +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, +}; + #define ADXL372_ACCEL_CHANNEL(index, reg, axis) { \ .type = IIO_ACCEL, \ .address = reg, \ @@ -193,12 +225,27 @@ 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; + u8 fifo_set_size; + u8 int1_bitmask; + u8 int2_bitmask; + u16 watermark; + __be16 fifo_buf[512]; }; +static int adxl372_read_fifo(struct adxl372_state *st, u16 fifo_entries) +{ + return regmap_bulk_read(st->regmap, + ADXL372_RD_FLAG_MSK(ADXL372_FIFO_DATA), + st->fifo_buf, fifo_entries * 2); +} + static int adxl372_read_axis(struct adxl372_state *st, u8 addr) { __be16 regval; @@ -313,6 +360,114 @@ static int adxl372_set_activity_threshold(struct adxl372_state *st, buf, ARRAY_SIZE(buf)); } +static int adxl372_set_interrupts(struct adxl372_state *st, + unsigned char int1_bitmask, + unsigned char int2_bitmask) +{ + unsigned char buf[2]; + int ret; + + buf[0] = int1_bitmask; + buf[1] = int2_bitmask; + + /* INT1_MAP and INT2_MAP are adjacent registers */ + ret = regmap_bulk_write(st->regmap, + ADXL372_WR_FLAG_MSK(ADXL372_INT1_MAP), + buf, ARRAY_SIZE(buf)); + if (ret < 0) + return ret; + + st->int1_bitmask = int1_bitmask; + st->int2_bitmask = int2_bitmask; + +
[PATCH 3/5] iio:adxl372: Add FIFO and interrupts support
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, while the format depends on the selected channels.The FIFO data along with the timestamp is pushed to the IIO device's buffer. Signed-off-by: Stefan Popa --- .../devicetree/bindings/iio/accel/adxl372.txt | 7 + drivers/iio/accel/adxl372.c| 346 - 2 files changed, 352 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/iio/accel/adxl372.txt b/Documentation/devicetree/bindings/iio/accel/adxl372.txt index fea4baf..73d7e03 100644 --- a/Documentation/devicetree/bindings/iio/accel/adxl372.txt +++ b/Documentation/devicetree/bindings/iio/accel/adxl372.txt @@ -7,10 +7,17 @@ Required properties: - reg: SPI chip select number for the device - spi-max-frequency: Max SPI frequency to use +Optional properties: + - interrupt-parent: phandle to the parent interrupt controller + - interrupts: interrupt mapping for GPIO IRQ, it should by configured with + flag IRQ_TYPE_EDGE_FALLING + Example: accelerometer@0 { compatible = "adi,adxl372"; reg = <0>; spi-max-frequency = <100>; + interrupt-parent = <>; + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; }; diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c index 62ce238..645902d 100644 --- a/drivers/iio/accel/adxl372.c +++ b/drivers/iio/accel/adxl372.c @@ -6,12 +6,20 @@ */ #include +#include +#include #include #include #include +#include #include #include +#include +#include +#include +#include +#include /* ADXL372 registers definition */ #define ADXL372_DEVID 0x00 @@ -126,6 +134,8 @@ #define ADXL372_INT1_MAP_LOW_MSK BIT(7) #define ADXL372_INT1_MAP_LOW_MODE(x) (((x) & 0x1) << 7) +#define ADXL372_FIFO_SIZE 512 + /* * At +/- 200g with 12-bit resolution, scale is computed as: * (200 + 200) * 9.81 / (2^12 - 1) = 0.958241 @@ -167,6 +177,28 @@ enum adxl372_bandwidth { ADXL372_BW_3200HZ, }; +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, +}; + #define ADXL372_ACCEL_CHANNEL(index, reg, axis) { \ .type = IIO_ACCEL, \ .address = reg, \ @@ -193,12 +225,27 @@ 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; + u8 fifo_set_size; + u8 int1_bitmask; + u8 int2_bitmask; + u16 watermark; + __be16 fifo_buf[512]; }; +static int adxl372_read_fifo(struct adxl372_state *st, u16 fifo_entries) +{ + return regmap_bulk_read(st->regmap, + ADXL372_RD_FLAG_MSK(ADXL372_FIFO_DATA), + st->fifo_buf, fifo_entries * 2); +} + static int adxl372_read_axis(struct adxl372_state *st, u8 addr) { __be16 regval; @@ -313,6 +360,114 @@ static int adxl372_set_activity_threshold(struct adxl372_state *st, buf, ARRAY_SIZE(buf)); } +static int adxl372_set_interrupts(struct adxl372_state *st, + unsigned char int1_bitmask, + unsigned char int2_bitmask) +{ + unsigned char buf[2]; + int ret; + + buf[0] = int1_bitmask; + buf[1] = int2_bitmask; + + /* INT1_MAP and INT2_MAP are adjacent registers */ + ret = regmap_bulk_write(st->regmap, + ADXL372_WR_FLAG_MSK(ADXL372_INT1_MAP), + buf, ARRAY_SIZE(buf)); + if (ret < 0) + return ret; + + st->int1_bitmask = int1_bitmask; + st->int2_bitmask = int2_bitmask; + +