diff mbox series

[v2,07/14] iio: accel: adxl345: add double tap suppress bit

Message ID 20250210110119.260858-8-l.rubusch@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series iio: accel: adxl345: add interrupt based sensor events | expand

Commit Message

Lothar Rubusch Feb. 10, 2025, 11:01 a.m. UTC
Add the suppress bit feature to the double tap feature.

Any tap event is defined by a rising signal edge above threshold, i.e.
duration time starts counting; and the falling edge under threshold
within duration time, i.e. then the tap event is issued. This means
duration is used individually for each tap event.

For double tap detection after a single tap, a latency time needs to be
specified. Usually tap events, i.e. spikes above and returning below
threshold will be ignored within latency. After latency, the window
time starts counting for a second tap detection which has to happen
within a duration time.

If the suppress bit is not set, spikes within latency time are ignored.
Setting the suppress bit will invalidate the double tap function. The
sensor will thus be able to save the window time for double tap
detection, and follow a more strict definition of what signal qualifies
for a double tap.

This brings in a new ABI functionality.
---
Q: Perhaps there is already some IIO ABI for it? If not, please let me
know which ABI documentation to extend. There will be a documentation
patch also later in this series.

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 drivers/iio/accel/adxl345_core.c | 82 ++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

Comments

Jonathan Cameron Feb. 16, 2025, 5:28 p.m. UTC | #1
On Mon, 10 Feb 2025 11:01:12 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Add the suppress bit feature to the double tap feature.
> 
> Any tap event is defined by a rising signal edge above threshold, i.e.
> duration time starts counting; and the falling edge under threshold
> within duration time, i.e. then the tap event is issued. This means
> duration is used individually for each tap event.
> 
> For double tap detection after a single tap, a latency time needs to be
> specified. Usually tap events, i.e. spikes above and returning below
> threshold will be ignored within latency. After latency, the window
> time starts counting for a second tap detection which has to happen
> within a duration time.
> 
> If the suppress bit is not set, spikes within latency time are ignored.
> Setting the suppress bit will invalidate the double tap function. The
> sensor will thus be able to save the window time for double tap
> detection, and follow a more strict definition of what signal qualifies
> for a double tap.

Silly question.  Is there a reason this function would ever be
turned off?   Seems like a sensible heuristic that would not stop
genuine double taps being detected.  Maybe we just always leave it on?

Sometimes the best ABI is the one that doesn't exist as userspace
can't use it wrong.

Jonathan


