[PATCH v3 08/12] iio: accel: adxl313: add FIFO watermark

Lothar Rubusch posted 12 patches 6 months, 4 weeks ago
Only 11 patches received!
There is a newer version of this series
[PATCH v3 08/12] iio: accel: adxl313: add FIFO watermark
Posted by Lothar Rubusch 6 months, 4 weeks ago
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      |  1 +
 drivers/iio/accel/adxl313_core.c | 63 ++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/drivers/iio/accel/adxl313.h b/drivers/iio/accel/adxl313.h
index 9cdad55dd350..8c68cff7569c 100644
--- a/drivers/iio/accel/adxl313.h
+++ b/drivers/iio/accel/adxl313.h
@@ -87,6 +87,7 @@ struct adxl313_data {
 	const struct adxl313_chip_info *chip_info;
 	struct mutex	lock; /* lock to protect transf_buf */
 	int irq;
+	u8 watermark;
 	u8 fifo_mode;
 	__le16		transf_buf __aligned(IIO_DMA_MINALIGN);
 	__le16		fifo_buf[ADXL313_NUM_AXIS * ADXL313_FIFO_SIZE + 1];
diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
index 1e085f0c61a0..80991cd9bd79 100644
--- a/drivers/iio/accel/adxl313_core.c
+++ b/drivers/iio/accel/adxl313_core.c
@@ -373,6 +373,25 @@ 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;
@@ -399,6 +418,7 @@ static int adxl313_set_fifo(struct adxl313_data *data)
 		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, data->fifo_mode));
 
 	return adxl313_set_measure_en(data, true);
@@ -471,6 +491,39 @@ static const struct iio_buffer_setup_ops adxl313_buffer_ops = {
 	.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;
@@ -480,6 +533,15 @@ static irqreturn_t adxl313_irq_handler(int irq, void *p)
 	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;
@@ -499,6 +561,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,
 };
 
-- 
2.39.5
Re: [PATCH v3 08/12] iio: accel: adxl313: add FIFO watermark
Posted by Jonathan Cameron 6 months, 4 weeks ago
On Fri, 23 May 2025 22:35:19 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> 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>
Hi Lothar,

I'd rethink the patch split a little and combine this with the initial interrupt
support.  That will avoid confusion over when the reset fires.

> diff --git a/drivers/iio/accel/adxl313_core.c b/drivers/iio/accel/adxl313_core.c
> index 1e085f0c61a0..80991cd9bd79 100644
> --- a/drivers/iio/accel/adxl313_core.c
> +++ b/drivers/iio/accel/adxl313_core.c
> @@ -373,6 +373,25 @@ static int adxl313_write_raw(struct iio_dev *indio_dev,

> +
> +static int adxl313_push_event(struct iio_dev *indio_dev, int int_stat)
I'd avoid the term 'event' for this as your only use so far is something
we'd not consider an 'event' in IIO terms.

I'd be tempted to just keep this as code directly in the _irq_handler()
but maybe it will become less manageable in later patches.

> +{
> +	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;
> @@ -480,6 +533,15 @@ static irqreturn_t adxl313_irq_handler(int irq, void *p)
>  	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);

Ah. This is all making a little more sense.  I think partly the issue
here is this got split up into a few too many patches.  FIFO plus
initial interrupt support in one would have been better perhaps.

Jonathan