2022-08-13 07:20:48

by Jagath Jog J

[permalink] [raw]
Subject: [PATCH v2 1/2] iio: Add new event type gesture and use direction for single and double tap

Add new event type for tap called gesture and the direction can be used
to differentiate single and double tap. This may be used by accelerometer
sensors to express single and double tap events. For directional tap,
modifiers like IIO_MOD_(X/Y/Z) can be used along with singletap and
doubletap direction.

Signed-off-by: Jagath Jog J <[email protected]>
---
Documentation/ABI/testing/sysfs-bus-iio | 41 +++++++++++++++++++++++++
drivers/iio/industrialio-event.c | 7 ++++-
include/linux/iio/types.h | 2 ++
include/uapi/linux/iio/types.h | 3 ++
tools/iio/iio_event_monitor.c | 8 ++++-
5 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index e81ba6f5e1c8..54cb925f714c 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2038,3 +2038,44 @@ Description:
Available range for the forced calibration value, expressed as:

- a range specified as "[min step max]"
+
+What: /sys/.../events/in_accel_gesture_singletap_en
+What: /sys/.../events/in_accel_gesture_doubletap_en
+KernelVersion: 5.21
+Contact: [email protected]
+Description:
+ Device generates an event on a single or double tap.
+
+What: /sys/.../events/in_accel_gesture_singletap_value
+What: /sys/.../events/in_accel_gesture_doubletap_value
+KernelVersion: 5.21
+Contact: [email protected]
+Description:
+ Specifies the threshold value that the device is comparing
+ against to generate the tap gesture event. Units and exact
+ meaning of value are device specific.
+
+What: /sys/.../events/in_accel_gesture_singletap_reset_timeout
+What: /sys/.../events/in_accel_gesture_doubletap_reset_timeout
+KernelVersion: 5.21
+Contact: [email protected]
+Description:
+ Specifies the timeout value in seconds for the tap detector
+ to not to look for another tap event after the event as
+ occoured. Basically the minimum quiet time between the two
+ single-tap's or two double-tap's.
+
+What: /sys/.../events/in_accel_gesture_doubletap_tap_2min_delay
+KernelVersion: 5.21
+Contact: [email protected]
+Description:
+ Specifies the minimum quiet time in seconds between the two
+ taps of a double tap.
+
+What: /sys/.../events/in_accel_gesture_maxtomin_time
+KernelVersion: 5.21
+Contact: [email protected]
+Description:
+ Specifies the maximum time difference allowed between upper
+ and lower peak of tap to consider it as the valid tap event.
+ Units in seconds.
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index b5e059e15b0a..b63be2a4ac02 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -231,12 +231,15 @@ static const char * const iio_ev_type_text[] = {
[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
[IIO_EV_TYPE_CHANGE] = "change",
[IIO_EV_TYPE_MAG_REFERENCED] = "mag_referenced",
+ [IIO_EV_TYPE_GESTURE] = "gesture",
};

static const char * const iio_ev_dir_text[] = {
[IIO_EV_DIR_EITHER] = "either",
[IIO_EV_DIR_RISING] = "rising",
- [IIO_EV_DIR_FALLING] = "falling"
+ [IIO_EV_DIR_FALLING] = "falling",
+ [IIO_EV_DIR_SINGLETAP] = "singletap",
+ [IIO_EV_DIR_DOUBLETAP] = "doubletap",
};

static const char * const iio_ev_info_text[] = {
@@ -247,6 +250,8 @@ static const char * const iio_ev_info_text[] = {
[IIO_EV_INFO_HIGH_PASS_FILTER_3DB] = "high_pass_filter_3db",
[IIO_EV_INFO_LOW_PASS_FILTER_3DB] = "low_pass_filter_3db",
[IIO_EV_INFO_TIMEOUT] = "timeout",
+ [IIO_EV_INFO_RESET_TIMEOUT] = "reset_timeout",
+ [IIO_EV_INFO_TAP_2MIN_DELAY] = "tap_2min_delay",
};

static enum iio_event_direction iio_ev_attr_dir(struct iio_dev_attr *attr)
diff --git a/include/linux/iio/types.h b/include/linux/iio/types.h
index a7aa91f3a8dc..e33069d6a3d8 100644
--- a/include/linux/iio/types.h
+++ b/include/linux/iio/types.h
@@ -17,6 +17,8 @@ enum iio_event_info {
IIO_EV_INFO_HIGH_PASS_FILTER_3DB,
IIO_EV_INFO_LOW_PASS_FILTER_3DB,
IIO_EV_INFO_TIMEOUT,
+ IIO_EV_INFO_RESET_TIMEOUT,
+ IIO_EV_INFO_TAP_2MIN_DELAY,
};

#define IIO_VAL_INT 1
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index 472cead10d8d..913864221ac4 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -105,6 +105,7 @@ enum iio_event_type {
IIO_EV_TYPE_MAG_ADAPTIVE,
IIO_EV_TYPE_CHANGE,
IIO_EV_TYPE_MAG_REFERENCED,
+ IIO_EV_TYPE_GESTURE,
};

enum iio_event_direction {
@@ -112,6 +113,8 @@ enum iio_event_direction {
IIO_EV_DIR_RISING,
IIO_EV_DIR_FALLING,
IIO_EV_DIR_NONE,
+ IIO_EV_DIR_SINGLETAP,
+ IIO_EV_DIR_DOUBLETAP,
};

#endif /* _UAPI_IIO_TYPES_H_ */
diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
index 2f4581658859..b3b3ea399f67 100644
--- a/tools/iio/iio_event_monitor.c
+++ b/tools/iio/iio_event_monitor.c
@@ -69,12 +69,15 @@ static const char * const iio_ev_type_text[] = {
[IIO_EV_TYPE_MAG_ADAPTIVE] = "mag_adaptive",
[IIO_EV_TYPE_CHANGE] = "change",
[IIO_EV_TYPE_MAG_REFERENCED] = "mag_referenced",
+ [IIO_EV_TYPE_GESTURE] = "gesture",
};

static const char * const iio_ev_dir_text[] = {
[IIO_EV_DIR_EITHER] = "either",
[IIO_EV_DIR_RISING] = "rising",
- [IIO_EV_DIR_FALLING] = "falling"
+ [IIO_EV_DIR_FALLING] = "falling",
+ [IIO_EV_DIR_SINGLETAP] = "singletap",
+ [IIO_EV_DIR_DOUBLETAP] = "doubletap",
};

static const char * const iio_modifier_names[] = {
@@ -227,6 +230,7 @@ static bool event_is_known(struct iio_event_data *event)
case IIO_EV_TYPE_THRESH_ADAPTIVE:
case IIO_EV_TYPE_MAG_ADAPTIVE:
case IIO_EV_TYPE_CHANGE:
+ case IIO_EV_TYPE_GESTURE:
break;
default:
return false;
@@ -236,6 +240,8 @@ static bool event_is_known(struct iio_event_data *event)
case IIO_EV_DIR_EITHER:
case IIO_EV_DIR_RISING:
case IIO_EV_DIR_FALLING:
+ case IIO_EV_DIR_SINGLETAP:
+ case IIO_EV_DIR_DOUBLETAP:
case IIO_EV_DIR_NONE:
break;
default:
--
2.17.1


2022-08-14 17:15:04

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: Add new event type gesture and use direction for single and double tap

On Sat, 13 Aug 2022 12:48:02 +0530
Jagath Jog J <[email protected]> wrote:

> Add new event type for tap called gesture and the direction can be used
> to differentiate single and double tap. This may be used by accelerometer
> sensors to express single and double tap events. For directional tap,
> modifiers like IIO_MOD_(X/Y/Z) can be used along with singletap and
> doubletap direction.
>
> Signed-off-by: Jagath Jog J <[email protected]>
Hi Jagath,

This ABI is definitely something I want more eyes than ours on, so
whatever happens I'll leave it on the list for a few weeks.

> ---
> Documentation/ABI/testing/sysfs-bus-iio | 41 +++++++++++++++++++++++++
> drivers/iio/industrialio-event.c | 7 ++++-
> include/linux/iio/types.h | 2 ++
> include/uapi/linux/iio/types.h | 3 ++
> tools/iio/iio_event_monitor.c | 8 ++++-
> 5 files changed, 59 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index e81ba6f5e1c8..54cb925f714c 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -2038,3 +2038,44 @@ Description:
> Available range for the forced calibration value, expressed as:
>
> - a range specified as "[min step max]"
> +
> +What: /sys/.../events/in_accel_gesture_singletap_en
> +What: /sys/.../events/in_accel_gesture_doubletap_en
> +KernelVersion: 5.21
> +Contact: [email protected]
> +Description:
> + Device generates an event on a single or double tap.
> +
> +What: /sys/.../events/in_accel_gesture_singletap_value
> +What: /sys/.../events/in_accel_gesture_doubletap_value
> +KernelVersion: 5.21
> +Contact: [email protected]
> +Description:
> + Specifies the threshold value that the device is comparing
> + against to generate the tap gesture event. Units and exact
> + meaning of value are device specific.

I wonder if we should list a direction? As in smaller is more sensitive?
(at least to first approximation)
That way a user would at least be able to consistently decide if they should
raise or lower the number to get the perf the want.

> +
> +What: /sys/.../events/in_accel_gesture_singletap_reset_timeout
> +What: /sys/.../events/in_accel_gesture_doubletap_reset_timeout
> +KernelVersion: 5.21
> +Contact: [email protected]
> +Description:
> + Specifies the timeout value in seconds for the tap detector
> + to not to look for another tap event after the event as
> + occoured. Basically the minimum quiet time between the two
spelling. occured

> + single-tap's or two double-tap's.
> +
> +What: /sys/.../events/in_accel_gesture_doubletap_tap_2min_delay

I'm not sure this naming is intuitive enough. Might be a simple
as doubletap_tap2_min_delay? My brain didn't parse 2min correctly.

This one is a bit odd, so definitely want to hear more view points on whether
this is general enough to cover sensors and intuitive enough that people
have some hope of setting it right.

> +KernelVersion: 5.21
> +Contact: [email protected]
> +Description:
> + Specifies the minimum quiet time in seconds between the two
> + taps of a double tap.
> +
> +What: /sys/.../events/in_accel_gesture_maxtomin_time
> +KernelVersion: 5.21
> +Contact: [email protected]
> +Description:
> + Specifies the maximum time difference allowed between upper
> + and lower peak of tap to consider it as the valid tap event.
> + Units in seconds.
Needs to be associated with 'tap' in the naming.
Easiest is probably only to define it as
singletap_maxtomin_time + doubletap_maxtomin_time and not have the
shared version as we'd lose the 'tap' part of the name.


2022-08-15 21:13:14

by Jagath Jog J

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: Add new event type gesture and use direction for single and double tap

Hi Jonathan,

On Sun, Aug 14, 2022 at 10:24 PM Jonathan Cameron <[email protected]> wrote:
>
> On Sat, 13 Aug 2022 12:48:02 +0530
> Jagath Jog J <[email protected]> wrote:
>
> > Add new event type for tap called gesture and the direction can be used
> > to differentiate single and double tap. This may be used by accelerometer
> > sensors to express single and double tap events. For directional tap,
> > modifiers like IIO_MOD_(X/Y/Z) can be used along with singletap and
> > doubletap direction.
> >
> > Signed-off-by: Jagath Jog J <[email protected]>
> Hi Jagath,
>
> This ABI is definitely something I want more eyes than ours on, so
> whatever happens I'll leave it on the list for a few weeks.

Sure, I will leave KernelVersion blank in the next series.

>
> > ---
> > Documentation/ABI/testing/sysfs-bus-iio | 41 +++++++++++++++++++++++++
> > drivers/iio/industrialio-event.c | 7 ++++-
> > include/linux/iio/types.h | 2 ++
> > include/uapi/linux/iio/types.h | 3 ++
> > tools/iio/iio_event_monitor.c | 8 ++++-
> > 5 files changed, 59 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > index e81ba6f5e1c8..54cb925f714c 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > @@ -2038,3 +2038,44 @@ Description:
> > Available range for the forced calibration value, expressed as:
> >
> > - a range specified as "[min step max]"
> > +
> > +What: /sys/.../events/in_accel_gesture_singletap_en
> > +What: /sys/.../events/in_accel_gesture_doubletap_en
> > +KernelVersion: 5.21
> > +Contact: [email protected]
> > +Description:
> > + Device generates an event on a single or double tap.
> > +
> > +What: /sys/.../events/in_accel_gesture_singletap_value
> > +What: /sys/.../events/in_accel_gesture_doubletap_value
> > +KernelVersion: 5.21
> > +Contact: [email protected]
> > +Description:
> > + Specifies the threshold value that the device is comparing
> > + against to generate the tap gesture event. Units and exact
> > + meaning of value are device specific.
>
> I wonder if we should list a direction? As in smaller is more sensitive?

Yeah in most of the devices which support tap, this value represents the
threshold, the lower the value higher the tap sensitivity. I will add it to the
description in the next series.

> (at least to first approximation)
Do I need to add available attributes into ABI docs?

> That way a user would at least be able to consistently decide if they should
> raise or lower the number to get the perf the want.
>
> > +
> > +What: /sys/.../events/in_accel_gesture_singletap_reset_timeout
> > +What: /sys/.../events/in_accel_gesture_doubletap_reset_timeout
> > +KernelVersion: 5.21
> > +Contact: [email protected]
> > +Description:
> > + Specifies the timeout value in seconds for the tap detector
> > + to not to look for another tap event after the event as
> > + occoured. Basically the minimum quiet time between the two
> spelling. occured

Sorry, I will correct this.

Thank you
Jagath

>
> > + single-tap's or two double-tap's.
> > +
> > +What: /sys/.../events/in_accel_gesture_doubletap_tap_2min_delay
>
> I'm not sure this naming is intuitive enough. Might be a simple
> as doubletap_tap2_min_delay? My brain didn't parse 2min correctly.
>
> This one is a bit odd, so definitely want to hear more view points on whether
> this is general enough to cover sensors and intuitive enough that people
> have some hope of setting it right.
>
> > +KernelVersion: 5.21
> > +Contact: [email protected]
> > +Description:
> > + Specifies the minimum quiet time in seconds between the two
> > + taps of a double tap.
> > +
> > +What: /sys/.../events/in_accel_gesture_maxtomin_time
> > +KernelVersion: 5.21
> > +Contact: [email protected]
> > +Description:
> > + Specifies the maximum time difference allowed between upper
> > + and lower peak of tap to consider it as the valid tap event.
> > + Units in seconds.
> Needs to be associated with 'tap' in the naming.
> Easiest is probably only to define it as
> singletap_maxtomin_time + doubletap_maxtomin_time and not have the
> shared version as we'd lose the 'tap' part of the name.
>
>

2022-08-19 09:19:04

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: Add new event type gesture and use direction for single and double tap

On Mon, Aug 15, 2022 at 9:33 PM Jagath Jog J <[email protected]> wrote:
> On Sun, Aug 14, 2022 at 10:24 PM Jonathan Cameron <[email protected]> wrote:

> Sure, I will leave KernelVersion blank in the next series.

You may use 6.1, that way we don't forget about it.

I'm wondering if checkpatch or other tools can validate the ABI descriptions.

--
With Best Regards,
Andy Shevchenko

2022-08-20 13:44:20

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] iio: Add new event type gesture and use direction for single and double tap

On Tue, 16 Aug 2022 00:03:47 +0530
Jagath Jog J <[email protected]> wrote:

> Hi Jonathan,
>
> On Sun, Aug 14, 2022 at 10:24 PM Jonathan Cameron <[email protected]> wrote:
> >
> > On Sat, 13 Aug 2022 12:48:02 +0530
> > Jagath Jog J <[email protected]> wrote:
> >
> > > Add new event type for tap called gesture and the direction can be used
> > > to differentiate single and double tap. This may be used by accelerometer
> > > sensors to express single and double tap events. For directional tap,
> > > modifiers like IIO_MOD_(X/Y/Z) can be used along with singletap and
> > > doubletap direction.
> > >
> > > Signed-off-by: Jagath Jog J <[email protected]>
> > Hi Jagath,
> >
> > This ABI is definitely something I want more eyes than ours on, so
> > whatever happens I'll leave it on the list for a few weeks.
>
> Sure, I will leave KernelVersion blank in the next series.
Please don't. Just take a guess at 6.1. That will probably be right
and it'll save me effort when applying :) If it's not right then
no harm done as I need to edit it anyway.

>
> >
> > > ---
> > > Documentation/ABI/testing/sysfs-bus-iio | 41 +++++++++++++++++++++++++
> > > drivers/iio/industrialio-event.c | 7 ++++-
> > > include/linux/iio/types.h | 2 ++
> > > include/uapi/linux/iio/types.h | 3 ++
> > > tools/iio/iio_event_monitor.c | 8 ++++-
> > > 5 files changed, 59 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > > index e81ba6f5e1c8..54cb925f714c 100644
> > > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > > @@ -2038,3 +2038,44 @@ Description:
> > > Available range for the forced calibration value, expressed as:
> > >
> > > - a range specified as "[min step max]"
> > > +
> > > +What: /sys/.../events/in_accel_gesture_singletap_en
> > > +What: /sys/.../events/in_accel_gesture_doubletap_en
> > > +KernelVersion: 5.21
> > > +Contact: [email protected]
> > > +Description:
> > > + Device generates an event on a single or double tap.
> > > +
> > > +What: /sys/.../events/in_accel_gesture_singletap_value
> > > +What: /sys/.../events/in_accel_gesture_doubletap_value
> > > +KernelVersion: 5.21
> > > +Contact: [email protected]
> > > +Description:
> > > + Specifies the threshold value that the device is comparing
> > > + against to generate the tap gesture event. Units and exact
> > > + meaning of value are device specific.
> >
> > I wonder if we should list a direction? As in smaller is more sensitive?
>
> Yeah in most of the devices which support tap, this value represents the
> threshold, the lower the value higher the tap sensitivity. I will add it to the
> description in the next series.
>
> > (at least to first approximation)
> Do I need to add available attributes into ABI docs?

Yes, though no need to give much description of them.
There is a script that compares the ABI exposed on a given platform with
these files and warns if things are missing. Right now it gives so many
warnings (outside of IIO) that it's more or less unusable but the end goal
is to get to the pointer where systems doing build and boot tests will general
warnings for undocumented ABI.

>
> > That way a user would at least be able to consistently decide if they should
> > raise or lower the number to get the perf the want.
> >
> > > +
> > > +What: /sys/.../events/in_accel_gesture_singletap_reset_timeout
> > > +What: /sys/.../events/in_accel_gesture_doubletap_reset_timeout
> > > +KernelVersion: 5.21
> > > +Contact: [email protected]
> > > +Description:
> > > + Specifies the timeout value in seconds for the tap detector
> > > + to not to look for another tap event after the event as
> > > + occoured. Basically the minimum quiet time between the two
> > spelling. occured
>
> Sorry, I will correct this.
>
> Thank you
> Jagath
>
> >
> > > + single-tap's or two double-tap's.
> > > +
> > > +What: /sys/.../events/in_accel_gesture_doubletap_tap_2min_delay
> >
> > I'm not sure this naming is intuitive enough. Might be a simple
> > as doubletap_tap2_min_delay? My brain didn't parse 2min correctly.
> >
> > This one is a bit odd, so definitely want to hear more view points on whether
> > this is general enough to cover sensors and intuitive enough that people
> > have some hope of setting it right.
> >
> > > +KernelVersion: 5.21
> > > +Contact: [email protected]
> > > +Description:
> > > + Specifies the minimum quiet time in seconds between the two
> > > + taps of a double tap.
> > > +
> > > +What: /sys/.../events/in_accel_gesture_maxtomin_time
> > > +KernelVersion: 5.21
> > > +Contact: [email protected]
> > > +Description:
> > > + Specifies the maximum time difference allowed between upper
> > > + and lower peak of tap to consider it as the valid tap event.
> > > + Units in seconds.
> > Needs to be associated with 'tap' in the naming.
> > Easiest is probably only to define it as
> > singletap_maxtomin_time + doubletap_maxtomin_time and not have the
> > shared version as we'd lose the 'tap' part of the name.
> >
> >