> 
> This brings in a new ABI functionality.
> ---
> Q: Perhaps there is already some IIO ABI for it? If not, please let me
> know which ABI documentation to extend. There will be a documentation
> patch also later in this series.
> 
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
>  drivers/iio/accel/adxl345_core.c | 82 ++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> index cf35a8f9f432..b6966fee3e3d 100644
> --- a/drivers/iio/accel/adxl345_core.c
> +++ b/drivers/iio/accel/adxl345_core.c
> @@ -34,6 +34,7 @@
>  #define ADXL345_INT2			1
>  
>  #define ADXL345_REG_TAP_AXIS_MSK	GENMASK(2, 0)
> +#define ADXL345_REG_TAP_SUPPRESS_MSK	BIT(3)
>  
>  enum adxl345_axis {
>  	ADXL345_Z_EN = BIT(0),
> @@ -81,6 +82,7 @@ struct adxl345_state {
>  	u32 tap_duration_us;
>  	u32 tap_latent_us;
>  	u32 tap_window_us;
> +	bool tap_suppressed;
>  
>  	__le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
>  };
> @@ -243,6 +245,31 @@ static int adxl345_set_doubletap_en(struct adxl345_state *st, bool en)
>  	return _adxl345_set_tap_int(st, ADXL345_DOUBLE_TAP, en);
>  }
>  
> +static int adxl345_is_suppressed_en(struct adxl345_state *st, bool *en)
> +{
> +	*en = st->tap_suppressed;
> +
> +	return 0;
> +}
> +
> +static int adxl345_set_suppressed_en(struct adxl345_state *st, bool en)
> +{
> +	unsigned long regval = 0;
> +	int ret;
> +
> +	en ? __set_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval)
> +		: __clear_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval);
> +
> +	ret = regmap_update_bits(st->regmap, ADXL345_REG_TAP_AXIS,
> +				 ADXL345_REG_TAP_SUPPRESS_MSK, regval);
> +	if (ret)
> +		return ret;
> +
> +	st->tap_suppressed = en;
> +
> +	return 0;
> +}
> +
>  static int adxl345_set_tap_threshold(struct adxl345_state *st, u8 val)
>  {
>  	int ret;
> @@ -616,6 +643,60 @@ static int adxl345_write_raw_get_fmt(struct iio_dev *indio_dev,
>  	}
>  }
>  
> +static ssize_t in_accel_gesture_doubletap_suppressed_en_show(struct device *dev,
> +							     struct device_attribute *attr,
> +							     char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct adxl345_state *st = iio_priv(indio_dev);
> +	bool en;
> +	int val, ret;
> +
> +	ret = adxl345_is_suppressed_en(st, &en);
> +	if (ret)
> +		return ret;
> +	val = en ? 1 : 0;
> +
> +	return iio_format_value(buf, IIO_VAL_INT, 1, &val);
> +}
> +
> +static ssize_t in_accel_gesture_doubletap_suppressed_en_store(struct device *dev,
> +							      struct device_attribute *attr,
> +							      const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct adxl345_state *st = iio_priv(indio_dev);
> +	int val, ret;
> +
> +	ret = kstrtoint(buf, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	ret = adxl345_set_measure_en(st, false);
> +	if (ret)
> +		return ret;
> +
> +	ret = adxl345_set_suppressed_en(st, val > 0);
> +	if (ret)
> +		return ret;
> +
> +	ret =  adxl345_set_measure_en(st, true);
> +	if (ret)
> +		return ret;
> +
> +	return len;
> +}
> +static IIO_DEVICE_ATTR_RW(in_accel_gesture_doubletap_suppressed_en, 0);
> +
> +static struct attribute *adxl345_event_attrs[] = {
> +	&iio_dev_attr_in_accel_gesture_doubletap_suppressed_en.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group adxl345_event_attrs_group = {
> +	.attrs = adxl345_event_attrs,
> +};
> +
>  static void adxl345_powerdown(void *ptr)
>  {
>  	struct adxl345_state *st = ptr;
> @@ -899,6 +980,7 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
>  
>  static const struct iio_info adxl345_info = {
>  	.attrs		= &adxl345_attrs_group,
> +	.event_attrs	= &adxl345_event_attrs_group,
>  	.read_raw	= adxl345_read_raw,
>  	.write_raw	= adxl345_write_raw,
>  	.write_raw_get_fmt	= adxl345_write_raw_get_fmt,
Lothar Rubusch Feb. 18, 2025, 10:29 p.m. UTC | #2
Dear Jonathan, find my answer down below.

On Sun, Feb 16, 2025 at 6:28 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 10 Feb 2025 11:01:12 +0000
> Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> > Add the suppress bit feature to the double tap feature.
> >
> > Any tap event is defined by a rising signal edge above threshold, i.e.
> > duration time starts counting; and the falling edge under threshold
> > within duration time, i.e. then the tap event is issued. This means
> > duration is used individually for each tap event.
> >
> > For double tap detection after a single tap, a latency time needs to be
> > specified. Usually tap events, i.e. spikes above and returning below
> > threshold will be ignored within latency. After latency, the window
> > time starts counting for a second tap detection which has to happen
> > within a duration time.
> >
> > If the suppress bit is not set, spikes within latency time are ignored.
> > Setting the suppress bit will invalidate the double tap function. The
> > sensor will thus be able to save the window time for double tap
> > detection, and follow a more strict definition of what signal qualifies
> > for a double tap.
>
> Silly question.  Is there a reason this function would ever be
> turned off?   Seems like a sensible heuristic that would not stop
> genuine double taps being detected.  Maybe we just always leave it on?
>
> Sometimes the best ABI is the one that doesn't exist as userspace
> can't use it wrong.
>
> Jonathan
>

hehehe..  you already mentioned this point, I guess. At least I tried
to put my understanding of it into the lengthy comment of the patch.
Well, patches with lengthy comments.... this seems to go into the same
direction as the wisdom of better limiting userspace interfaces in
general ;)

TBH you have probably seen far more sensors than me, as I'm doing this
just as hobbyist to learn and for fun. I only can provide my
understanding of the particular datasheet.
I think, to set or not to set this bit changes little. It influences a
bit how restrictive the latency period is handled at detection.
Doubletaps are detected with or without having the "suppress" bit set.
If set, AFAIK it could be harder to detect doubletaps. So to speak,
you could reduce "noise" in double tapping (?), or if one receives too
many double taps...(?) perhaps,  ..eh.. legal reasons?! Personally,
I'd liked to play with this sensor a bit, and I found it then useful
to have some kind of knob to change a bit and see what happens without
really messing things up.
As I'm not too familiar with the accelerometer scene and such kind of
"power user settings". I'm unsure if there are typical usecases here.
I would agree that usually one would leave that in one  setting,
turned on or off (unless he/she enters in obsession with double taps).

Perhaps I'll change this patch so that it's always set or not set (to
bring it initially into a defined state), but no sysfs is around.
Let's see. If you think I'd better just drop it entirly, let me know
then.

Best,
L

>
> >
> > This brings in a new ABI functionality.
> > ---
> > Q: Perhaps there is already some IIO ABI for it? If not, please let me
> > know which ABI documentation to extend. There will be a documentation
> > patch also later in this series.
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> > ---
> >  drivers/iio/accel/adxl345_core.c | 82 ++++++++++++++++++++++++++++++++
> >  1 file changed, 82 insertions(+)
> >
> > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> > index cf35a8f9f432..b6966fee3e3d 100644
> > --- a/drivers/iio/accel/adxl345_core.c
> > +++ b/drivers/iio/accel/adxl345_core.c
> > @@ -34,6 +34,7 @@
> >  #define ADXL345_INT2                 1
> >
> >  #define ADXL345_REG_TAP_AXIS_MSK     GENMASK(2, 0)
> > +#define ADXL345_REG_TAP_SUPPRESS_MSK BIT(3)
> >
> >  enum adxl345_axis {
> >       ADXL345_Z_EN = BIT(0),
> > @@ -81,6 +82,7 @@ struct adxl345_state {
> >       u32 tap_duration_us;
> >       u32 tap_latent_us;
> >       u32 tap_window_us;
> > +     bool tap_suppressed;
> >
> >       __le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
> >  };
> > @@ -243,6 +245,31 @@ static int adxl345_set_doubletap_en(struct adxl345_state *st, bool en)
> >       return _adxl345_set_tap_int(st, ADXL345_DOUBLE_TAP, en);
> >  }
> >
> > +static int adxl345_is_suppressed_en(struct adxl345_state *st, bool *en)
> > +{
> > +     *en = st->tap_suppressed;
> > +
> > +     return 0;
> > +}
> > +
> > +static int adxl345_set_suppressed_en(struct adxl345_state *st, bool en)
> > +{
> > +     unsigned long regval = 0;
> > +     int ret;
> > +
> > +     en ? __set_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval)
> > +             : __clear_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval);
> > +
> > +     ret = regmap_update_bits(st->regmap, ADXL345_REG_TAP_AXIS,
> > +                              ADXL345_REG_TAP_SUPPRESS_MSK, regval);
> > +     if (ret)
> > +             return ret;
> > +
> > +     st->tap_suppressed = en;
> > +
> > +     return 0;
> > +}
> > +
> >  static int adxl345_set_tap_threshold(struct adxl345_state *st, u8 val)
> >  {
> >       int ret;
> > @@ -616,6 +643,60 @@ static int adxl345_write_raw_get_fmt(struct iio_dev *indio_dev,
> >       }
> >  }
> >
> > +static ssize_t in_accel_gesture_doubletap_suppressed_en_show(struct device *dev,
> > +                                                          struct device_attribute *attr,
> > +                                                          char *buf)
> > +{
> > +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > +     struct adxl345_state *st = iio_priv(indio_dev);
> > +     bool en;
> > +     int val, ret;
> > +
> > +     ret = adxl345_is_suppressed_en(st, &en);
> > +     if (ret)
> > +             return ret;
> > +     val = en ? 1 : 0;
> > +
> > +     return iio_format_value(buf, IIO_VAL_INT, 1, &val);
> > +}
> > +
> > +static ssize_t in_accel_gesture_doubletap_suppressed_en_store(struct device *dev,
> > +                                                           struct device_attribute *attr,
> > +                                                           const char *buf, size_t len)
> > +{
> > +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > +     struct adxl345_state *st = iio_priv(indio_dev);
> > +     int val, ret;
> > +
> > +     ret = kstrtoint(buf, 0, &val);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = adxl345_set_measure_en(st, false);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret = adxl345_set_suppressed_en(st, val > 0);
> > +     if (ret)
> > +             return ret;
> > +
> > +     ret =  adxl345_set_measure_en(st, true);
> > +     if (ret)
> > +             return ret;
> > +
> > +     return len;
> > +}
> > +static IIO_DEVICE_ATTR_RW(in_accel_gesture_doubletap_suppressed_en, 0);
> > +
> > +static struct attribute *adxl345_event_attrs[] = {
> > +     &iio_dev_attr_in_accel_gesture_doubletap_suppressed_en.dev_attr.attr,
> > +     NULL
> > +};
> > +
> > +static const struct attribute_group adxl345_event_attrs_group = {
> > +     .attrs = adxl345_event_attrs,
> > +};
> > +
> >  static void adxl345_powerdown(void *ptr)
> >  {
> >       struct adxl345_state *st = ptr;
> > @@ -899,6 +980,7 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
> >
> >  static const struct iio_info adxl345_info = {
> >       .attrs          = &adxl345_attrs_group,
> > +     .event_attrs    = &adxl345_event_attrs_group,
> >       .read_raw       = adxl345_read_raw,
> >       .write_raw      = adxl345_write_raw,
> >       .write_raw_get_fmt      = adxl345_write_raw_get_fmt,
>
Jonathan Cameron Feb. 20, 2025, 5:47 p.m. UTC | #3
On Tue, 18 Feb 2025 23:29:46 +0100
Lothar Rubusch <l.rubusch@gmail.com> wrote:

> Dear Jonathan, find my answer down below.
> 
> On Sun, Feb 16, 2025 at 6:28 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Mon, 10 Feb 2025 11:01:12 +0000
> > Lothar Rubusch <l.rubusch@gmail.com> wrote:
> >  
> > > Add the suppress bit feature to the double tap feature.
> > >
> > > Any tap event is defined by a rising signal edge above threshold, i.e.
> > > duration time starts counting; and the falling edge under threshold
> > > within duration time, i.e. then the tap event is issued. This means
> > > duration is used individually for each tap event.
> > >
> > > For double tap detection after a single tap, a latency time needs to be
> > > specified. Usually tap events, i.e. spikes above and returning below
> > > threshold will be ignored within latency. After latency, the window
> > > time starts counting for a second tap detection which has to happen
> > > within a duration time.
> > >
> > > If the suppress bit is not set, spikes within latency time are ignored.
> > > Setting the suppress bit will invalidate the double tap function. The
> > > sensor will thus be able to save the window time for double tap
> > > detection, and follow a more strict definition of what signal qualifies
> > > for a double tap.  
> >
> > Silly question.  Is there a reason this function would ever be
> > turned off?   Seems like a sensible heuristic that would not stop
> > genuine double taps being detected.  Maybe we just always leave it on?
> >
> > Sometimes the best ABI is the one that doesn't exist as userspace
> > can't use it wrong.
> >
> > Jonathan
> >  
> 
> hehehe..  you already mentioned this point, I guess. At least I tried
> to put my understanding of it into the lengthy comment of the patch.
> Well, patches with lengthy comments.... this seems to go into the same
> direction as the wisdom of better limiting userspace interfaces in
> general ;)
> 
> TBH you have probably seen far more sensors than me, as I'm doing this
> just as hobbyist to learn and for fun. I only can provide my
> understanding of the particular datasheet.
> I think, to set or not to set this bit changes little. It influences a
> bit how restrictive the latency period is handled at detection.
> Doubletaps are detected with or without having the "suppress" bit set.
> If set, AFAIK it could be harder to detect doubletaps. So to speak,
> you could reduce "noise" in double tapping (?), or if one receives too
> many double taps...(?) perhaps,  ..eh.. legal reasons?! Personally,
> I'd liked to play with this sensor a bit, and I found it then useful
> to have some kind of knob to change a bit and see what happens without
> really messing things up.
> As I'm not too familiar with the accelerometer scene and such kind of
> "power user settings". I'm unsure if there are typical usecases here.
> I would agree that usually one would leave that in one  setting,
> turned on or off (unless he/she enters in obsession with double taps).
> 
> Perhaps I'll change this patch so that it's always set or not set (to
> bring it initially into a defined state), but no sysfs is around.
> Let's see. If you think I'd better just drop it entirly, let me know
> then.
I think default to always set.  We can revisit the ABI question later
if turns out to have be something people change in practice!

Jonathan

> 
> Best,
> L
> 
> >  
> > >
> > > This brings in a new ABI functionality.
> > > ---
> > > Q: Perhaps there is already some IIO ABI for it? If not, please let me
> > > know which ABI documentation to extend. There will be a documentation
> > > patch also later in this series.
> > >
> > > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> > > ---
> > >  drivers/iio/accel/adxl345_core.c | 82 ++++++++++++++++++++++++++++++++
> > >  1 file changed, 82 insertions(+)
> > >
> > > diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
> > > index cf35a8f9f432..b6966fee3e3d 100644
> > > --- a/drivers/iio/accel/adxl345_core.c
> > > +++ b/drivers/iio/accel/adxl345_core.c
> > > @@ -34,6 +34,7 @@
> > >  #define ADXL345_INT2                 1
> > >
> > >  #define ADXL345_REG_TAP_AXIS_MSK     GENMASK(2, 0)
> > > +#define ADXL345_REG_TAP_SUPPRESS_MSK BIT(3)
> > >
> > >  enum adxl345_axis {
> > >       ADXL345_Z_EN = BIT(0),
> > > @@ -81,6 +82,7 @@ struct adxl345_state {
> > >       u32 tap_duration_us;
> > >       u32 tap_latent_us;
> > >       u32 tap_window_us;
> > > +     bool tap_suppressed;
> > >
> > >       __le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
> > >  };
> > > @@ -243,6 +245,31 @@ static int adxl345_set_doubletap_en(struct adxl345_state *st, bool en)
> > >       return _adxl345_set_tap_int(st, ADXL345_DOUBLE_TAP, en);
> > >  }
> > >
> > > +static int adxl345_is_suppressed_en(struct adxl345_state *st, bool *en)
> > > +{
> > > +     *en = st->tap_suppressed;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int adxl345_set_suppressed_en(struct adxl345_state *st, bool en)
> > > +{
> > > +     unsigned long regval = 0;
> > > +     int ret;
> > > +
> > > +     en ? __set_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval)
> > > +             : __clear_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval);
> > > +
> > > +     ret = regmap_update_bits(st->regmap, ADXL345_REG_TAP_AXIS,
> > > +                              ADXL345_REG_TAP_SUPPRESS_MSK, regval);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     st->tap_suppressed = en;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > >  static int adxl345_set_tap_threshold(struct adxl345_state *st, u8 val)
> > >  {
> > >       int ret;
> > > @@ -616,6 +643,60 @@ static int adxl345_write_raw_get_fmt(struct iio_dev *indio_dev,
> > >       }
> > >  }
> > >
> > > +static ssize_t in_accel_gesture_doubletap_suppressed_en_show(struct device *dev,
> > > +                                                          struct device_attribute *attr,
> > > +                                                          char *buf)
> > > +{
> > > +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > > +     struct adxl345_state *st = iio_priv(indio_dev);
> > > +     bool en;
> > > +     int val, ret;
> > > +
> > > +     ret = adxl345_is_suppressed_en(st, &en);
> > > +     if (ret)
> > > +             return ret;
> > > +     val = en ? 1 : 0;
> > > +
> > > +     return iio_format_value(buf, IIO_VAL_INT, 1, &val);
> > > +}
> > > +
> > > +static ssize_t in_accel_gesture_doubletap_suppressed_en_store(struct device *dev,
> > > +                                                           struct device_attribute *attr,
> > > +                                                           const char *buf, size_t len)
> > > +{
> > > +     struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > > +     struct adxl345_state *st = iio_priv(indio_dev);
> > > +     int val, ret;
> > > +
> > > +     ret = kstrtoint(buf, 0, &val);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = adxl345_set_measure_en(st, false);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret = adxl345_set_suppressed_en(st, val > 0);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     ret =  adxl345_set_measure_en(st, true);
> > > +     if (ret)
> > > +             return ret;
> > > +
> > > +     return len;
> > > +}
> > > +static IIO_DEVICE_ATTR_RW(in_accel_gesture_doubletap_suppressed_en, 0);
> > > +
> > > +static struct attribute *adxl345_event_attrs[] = {
> > > +     &iio_dev_attr_in_accel_gesture_doubletap_suppressed_en.dev_attr.attr,
> > > +     NULL
> > > +};
> > > +
> > > +static const struct attribute_group adxl345_event_attrs_group = {
> > > +     .attrs = adxl345_event_attrs,
> > > +};
> > > +
> > >  static void adxl345_powerdown(void *ptr)
> > >  {
> > >       struct adxl345_state *st = ptr;
> > > @@ -899,6 +980,7 @@ static irqreturn_t adxl345_irq_handler(int irq, void *p)
> > >
> > >  static const struct iio_info adxl345_info = {
> > >       .attrs          = &adxl345_attrs_group,
> > > +     .event_attrs    = &adxl345_event_attrs_group,
> > >       .read_raw       = adxl345_read_raw,
> > >       .write_raw      = adxl345_write_raw,
> > >       .write_raw_get_fmt      = adxl345_write_raw_get_fmt,  
> >  
>
diff mbox series

Patch

diff --git a/drivers/iio/accel/adxl345_core.c b/drivers/iio/accel/adxl345_core.c
index cf35a8f9f432..b6966fee3e3d 100644
--- a/drivers/iio/accel/adxl345_core.c
+++ b/drivers/iio/accel/adxl345_core.c
@@ -34,6 +34,7 @@ 
 #define ADXL345_INT2			1
 
 #define ADXL345_REG_TAP_AXIS_MSK	GENMASK(2, 0)
+#define ADXL345_REG_TAP_SUPPRESS_MSK	BIT(3)
 
 enum adxl345_axis {
 	ADXL345_Z_EN = BIT(0),
@@ -81,6 +82,7 @@  struct adxl345_state {
 	u32 tap_duration_us;
 	u32 tap_latent_us;
 	u32 tap_window_us;
+	bool tap_suppressed;
 
 	__le16 fifo_buf[ADXL345_DIRS * ADXL345_FIFO_SIZE + 1] __aligned(IIO_DMA_MINALIGN);
 };
@@ -243,6 +245,31 @@  static int adxl345_set_doubletap_en(struct adxl345_state *st, bool en)
 	return _adxl345_set_tap_int(st, ADXL345_DOUBLE_TAP, en);
 }
 
+static int adxl345_is_suppressed_en(struct adxl345_state *st, bool *en)
+{
+	*en = st->tap_suppressed;
+
+	return 0;
+}
+
+static int adxl345_set_suppressed_en(struct adxl345_state *st, bool en)
+{
+	unsigned long regval = 0;
+	int ret;
+
+	en ? __set_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval)
+		: __clear_bit(ilog2(ADXL345_TAP_SUPPRESS), &regval);
+
+	ret = regmap_update_bits(st->regmap, ADXL345_REG_TAP_AXIS,
+				 ADXL345_REG_TAP_SUPPRESS_MSK, regval);
+	if (ret)
+		return ret;
+
+	st->tap_suppressed = en;
+
+	return 0;
+}
+
 static int adxl345_set_tap_threshold(struct adxl345_state *st, u8 val)
 {
 	int ret;
@@ -616,6 +643,60 @@  static int adxl345_write_raw_get_fmt(struct iio_dev *indio_dev,
 	}
 }
 
+static ssize_t in_accel_gesture_doubletap_suppressed_en_show(struct device *dev,
+							     struct device_attribute *attr,
+							     char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct adxl345_state *st = iio_priv(indio_dev);
+	bool en;
+	int val, ret;
+
+	ret = adxl345_is_suppressed_en(st, &en);
+	if (ret)
+		return ret;
+	val = en ? 1 : 0;
+
+	return iio_format_value(buf, IIO_VAL_INT, 1, &val);
+}
+
+static ssize_t in_accel_gesture_doubletap_suppressed_en_store(struct device *dev,
+							      struct device_attribute *attr,
+							      const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct adxl345_state *st = iio_priv(indio_dev);
+	int val, ret;
+
+	ret = kstrtoint(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = adxl345_set_measure_en(st, false);
+	if (ret)
+		return ret;
+
+	ret = adxl345_set_suppressed_en(st, val > 0);
+	if (ret)
+		return ret;
+
+	ret =  adxl345_set_measure_en(st, true);
+	if (ret)
+		return ret;
+
+	return len;
+}
+static IIO_DEVICE_ATTR_RW(in_accel_gesture_doubletap_suppressed_en, 0);
+
+static struct attribute *adxl345_event_attrs[] = {
+	&iio_dev_attr_in_accel_gesture_doubletap_suppressed_en.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group adxl345_event_attrs_group = {
+	.attrs = adxl345_event_attrs,
+};
+
 static void adxl345_powerdown(void *ptr)
 {
 	struct adxl345_state *st = ptr;
@@ -899,6 +980,7 @@  static irqreturn_t adxl345_irq_handler(int irq, void *p)
 
 static const struct iio_info adxl345_info = {
 	.attrs		= &adxl345_attrs_group,
+	.event_attrs	= &adxl345_event_attrs_group,
 	.read_raw	= adxl345_read_raw,
 	.write_raw	= adxl345_write_raw,
 	.write_raw_get_fmt	= adxl345_write_raw_get_fmt,