Prepare the interrupt handler. Add register entries to evaluate the
incoming interrupt. Add functions to clear status registers and reset the
FIFO.
Add FIFO watermark configuration and evaluation. Let a watermark to be
configured. Evaluate the interrupt accordingly. Read out the FIFO content
and push the values to the IIO channel.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
drivers/iio/accel/adxl313.h | 10 ++
drivers/iio/accel/adxl313_core.c | 190 +++++++++++++++++++++++++++++++
2 files changed, 200 insertions(+)
diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h
index ab6b9e11fde4..4f4a9fd39f6d 100644
--- a/drivers/iio/accel/adxl313.h
+++ b/drivers/iio/accel/adxl313.h
@@ -53,11 +53,19 @@
#define ADXL313_INT_ACTIVITY BIT(4)
#define ADXL313_INT_DREADY BIT(7)
+/* FIFO entries: how many values are stored in the FIFO */
+#define ADXL313_REG_FIFO_STATUS_ENTRIES_MSK GENMASK(5, 0)
+/* FIFO samples: number of samples needed for watermark (FIFO mode) */
+#define ADXL313_REG_FIFO_CTL_SAMPLES_MSK GENMASK(4, 0)
#define ADXL313_REG_FIFO_CTL_MODE_MSK GENMASK(7, 6)
#define ADXL313_FIFO_BYPASS 0
#define ADXL313_FIFO_STREAM 2
+#define ADXL313_FIFO_SIZE 32
+
+#define ADXL313_NUM_AXIS 3
+
extern const struct regmap_access_table adxl312_readable_regs_table;
extern const struct regmap_access_table adxl313_readable_regs_table;
extern const struct regmap_access_table adxl314_readable_regs_table;
@@ -78,7 +86,9 @@ struct adxl313_data {
struct regmap *regmap;
const struct adxl313_chip_info *chip_info;
struct mutex lock; /* lock to protect transf_buf */
+ u8 watermark;
__le16 transf_buf __aligned(IIO_DMA_MINALIGN);
+ __le16 fifo_buf[ADXL313_NUM_AXIS * ADXL313_FIFO_SIZE + 1];
};
struct adxl313_chip_info {
diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
index 31ce1b218488..8a0b5542fb40 100644
--- a/drivers/iio/accel/adxl313_core.c
+++ b/drivers/iio/accel/adxl313_core.c
@@ -10,15 +10,21 @@
#include <linux/bitfield.h>
#include <linux/interrupt.h>
#include <linux/module.h>
+#include <linux/overflow.h>
#include <linux/property.h>
#include <linux/regmap.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/kfifo_buf.h>
+
#include "adxl313.h"
#define ADXL313_INT_NONE U8_MAX
#define ADXL313_INT1 1
#define ADXL313_INT2 2
+#define ADXL313_REG_XYZ_BASE ADXL313_REG_DATA_AXIS(0)
+
static const struct regmap_range adxl312_readable_reg_range[] = {
regmap_reg_range(ADXL313_REG_DEVID0, ADXL313_REG_DEVID0),
regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)),
@@ -62,6 +68,7 @@ bool adxl313_is_volatile_reg(struct device *dev, unsigned int reg)
case ADXL313_REG_DATA_AXIS(4):
case ADXL313_REG_DATA_AXIS(5):
case ADXL313_REG_FIFO_STATUS:
+ case ADXL313_REG_INT_SOURCE:
return true;
default:
return false;
@@ -363,6 +370,176 @@ static int adxl313_write_raw(struct iio_dev *indio_dev,
}
}
+static int adxl313_set_watermark(struct iio_dev *indio_dev, unsigned int value)
+{
+ struct adxl313_data *data = iio_priv(indio_dev);
+ const unsigned int fifo_mask = 0x1f, interrupt_mask = 0x02;
+ int ret;
+
+ value = min(value, ADXL313_FIFO_SIZE - 1);
+
+ ret = regmap_update_bits(data->regmap, ADXL313_REG_FIFO_CTL,
+ fifo_mask, value);
+ if (ret)
+ return ret;
+
+ data->watermark = value;
+
+ return regmap_update_bits(data->regmap, ADXL313_REG_INT_ENABLE,
+ interrupt_mask, ADXL313_INT_WATERMARK);
+}
+
+static int adxl313_get_samples(struct adxl313_data *data)
+{
+ unsigned int regval = 0;
+ int ret;
+
+ ret = regmap_read(data->regmap, ADXL313_REG_FIFO_STATUS, ®val);
+ if (ret)
+ return ret;
+
+ return FIELD_GET(ADXL313_REG_FIFO_STATUS_ENTRIES_MSK, regval);
+}
+
+static int adxl313_fifo_transfer(struct adxl313_data *data, int samples)
+{
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < samples; i++) {
+ ret = regmap_bulk_read(data->regmap, ADXL313_REG_XYZ_BASE,
+ data->fifo_buf + (i * ADXL313_NUM_AXIS),
+ 2 * ADXL313_NUM_AXIS);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+/**
+ * adxl313_fifo_reset() - Reset the FIFO and interrupt status registers.
+ * @data: The device data.
+ *
+ * Reset the FIFO status registers. Reading out status registers clears the
+ * FIFO and interrupt configuration. Thus do not evaluate regmap return values.
+ * Ignore particular read register content. Register content is not processed
+ * any further. Therefore the function returns void.
+ */
+static void adxl313_fifo_reset(struct adxl313_data *data)
+{
+ unsigned int regval;
+ int samples;
+
+ adxl313_set_measure_en(data, false);
+
+ samples = adxl313_get_samples(data);
+ if (samples > 0)
+ adxl313_fifo_transfer(data, samples);
+
+ regmap_read(data->regmap, ADXL313_REG_INT_SOURCE, ®val);
+
+ adxl313_set_measure_en(data, true);
+}
+
+static int adxl313_buffer_postenable(struct iio_dev *indio_dev)
+{
+ struct adxl313_data *data = iio_priv(indio_dev);
+ int ret;
+
+ ret = adxl313_set_measure_en(data, false);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(data->regmap, ADXL313_REG_FIFO_CTL,
+ FIELD_PREP(ADXL313_REG_FIFO_CTL_SAMPLES_MSK, data->watermark) |
+ FIELD_PREP(ADXL313_REG_FIFO_CTL_MODE_MSK, ADXL313_FIFO_STREAM));
+ if (ret)
+ return ret;
+
+ return adxl313_set_measure_en(data, true);
+}
+
+static int adxl313_buffer_predisable(struct iio_dev *indio_dev)
+{
+ struct adxl313_data *data = iio_priv(indio_dev);
+ int ret;
+
+ ret = adxl313_set_measure_en(data, false);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(data->regmap, ADXL313_REG_FIFO_CTL,
+ FIELD_PREP(ADXL313_REG_FIFO_CTL_MODE_MSK, ADXL313_FIFO_BYPASS));
+
+ ret = regmap_write(data->regmap, ADXL313_REG_INT_ENABLE, 0);
+ if (ret)
+ return ret;
+
+ return adxl313_set_measure_en(data, true);
+}
+
+static const struct iio_buffer_setup_ops adxl313_buffer_ops = {
+ .postenable = adxl313_buffer_postenable,
+ .predisable = adxl313_buffer_predisable,
+};
+
+static int adxl313_fifo_push(struct iio_dev *indio_dev, int samples)
+{
+ struct adxl313_data *data = iio_priv(indio_dev);
+ unsigned int i;
+ int ret;
+
+ ret = adxl313_fifo_transfer(data, samples);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < ADXL313_NUM_AXIS * samples; i += ADXL313_NUM_AXIS)
+ iio_push_to_buffers(indio_dev, &data->fifo_buf[i]);
+
+ return 0;
+}
+
+static int adxl313_push_event(struct iio_dev *indio_dev, int int_stat)
+{
+ struct adxl313_data *data = iio_priv(indio_dev);
+ int samples;
+
+ if (FIELD_GET(ADXL313_INT_WATERMARK, int_stat)) {
+ samples = adxl313_get_samples(data);
+ if (samples < 0)
+ return samples;
+
+ return adxl313_fifo_push(indio_dev, samples);
+ }
+
+ /* Return error if no event data was pushed to the IIO channel. */
+ return -ENOENT;
+}
+
+static irqreturn_t adxl313_irq_handler(int irq, void *p)
+{
+ struct iio_dev *indio_dev = p;
+ struct adxl313_data *data = iio_priv(indio_dev);
+ int int_stat;
+
+ if (regmap_read(data->regmap, ADXL313_REG_INT_SOURCE, &int_stat))
+ return IRQ_NONE;
+
+ if (adxl313_push_event(indio_dev, int_stat))
+ goto err;
+
+ if (FIELD_GET(ADXL313_INT_OVERRUN, int_stat))
+ goto err;
+
+ return IRQ_HANDLED;
+
+err:
+ adxl313_fifo_reset(data);
+
+ return IRQ_HANDLED;
+}
+
static int adxl313_reg_access(struct iio_dev *indio_dev, unsigned int reg,
unsigned int writeval, unsigned int *readval)
{
@@ -377,6 +554,7 @@ static const struct iio_info adxl313_info = {
.read_raw = adxl313_read_raw,
.write_raw = adxl313_write_raw,
.read_avail = adxl313_read_freq_avail,
+ .hwfifo_set_watermark = adxl313_set_watermark,
.debugfs_reg_access = &adxl313_reg_access,
};
@@ -487,6 +665,18 @@ int adxl313_core_probe(struct device *dev,
int_map_msk, int_line == ADXL313_INT2);
if (ret)
return ret;
+
+ ret = devm_iio_kfifo_buffer_setup(dev, indio_dev,
+ &adxl313_buffer_ops);
+ if (ret)
+ return ret;
+
+ ret = devm_request_threaded_irq(dev, irq, NULL,
+ &adxl313_irq_handler,
+ IRQF_SHARED | IRQF_ONESHOT,
+ indio_dev->name, indio_dev);
+ if (ret)
+ return ret;
} else {
/*
* FIFO_BYPASSED mode
--
2.39.5On Sun, 1 Jun 2025 17:21:34 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:
> Prepare the interrupt handler. Add register entries to evaluate the
> incoming interrupt. Add functions to clear status registers and reset the
> FIFO.
>
> Add FIFO watermark configuration and evaluation. Let a watermark to be
> configured. Evaluate the interrupt accordingly. Read out the FIFO content
> and push the values to the IIO channel.
>
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
> drivers/iio/accel/adxl313.h | 10 ++
> drivers/iio/accel/adxl313_core.c | 190 +++++++++++++++++++++++++++++++
> 2 files changed, 200 insertions(+)
>
> diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h
> index ab6b9e11fde4..4f4a9fd39f6d 100644
> --- a/drivers/iio/accel/adxl313.h
> +++ b/drivers/iio/accel/adxl313.h
> @@ -53,11 +53,19 @@
> #define ADXL313_INT_ACTIVITY BIT(4)
> #define ADXL313_INT_DREADY BIT(7)
>
> +/* FIFO entries: how many values are stored in the FIFO */
> +#define ADXL313_REG_FIFO_STATUS_ENTRIES_MSK GENMASK(5, 0)
> +/* FIFO samples: number of samples needed for watermark (FIFO mode) */
> +#define ADXL313_REG_FIFO_CTL_SAMPLES_MSK GENMASK(4, 0)
> #define ADXL313_REG_FIFO_CTL_MODE_MSK GENMASK(7, 6)
>
> #define ADXL313_FIFO_BYPASS 0
> #define ADXL313_FIFO_STREAM 2
>
> +#define ADXL313_FIFO_SIZE 32
> +
> +#define ADXL313_NUM_AXIS 3
> +
> extern const struct regmap_access_table adxl312_readable_regs_table;
> extern const struct regmap_access_table adxl313_readable_regs_table;
> extern const struct regmap_access_table adxl314_readable_regs_table;
> @@ -78,7 +86,9 @@ struct adxl313_data {
> struct regmap *regmap;
> const struct adxl313_chip_info *chip_info;
> struct mutex lock; /* lock to protect transf_buf */
> + u8 watermark;
> __le16 transf_buf __aligned(IIO_DMA_MINALIGN);
> + __le16 fifo_buf[ADXL313_NUM_AXIS * ADXL313_FIFO_SIZE + 1];
> };
>
> struct adxl313_chip_info {
> diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
> index 31ce1b218488..8a0b5542fb40 100644
> --- a/drivers/iio/accel/adxl313_core.c
> +++ b/drivers/iio/accel/adxl313_core.c
> @@ -10,15 +10,21 @@
> #include <linux/bitfield.h>
> #include <linux/interrupt.h>
> #include <linux/module.h>
> +#include <linux/overflow.h>
> #include <linux/property.h>
> #include <linux/regmap.h>
>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/kfifo_buf.h>
> +
> #include "adxl313.h"
>
> #define ADXL313_INT_NONE U8_MAX
> #define ADXL313_INT1 1
> #define ADXL313_INT2 2
>
> +#define ADXL313_REG_XYZ_BASE ADXL313_REG_DATA_AXIS(0)
> +
> static const struct regmap_range adxl312_readable_reg_range[] = {
> regmap_reg_range(ADXL313_REG_DEVID0, ADXL313_REG_DEVID0),
> regmap_reg_range(ADXL313_REG_OFS_AXIS(0), ADXL313_REG_OFS_AXIS(2)),
> @@ -62,6 +68,7 @@ bool adxl313_is_volatile_reg(struct device *dev, unsigned int reg)
> case ADXL313_REG_DATA_AXIS(4):
> case ADXL313_REG_DATA_AXIS(5):
> case ADXL313_REG_FIFO_STATUS:
> + case ADXL313_REG_INT_SOURCE:
It's always been volatile, whether or not we were writing it.
Hmm. Given this I'm dropping the regcache patch as I'd missed that was a partial
list when reviewing that one.
> return true;
> default:
> return false;
> @@ -363,6 +370,176 @@ static int adxl313_write_raw(struct iio_dev *indio_dev,
> }
> }
>
> +static int adxl313_set_watermark(struct iio_dev *indio_dev, unsigned int value)
> +{
> + struct adxl313_data *data = iio_priv(indio_dev);
> + const unsigned int fifo_mask = 0x1f, interrupt_mask = 0x02;
Why not just use defines for the fields? The second one is particularly
confusing as that is just the mask for the watermark interrupt not of
a general 'interrupt' field as the name suggests.
> + int ret;
> +
> + value = min(value, ADXL313_FIFO_SIZE - 1);
> +
> + ret = regmap_update_bits(data->regmap, ADXL313_REG_FIFO_CTL,
> + fifo_mask, value);
> + if (ret)
> + return ret;
> +
> + data->watermark = value;
> +
> + return regmap_update_bits(data->regmap, ADXL313_REG_INT_ENABLE,
> + interrupt_mask, ADXL313_INT_WATERMARK);
return regmap_set_bits(data->regmap, ADXL313_REG_INT_ENABLED,
ADXL313_INT_WATERMARK);
> +}
> +
> +static int adxl313_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + struct adxl313_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + ret = adxl313_set_measure_en(data, false);
I'd like a comment on why we need to disable measurements
here. Some reference to the datasheet probably and that fifo mode
can only be changed with measurements disabled.
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(data->regmap, ADXL313_REG_FIFO_CTL,
> + FIELD_PREP(ADXL313_REG_FIFO_CTL_SAMPLES_MSK, data->watermark) |
> + FIELD_PREP(ADXL313_REG_FIFO_CTL_MODE_MSK, ADXL313_FIFO_STREAM));
> + if (ret)
> + return ret;
> +
> + return adxl313_set_measure_en(data, true);
> +}
> +static int adxl313_push_event(struct iio_dev *indio_dev, int int_stat)
Can we avoid 'event' naming here. Events in IIO terms would not include
watermarks used to drain a fifo.
> +{
> + struct adxl313_data *data = iio_priv(indio_dev);
> + int samples;
> +
> + if (FIELD_GET(ADXL313_INT_WATERMARK, int_stat)) {
> + samples = adxl313_get_samples(data);
> + if (samples < 0)
> + return samples;
> +
> + return adxl313_fifo_push(indio_dev, samples);
> + }
> +
> + /* Return error if no event data was pushed to the IIO channel. */
> + return -ENOENT;
> +}
> +
> +static irqreturn_t adxl313_irq_handler(int irq, void *p)
> +{
> + struct iio_dev *indio_dev = p;
> + struct adxl313_data *data = iio_priv(indio_dev);
> + int int_stat;
> +
> + if (regmap_read(data->regmap, ADXL313_REG_INT_SOURCE, &int_stat))
> + return IRQ_NONE;
> +
> + if (adxl313_push_event(indio_dev, int_stat))
> + goto err;
> +
> + if (FIELD_GET(ADXL313_INT_OVERRUN, int_stat))
I suspect that we can have watermark and overrun set. Whether it is appropriate
to drain the data out and push it to userspace isn't clear to me. Maybe
add a comment on that so we can refer to it when considering the logic.
> + goto err;
> +
> + return IRQ_HANDLED;
> +
> +err:
> + adxl313_fifo_reset(data);
> +
> + return IRQ_HANDLED;
> +}
> +
> static int adxl313_reg_access(struct iio_dev *indio_dev, unsigned int reg,
> unsigned int writeval, unsigned int *readval)
> {
> @@ -377,6 +554,7 @@ static const struct iio_info adxl313_info = {
> .read_raw = adxl313_read_raw,
> .write_raw = adxl313_write_raw,
> .read_avail = adxl313_read_freq_avail,
> + .hwfifo_set_watermark = adxl313_set_watermark,
> .debugfs_reg_access = &adxl313_reg_access,
> };
>
> @@ -487,6 +665,18 @@ int adxl313_core_probe(struct device *dev,
> int_map_msk, int_line == ADXL313_INT2);
> if (ret)
> return ret;
> +
> + ret = devm_iio_kfifo_buffer_setup(dev, indio_dev,
> + &adxl313_buffer_ops);
> + if (ret)
> + return ret;
> +
> + ret = devm_request_threaded_irq(dev, irq, NULL,
> + &adxl313_irq_handler,
> + IRQF_SHARED | IRQF_ONESHOT,
> + indio_dev->name, indio_dev);
> + if (ret)
> + return ret;
> } else {
> /*
> * FIFO_BYPASSED mode
On Sun, Jun 1, 2025 at 8:22 PM Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> Prepare the interrupt handler. Add register entries to evaluate the
> incoming interrupt. Add functions to clear status registers and reset the
> FIFO.
>
> Add FIFO watermark configuration and evaluation. Let a watermark to be
> configured. Evaluate the interrupt accordingly. Read out the FIFO content
> and push the values to the IIO channel.
...
> +static int adxl313_set_watermark(struct iio_dev *indio_dev, unsigned int value)
> +{
> + struct adxl313_data *data = iio_priv(indio_dev);
> + const unsigned int fifo_mask = 0x1f, interrupt_mask = 0x02;
GENMASK()
BIT()
> + int ret;
> +
> + value = min(value, ADXL313_FIFO_SIZE - 1);
> +
> + ret = regmap_update_bits(data->regmap, ADXL313_REG_FIFO_CTL,
> + fifo_mask, value);
> + if (ret)
> + return ret;
> +
> + data->watermark = value;
> +
> + return regmap_update_bits(data->regmap, ADXL313_REG_INT_ENABLE,
> + interrupt_mask, ADXL313_INT_WATERMARK);
> +}
...
> +static int adxl313_get_samples(struct adxl313_data *data)
> +{
> + unsigned int regval = 0;
Useless assignment.
> + int ret;
> +
> + ret = regmap_read(data->regmap, ADXL313_REG_FIFO_STATUS, ®val);
> + if (ret)
> + return ret;
> +
> + return FIELD_GET(ADXL313_REG_FIFO_STATUS_ENTRIES_MSK, regval);
> +}
...
> + ret = devm_request_threaded_irq(dev, irq, NULL,
> + &adxl313_irq_handler,
> + IRQF_SHARED | IRQF_ONESHOT,
> + indio_dev->name, indio_dev);
> + if (ret)
> + return ret;
Now I see the first user of 'irq'. Logically these two patches may not
be split. Or split should be made differently, let's say IRQ type
holding variable + switch case can go in the first preparatory patch
(however it will make a little sense without real users, as it is/will
be a dead code).
--
With Best Regards,
Andy Shevchenko
On Sun, 1 Jun 2025 22:26:42 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Sun, Jun 1, 2025 at 8:22 PM Lothar Rubusch <l.rubusch@gmail.com> wrote:
> >
> > Prepare the interrupt handler. Add register entries to evaluate the
> > incoming interrupt. Add functions to clear status registers and reset the
> > FIFO.
> >
> > Add FIFO watermark configuration and evaluation. Let a watermark to be
> > configured. Evaluate the interrupt accordingly. Read out the FIFO content
> > and push the values to the IIO channel.
>
> ...
>
> > +static int adxl313_set_watermark(struct iio_dev *indio_dev, unsigned int value)
> > +{
> > + struct adxl313_data *data = iio_priv(indio_dev);
> > + const unsigned int fifo_mask = 0x1f, interrupt_mask = 0x02;
>
> GENMASK()
> BIT()
>
> > + int ret;
> > +
> > + value = min(value, ADXL313_FIFO_SIZE - 1);
> > +
> > + ret = regmap_update_bits(data->regmap, ADXL313_REG_FIFO_CTL,
> > + fifo_mask, value);
> > + if (ret)
> > + return ret;
> > +
> > + data->watermark = value;
> > +
> > + return regmap_update_bits(data->regmap, ADXL313_REG_INT_ENABLE,
> > + interrupt_mask, ADXL313_INT_WATERMARK);
> > +}
>
> ...
>
> > +static int adxl313_get_samples(struct adxl313_data *data)
> > +{
> > + unsigned int regval = 0;
>
> Useless assignment.
>
> > + int ret;
> > +
> > + ret = regmap_read(data->regmap, ADXL313_REG_FIFO_STATUS, ®val);
> > + if (ret)
> > + return ret;
> > +
> > + return FIELD_GET(ADXL313_REG_FIFO_STATUS_ENTRIES_MSK, regval);
> > +}
>
> ...
>
> > + ret = devm_request_threaded_irq(dev, irq, NULL,
> > + &adxl313_irq_handler,
> > + IRQF_SHARED | IRQF_ONESHOT,
> > + indio_dev->name, indio_dev);
> > + if (ret)
> > + return ret;
>
> Now I see the first user of 'irq'. Logically these two patches may not
> be split. Or split should be made differently, let's say IRQ type
> holding variable + switch case can go in the first preparatory patch
> (however it will make a little sense without real users, as it is/will
> be a dead code).
>
I'd just combine these two patches and patch 2 (which is also dead code
until this one is in place).
Jonathan
© 2016 - 2026 Red Hat, Inc.