On Tue, Mar 4, 2025 at 2:23 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > On Thu, 20 Feb 2025 10:42:28 +0000 > Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote: > > > Add the freefall detection of the sensor together with a threshold and > > time parameter. A freefall event is detected if the measuring signal > > falls below the threshold. > > > > Introduce a freefall threshold stored in regmap cache, and a freefall > > time, having the scaled time value stored as a member variable in the > > state instance. > > > > Signed-off-by: Lothar Rubusch <l.rubusch@xxxxxxxxx> > Hi, one thing inline. > > > @@ -855,6 +958,17 @@ static int adxl345_push_event(struct iio_dev *indio_dev, int int_stat, > > return ret; > > } > > > > + if (FIELD_GET(ADXL345_INT_FREE_FALL, int_stat)) { > > + ret = iio_push_event(indio_dev, > > + IIO_MOD_EVENT_CODE(IIO_ACCEL, 0, > > + IIO_MOD_X_OR_Y_OR_Z, > > + IIO_EV_TYPE_MAG, > > + IIO_EV_DIR_FALLING), > > + ts); > > + if (ret) > > + return ret; > > + } > > + > > Seems unlikely to be right. Pushed an event without error yet this function > is returning an error here? > > > return -ENOENT; > > } > > "it worked on my machine" - Of course, you're right. So, I tried to understand why this "worked". In consequence, I think the best solution will be to put also fifo handling based on int_stat into this function, which I currently made a separate patch, you'll see that in v4. > > @@ -954,6 +1068,7 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap, > > ADXL345_DATA_FORMAT_FULL_RES | > > ADXL345_DATA_FORMAT_SELF_TEST); > > unsigned int tap_threshold; > > + unsigned int ff_threshold; > > int ret; > > > > indio_dev = devm_iio_device_alloc(dev, sizeof(*st)); > > @@ -973,6 +1088,9 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap, > > st->tap_window_us = 64; /* 64 [0x40] -> .080 */ > > st->tap_latent_us = 16; /* 16 [0x10] -> .020 */ > > > > + ff_threshold = 8; /* 8 [0x08] */ > > + st->ff_time_ms = 32; /* 32 [0x20] -> 0.16 */ > > + > > indio_dev->name = st->info->name; > > indio_dev->info = &adxl345_info; > > indio_dev->modes = INDIO_DIRECT_MODE; > > @@ -1049,6 +1167,10 @@ int adxl345_core_probe(struct device *dev, struct regmap *regmap, > > if (ret) > > return ret; > > > > + ret = regmap_write(st->regmap, ADXL345_REG_THRESH_FF, ff_threshold); > > + if (ret) > > + return ret; > > + > > /* FIFO_STREAM mode is going to be activated later */ > > ret = devm_iio_kfifo_buffer_setup(dev, indio_dev, &adxl345_buffer_ops); > > if (ret) >