On Sun, 17 Nov 2024 18:26:43 +0000
Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote:
> Add a function to read the elements of the adxl345 FIFO. This will
> flush the FIFO, and brings it into a ready state. The read out
> is used to read the elemnts and to reset the fifo again. The cleanup
> equally needs a read on the INT_SOURCE register.
>
> Signed-off-by: Lothar Rubusch <l.rubusch@xxxxxxxxx>
> ---
> drivers/iio/accel/adxl345_core.c | 40 ++++++++++++++++++++++++++++++--
> 1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index 508de81bb9..40e78dbdb0 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -140,6 +140,8 @@ struct adxl34x_state {
> const struct adxl345_chip_info *info;
> struct regmap *regmap;
> struct adxl34x_platform_data data; /* watermark, fifo_mode, etc */
> +
> + __le16 fifo_buf[3 * ADXL34x_FIFO_SIZE] __aligned(IIO_DMA_MINALIGN);
That doesn't work. Look at why we have IIO_DMA_MINALIGN().
Hint is that the buffer needs to end up in a cacheline with nothing that might
be written by software in the same line. int_map and below will be so this
is not doing it's job.
Also not used in this patch.
> u8 int_map;
> bool fifo_delay; /* delay: delay is needed for SPI */
> u8 intio;
> @@ -323,6 +325,37 @@ static const struct attribute_group adxl345_attrs_group = {
> .attrs = adxl345_attrs,
> };
>
> +/**
> + * adxl345_get_fifo_entries() - Read number of FIFO entries into *fifo_entries.
> + * @st: The initialized state instance of this driver.
> + * @fifo_entries: A field to be initialized by this function with the number of
> + * FIFO entries.
> + *
> + * Since one read on the FIFO is reading all three axis, X, Y and Z in one, the
> + * number of FIFO entries corresponds to the number of triples of those values.
> + * Note, this sensor does not support treating any axis individually, or
> + * exclude them from measuring.
> + *
> + * Return: 0 or error value.
> + */
> +static int adxl345_get_fifo_entries(struct adxl34x_state *st, int *fifo_entries)
> +{
> + unsigned int regval = 0;
> + int ret;
> +
> + ret = regmap_read(st->regmap, ADXL345_REG_FIFO_STATUS, ®val);
> + if (ret) {
> + pr_warn("%s(): Failed to read FIFO_STATUS register\n", __func__);
As before, seems overkill to warn int his path but not really anywhere else.
dev_err() if you do want to.
> + *fifo_entries = 0;
> + return ret;
> + }
> +
> + *fifo_entries = 0x3f & regval;
> + pr_debug("%s(): fifo_entries %d\n", __func__, *fifo_entries);
> +
> + return 0;
return the value (similar comment to previous patch review.)
> +}
> +
> static const struct iio_buffer_setup_ops adxl345_buffer_ops = {
> };
>
> @@ -363,6 +396,7 @@ static irqreturn_t adxl345_trigger_handler(int irq, void *p)
> struct iio_dev *indio_dev = ((struct iio_poll_func *) p)->indio_dev;
> struct adxl34x_state *st = iio_priv(indio_dev);
> u8 int_stat;
> + int fifo_entries;
> int ret;
>
> ret = adxl345_get_status(st, &int_stat);
> @@ -380,9 +414,11 @@ static irqreturn_t adxl345_trigger_handler(int irq, void *p)
> goto done;
> }
>
> - if (int_stat & (ADXL345_INT_DATA_READY | ADXL345_INT_WATERMARK))
> + if (int_stat & (ADXL345_INT_DATA_READY | ADXL345_INT_WATERMARK)) {
> pr_debug("%s(): WATERMARK or DATA_READY event detected\n", __func__);
> -
> + if (adxl345_get_fifo_entries(st, &fifo_entries) < 0)
Do it all in one go. I want to see interrupt through to data hitting the kfifo +
registration of that kfifo all in one patch.
It is not complicated enough to need to jump through this very slow introduction
of code.
Jonathan
> + goto done;
> + }
> goto done;
> done:
>
