Re: [PATCH v5 09/10] iio: Add channel for tap and new modifiers for single and double tap
From: Jagath Jog J
Date: Mon May 09 2022 - 01:20:01 EST
Hi Jonathan,
Thanks for accepting the patches.
On Sat, May 7, 2022 at 9:45 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Thu, 5 May 2022 19:00:20 +0530
> Jagath Jog J <jagathjog1996@xxxxxxxxx> wrote:
>
> Strangely the last time I can remember discussion around how to
> support tap detection was way back in the early days of IIO, perhaps
> 10 years ago. I don't recall us ever coming to a conclusion on how to do it.
>
> > Add new channel type for tap and also add new modifiers for single and
> > double tap. This channel and modifiers 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 rising and
> > falling direction.
>
> Not sure how that would work seeing as there is only one modifier
> field and it's not a bitmap.
> The event code would need to encode both what type of tap and
> the direction and there aren't two fields in which to do that.
>
> One way I can see this 'might' work would be to use
> the event type to encode tap and the direction could be 'abused'
> to encode single vs double (or other events like this)
>
> in_accel_x_tap_single
> in_accel_x_tap_double
>
> We could possibly be more generic and have the 'type' as
> 'event' or something like that allowing us to use the
> 7 bit direction field to encode different detectable events
> (I'm not that keen on the name event though, could maybe
> map it to gesture which would cover some of the other
> motion pattern detection devices out there)
>
> That would give us
>
> in_accel_x_event_singletap
> in_accel_y_event_doubletap
>
> etc.
>
> How ever we move forwards we do it this want to be in a new series with a nice
> bold title to attract that attention of people who don't really
> care about he bma400 but do care about tap detection; it's
> a common feature of accelerometers.
Sure, I will try to make a new series for tap events with the given
inputs and send the RFC first to get all the comments from everyone.
>
> Jonathan
>
>
>
> >
> > Signed-off-by: Jagath Jog J <jagathjog1996@xxxxxxxxx>
>
>
> > ---
> > Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
> > drivers/iio/industrialio-core.c | 3 +++
> > include/uapi/linux/iio/types.h | 3 +++
> > tools/iio/iio_event_monitor.c | 6 ++++++
> > 4 files changed, 23 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> > index d4ccc68fdcf0..bf2d10d6ad9b 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-iio
> > +++ b/Documentation/ABI/testing/sysfs-bus-iio
> > @@ -2030,3 +2030,14 @@ Description:
> > Available range for the forced calibration value, expressed as:
> >
> > - a range specified as "[min step max]"
> > +
> > +What: /sys/.../events/in_tap_single_change_en
> > +What: /sys/.../events/in_tap_double_change_en
> > +KernelVersion: 5.19
> > +Contact: linux-iio@xxxxxxxxxxxxxxx
> > +Description:
> > + Accelerometer device detects single or double taps and generate
> > + events when threshold for minimum tap amplitide passes.
> > + E.g. a single tap event is generated when acceleration value
> > + crosses the minimum tap amplitude value set. Where tap threshold
> > + value is set by using in_tap_change_value.
> > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> > index e1ed44dec2ab..9b0d7bbd07fc 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -87,6 +87,7 @@ static const char * const iio_chan_type_name_spec[] = {
> > [IIO_POSITIONRELATIVE] = "positionrelative",
> > [IIO_PHASE] = "phase",
> > [IIO_MASSCONCENTRATION] = "massconcentration",
> > + [IIO_TAP] = "tap"
> > };
> >
> > static const char * const iio_modifier_names[] = {
> > @@ -134,6 +135,8 @@ static const char * const iio_modifier_names[] = {
> > [IIO_MOD_ETHANOL] = "ethanol",
> > [IIO_MOD_H2] = "h2",
> > [IIO_MOD_O2] = "o2",
> > + [IIO_MOD_TAP_SINGLE] = "single",
> > + [IIO_MOD_TAP_DOUBLE] = "double",
> > };
> >
> > /* relies on pairs of these shared then separate */
> > diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
> > index 472cead10d8d..d1e61c84e0d5 100644
> > --- a/include/uapi/linux/iio/types.h
> > +++ b/include/uapi/linux/iio/types.h
> > @@ -47,6 +47,7 @@ enum iio_chan_type {
> > IIO_POSITIONRELATIVE,
> > IIO_PHASE,
> > IIO_MASSCONCENTRATION,
> > + IIO_TAP,
> > };
> >
> > enum iio_modifier {
> > @@ -95,6 +96,8 @@ enum iio_modifier {
> > IIO_MOD_ETHANOL,
> > IIO_MOD_H2,
> > IIO_MOD_O2,
> > + IIO_MOD_TAP_SINGLE,
> > + IIO_MOD_TAP_DOUBLE,
> > };
> >
> > enum iio_event_type {
> > diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
> > index 2f4581658859..7fa7d4285f40 100644
> > --- a/tools/iio/iio_event_monitor.c
> > +++ b/tools/iio/iio_event_monitor.c
> > @@ -59,6 +59,7 @@ static const char * const iio_chan_type_name_spec[] = {
> > [IIO_POSITIONRELATIVE] = "positionrelative",
> > [IIO_PHASE] = "phase",
> > [IIO_MASSCONCENTRATION] = "massconcentration",
> > + [IIO_TAP] = "tap",
> > };
> >
> > static const char * const iio_ev_type_text[] = {
> > @@ -122,6 +123,8 @@ static const char * const iio_modifier_names[] = {
> > [IIO_MOD_PM4] = "pm4",
> > [IIO_MOD_PM10] = "pm10",
> > [IIO_MOD_O2] = "o2",
> > + [IIO_MOD_TAP_SINGLE] = "single",
> > + [IIO_MOD_TAP_DOUBLE] = "double",
> > };
> >
> > static bool event_is_known(struct iio_event_data *event)
> > @@ -164,6 +167,7 @@ static bool event_is_known(struct iio_event_data *event)
> > case IIO_POSITIONRELATIVE:
> > case IIO_PHASE:
> > case IIO_MASSCONCENTRATION:
> > + case IIO_TAP:
> > break;
> > default:
> > return false;
> > @@ -215,6 +219,8 @@ static bool event_is_known(struct iio_event_data *event)
> > case IIO_MOD_PM4:
> > case IIO_MOD_PM10:
> > case IIO_MOD_O2:
> > + case IIO_MOD_TAP_SINGLE:
> > + case IIO_MOD_TAP_DOUBLE:
> > break;
> > default:
> > return false;
>