[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20241124190057.73dd7cd5@jic23-huawei>
Date: Sun, 24 Nov 2024 19:00:57 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Lothar Rubusch <l.rubusch@...il.com>
Cc: lars@...afoo.de, Michael.Hennerich@...log.com,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
eraretuya@...il.com
Subject: Re: [PATCH v2 18/22] iio: accel: adxl345: start measure at buffer
en/disable
On Sun, 17 Nov 2024 18:26:47 +0000
Lothar Rubusch <l.rubusch@...il.com> wrote:
> Add and initialize the buffer options to use the FIFO and watermark
> feature of the adxl345 sensor. In this way measure enable will happen
> in at enabling the buffer.
>
> Signed-off-by: Lothar Rubusch <l.rubusch@...il.com>
Various minor comments. In general I haven't commented on every
instance of formatting problems etc. Please generalize though all the
patches based on comments that are made.
Thanks,
Jonathan
> ---
> drivers/iio/accel/adxl345_core.c | 105 +++++++++++++++++++++++++++++--
> 1 file changed, 101 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index a653774db8..b57a123ac9 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -181,6 +181,28 @@ static const struct iio_chan_spec adxl34x_channels[] = {
> ADXL34x_CHANNEL(2, chan_z, Z),
> };
>
> +static int adxl345_set_interrupts(struct adxl34x_state *st)
> +{
> + int ret;
> + unsigned int int_enable = st->int_map;
> + unsigned int int_map;
> +
> + /* Any bits set to 0 in the INT map register send their respective
> + * interrupts to the INT1 pin, whereas bits set to 1 send their respective
> + * interrupts to the INT2 pin. The intio shall convert this accordingly.
> + */
> + int_map = 0xFF & (st->intio ? st->int_map : ~st->int_map);
> + pr_debug("%s(): Setting INT_MAP 0x%02X\n", __func__, int_map);
> +
> + ret = regmap_write(st->regmap, ADXL345_REG_INT_MAP, int_map);
> + if (ret)
> + return ret;
> +
> + pr_debug("%s(): Setting INT_ENABLE 0x%02X\n", __func__, int_enable);
> + ret = regmap_write(st->regmap, ADXL345_REG_INT_ENABLE, int_enable);
> + return ret;
> +}
> +
> static int adxl345_read_raw(struct iio_dev *indio_dev,
> struct iio_chan_spec const *chan,
> int *val, int *val2, long mask)
> @@ -330,6 +352,41 @@ static const struct attribute_group adxl345_attrs_group = {
> .attrs = adxl345_attrs,
> };
>
> +static int adxl345_set_fifo(struct adxl34x_state *st)
> +{
> + struct adxl34x_platform_data *data = &st->data;
> + u8 fifo_ctl;
> + int ret;
> +
> + /* FIFO should be configured while in standby mode */
> + adxl345_set_measure_en(st, false);
Check for errors.
> +
> + /* The watermark bit is set when the number of samples in FIFO
> + * equals the value stored in the samples bits (register
> + * FIFO_CTL). The watermark bit is cleared automatically when
> + * FIFO is read, and the content returns to a value below the
> + * value stored in the samples bits.
> + */
> + fifo_ctl = 0x00 |
What does the 0x00 bring? Get rid of that.
> + ADXL345_FIFO_CTL_SAMLPES(data->watermark) |
> + ADXL345_FIFO_CTL_TRIGGER(st->intio) |
> + ADXL345_FIFO_CTL_MODE(data->fifo_mode);
> +
> + pr_debug("%s(): fifo_ctl 0x%02X\n", __func__, fifo_ctl);
> +
> + /* The watermark bit is set when the number of samples in FIFO
> + * equals the value stored in the samples bits (register
> + * FIFO_CTL). The watermark bit is cleared automatically when
> + * FIFO is read, and the content returns to a value below the
> + * value stored in the samples bits.
> + */
> + ret = regmap_write(st->regmap, ADXL345_REG_FIFO_CTL, fifo_ctl);
> + if (ret < 0)
> + return ret;
> +
> + return adxl345_set_measure_en(st, true);
> +}
> +
> /**
> * adxl345_get_fifo_entries() - Read number of FIFO entries into *fifo_entries.
> * @st: The initialized state instance of this driver.
> @@ -416,7 +473,50 @@ static void adxl345_empty_fifo(struct adxl34x_state *st)
> regmap_read(st->regmap, ADXL345_REG_INT_SOURCE, ®val);
> }
>
> +static int adxl345_buffer_postenable(struct iio_dev *indio_dev)
> +{
> + struct adxl34x_state *st = iio_priv(indio_dev);
> + struct adxl34x_platform_data *data = &st->data;
> + int ret;
> +
> + ret = adxl345_set_interrupts(st);
> + if (ret)
> + return -EINVAL;
> +
> + /* Default to FIFO mode: STREAM, since it covers the general usage
> + * and does not bypass the FIFO
> + */
> + data->fifo_mode = ADXL_FIFO_STREAM;
> + adxl345_set_fifo(st);
check for errors and probably try and unwind what was done in here if you
get one.
> +
> + return 0;
> +}
> +
> +static int adxl345_buffer_predisable(struct iio_dev *indio_dev)
> +{
> + struct adxl34x_state *st = iio_priv(indio_dev);
> + struct adxl34x_platform_data *data = &st->data;
> + int ret;
> +
> + /* Turn off interrupts */
> + st->int_map = 0x00;
> +
> + ret = adxl345_set_interrupts(st);
> + if (ret) {
> + pr_warn("%s(): Failed to disable INTs\n", __func__);
> + return -EINVAL;
> + }
> +
> + /* Set FIFO mode: BYPASS, according to the situation */
> + data->fifo_mode = ADXL_FIFO_BYPASS;
> + adxl345_set_fifo(st);
always check for errors.
> +
> + return 0;
> +}
>
> static int adxl345_get_status(struct adxl34x_state *st, u8 *int_stat)
> @@ -625,7 +725,7 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap,
>
> indio_dev->name = st->info->name;
> indio_dev->info = &adxl345_info;
> - indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
That is set when you register a triggered buffer.
> indio_dev->channels = adxl34x_channels;
> indio_dev->num_channels = ARRAY_SIZE(adxl34x_channels);
Powered by blists - more mailing lists