On Sun, 17 Nov 2024 18:26:47 +0000 Lothar Rubusch <l.rubusch@xxxxxxxxx> 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@xxxxxxxxx> 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);