2022-08-25 20:03:13

by Jagath Jog J

[permalink] [raw]
Subject: [PATCH v3 0/2] iio: Add single and double tap events support

This patch series adds new event type for tap called gesture and direction
is used to differentiate single and double tap. This series adds single
and double tap support for bma400 accelerometer device driver.

Changes since v2
1. Replaced doubletap_tap_2min to doubletap_tap2_min.
2. Added ABI docs for available attributes which lists tap configurations
values.
3. Added 'tap' in the naming of available attributes which are related to
tap configurations.
5. Added check for channel type in _read_event_value() and
_write_event_value().
6. KernelVersion changed to 6.1.
7. Corrected typos.

Changes since v1
1. Included headers in alphabetical order.
2. Changing tap_event_en variable name to tap_event_en_bitmask since it is
used in bit manipulation operation.
3. Assigning boolean value to step_event_en and activity_event_en instead
of 0, since they are boolean type members.
4. Using local variable for regmap_read() instead for *val itself.
5. Correcting typos.
6. Remove of IIO_EV_INFO_PERIOD.
7. Now all 4 tap controls like threshold, quiet, tics_dt and quiet_dt can
be configured from the userspace.
8. Introducing new event info IIO_EV_INFO_RESET_TIMEOUT, and
IIO_EV_INFO_TAP_2MIN_DELAY into iio_event_info.
9. Creating custom read/write attributes for tics_dt called
in_accel_gesture_maxtomin_time.
10. Time based tap controls can be configured in seconds instead of raw
values.
11. Provided all available values for time base tap controls in seconds.
12. Adding one more MODULE_AUTHOR().

Changes since RFC
1. Corrected the "quite" typo to "quiet".
2. Added proper reference and name of the section from datasheet.
3. Changed the ABI documentation to make it more generic.
4. Added ABI documentation for double tap quiet period.
5. Added available list by registering new event attribute for tap
threshold values and double tap quiet period values.
6. Sending both single and double tap events separately.
8. Removed checking for tap enabled while changing data rate.
9. Returning invalid with error message if the input data rate is not
200Hz while enabling tap interrupts.
7. Added datasheet reference for interrupt engine overrun.


Jagath Jog J (2):
iio: Add new event type gesture and use direction for single and
double tap
iio: accel: bma400: Add support for single and double tap events

Documentation/ABI/testing/sysfs-bus-iio | 69 +++++
drivers/iio/accel/bma400.h | 13 +
drivers/iio/accel/bma400_core.c | 343 +++++++++++++++++++++++-
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 +-
7 files changed, 433 insertions(+), 12 deletions(-)

--
2.17.1


2022-08-25 20:03:13

by Jagath Jog J

[permalink] [raw]
Subject: [PATCH v3 2/2] iio: accel: bma400: Add support for single and double tap events

Add support for single and double tap events based on the tap threshold
value, minimum quiet time before and after the tap and minimum time
between the taps in the double tap. The INT1 pin is used to interrupt
and the event is pushed to userspace.

Signed-off-by: Jagath Jog J <[email protected]>
---
drivers/iio/accel/bma400.h | 13 ++
drivers/iio/accel/bma400_core.c | 343 +++++++++++++++++++++++++++++++-
2 files changed, 346 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
index e8f802a82300..d9e8eccb524b 100644
--- a/drivers/iio/accel/bma400.h
+++ b/drivers/iio/accel/bma400.h
@@ -40,6 +40,7 @@
#define BMA400_INT_STAT1_REG 0x0f
#define BMA400_INT_STAT2_REG 0x10
#define BMA400_INT12_MAP_REG 0x23
+#define BMA400_INT_ENG_OVRUN_MSK BIT(4)

/* Temperature register */
#define BMA400_TEMP_DATA_REG 0x11
@@ -105,6 +106,18 @@
#define BMA400_INT_GEN2_MSK BIT(3)
#define BMA400_GEN_HYST_MSK GENMASK(1, 0)

+/* TAP config registers */
+#define BMA400_TAP_CONFIG 0x57
+#define BMA400_TAP_CONFIG1 0x58
+#define BMA400_S_TAP_MSK BIT(2)
+#define BMA400_D_TAP_MSK BIT(3)
+#define BMA400_INT_S_TAP_MSK BIT(10)
+#define BMA400_INT_D_TAP_MSK BIT(11)
+#define BMA400_TAP_SEN_MSK GENMASK(2, 0)
+#define BMA400_TAP_TICSTH_MSK GENMASK(1, 0)
+#define BMA400_TAP_QUIET_MSK GENMASK(3, 2)
+#define BMA400_TAP_QUIETDT_MSK GENMASK(5, 4)
+
/*
* BMA400_SCALE_MIN macro value represents m/s^2 for 1 LSB before
* converting to micro values for +-2g range.
diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
index c31bdd9b168e..1150778bda3a 100644
--- a/drivers/iio/accel/bma400_core.c
+++ b/drivers/iio/accel/bma400_core.c
@@ -26,6 +26,7 @@
#include <linux/iio/iio.h>
#include <linux/iio/buffer.h>
#include <linux/iio/events.h>
+#include <linux/iio/sysfs.h>
#include <linux/iio/trigger.h>
#include <linux/iio/trigger_consumer.h>
#include <linux/iio/triggered_buffer.h>
@@ -47,6 +48,27 @@ static int bma400_sample_freqs[14];

static const int bma400_osr_range[] = { 0, 1, 3 };

+static int tap_reset_timeout[] = {
+ 300000,
+ 400000,
+ 500000,
+ 600000
+};
+
+static int tap_max2min_time[] = {
+ 30000,
+ 45000,
+ 60000,
+ 90000
+};
+
+static int double_tap2_min_delay[] = {
+ 20000,
+ 40000,
+ 60000,
+ 80000
+};
+
/* See the ACC_CONFIG0 section of the datasheet */
enum bma400_power_mode {
POWER_MODE_SLEEP = 0x00,
@@ -88,6 +110,7 @@ struct bma400_data {
bool step_event_en;
bool activity_event_en;
unsigned int generic_event_en;
+ unsigned int tap_event_en_bitmask;
/* Correct time stamp alignment */
struct {
__le16 buff[3];
@@ -216,6 +239,115 @@ static const struct iio_event_spec bma400_accel_event[] = {
BIT(IIO_EV_INFO_HYSTERESIS) |
BIT(IIO_EV_INFO_ENABLE),
},
+ {
+ .type = IIO_EV_TYPE_GESTURE,
+ .dir = IIO_EV_DIR_SINGLETAP,
+ .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_ENABLE) |
+ BIT(IIO_EV_INFO_RESET_TIMEOUT),
+ },
+ {
+ .type = IIO_EV_TYPE_GESTURE,
+ .dir = IIO_EV_DIR_DOUBLETAP,
+ .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_ENABLE) |
+ BIT(IIO_EV_INFO_RESET_TIMEOUT) |
+ BIT(IIO_EV_INFO_TAP2_MIN_DELAY),
+ },
+};
+
+static int usec_to_tapreg_raw(int usec, const int *time_list)
+{
+ int index;
+
+ for (index = 0; index < 4; index++) {
+ if (usec == time_list[index])
+ return index;
+ }
+ return -EINVAL;
+}
+
+static ssize_t in_accel_gesture_tap_maxtomin_time_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+ struct bma400_data *data = iio_priv(indio_dev);
+ int ret, reg_val, raw, vals[2];
+
+ ret = regmap_read(data->regmap, BMA400_TAP_CONFIG1, &reg_val);
+ if (ret)
+ return ret;
+
+ raw = FIELD_GET(BMA400_TAP_TICSTH_MSK, reg_val);
+ vals[0] = 0;
+ vals[1] = tap_max2min_time[raw];
+
+ return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
+}
+
+static ssize_t in_accel_gesture_tap_maxtomin_time_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 bma400_data *data = iio_priv(indio_dev);
+ int ret, val_int, val_fract, raw;
+
+ ret = iio_str_to_fixpoint(buf, 100000, &val_int, &val_fract);
+ if (ret)
+ return ret;
+
+ raw = usec_to_tapreg_raw(val_fract, tap_max2min_time);
+ if (raw < 0)
+ return -EINVAL;
+
+ ret = regmap_update_bits(data->regmap, BMA400_TAP_CONFIG1,
+ BMA400_TAP_TICSTH_MSK,
+ FIELD_PREP(BMA400_TAP_TICSTH_MSK, raw));
+ if (ret)
+ return ret;
+
+ return len;
+}
+
+static IIO_DEVICE_ATTR_RW(in_accel_gesture_tap_maxtomin_time, 0);
+
+/*
+ * Tap interrupts works with 200 Hz input data rate and the time based tap
+ * controls are in the terms of data samples so the below calculation is
+ * used to convert the configuration values into seconds.
+ * e.g.:
+ * 60 data samples * 0.005 ms = 0.3 seconds.
+ * 80 data samples * 0.005 ms = 0.4 seconds.
+ */
+
+/* quiet configuration values in seconds */
+static IIO_CONST_ATTR(in_accel_gesture_tap_reset_timeout_available,
+ "0.3 0.4 0.5 0.6");
+
+/* tics_th configuration values in seconds */
+static IIO_CONST_ATTR(in_accel_gesture_tap_maxtomin_time_available,
+ "0.03 0.045 0.06 0.09");
+
+/* quiet_dt configuration values in seconds */
+static IIO_CONST_ATTR(in_accel_gesture_doubletap_tap2_min_delay_available,
+ "0.02 0.04 0.06 0.08");
+
+/* List of sensitivity values available to configure tap interrupts */
+static IIO_CONST_ATTR(in_accel_gesture_tap_value_available, "0 1 2 3 4 5 6 7");
+
+static struct attribute *bma400_event_attributes[] = {
+ &iio_const_attr_in_accel_gesture_tap_value_available.dev_attr.attr,
+ &iio_const_attr_in_accel_gesture_tap_reset_timeout_available.dev_attr.attr,
+ &iio_const_attr_in_accel_gesture_tap_maxtomin_time_available.dev_attr.attr,
+ &iio_const_attr_in_accel_gesture_doubletap_tap2_min_delay_available.dev_attr.attr,
+ &iio_dev_attr_in_accel_gesture_tap_maxtomin_time.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group bma400_event_attribute_group = {
+ .attrs = bma400_event_attributes,
};

#define BMA400_ACC_CHANNEL(_index, _axis) { \
@@ -1012,6 +1144,12 @@ static int bma400_read_event_config(struct iio_dev *indio_dev,
case IIO_EV_DIR_FALLING:
return FIELD_GET(BMA400_INT_GEN2_MSK,
data->generic_event_en);
+ case IIO_EV_DIR_SINGLETAP:
+ return FIELD_GET(BMA400_S_TAP_MSK,
+ data->tap_event_en_bitmask);
+ case IIO_EV_DIR_DOUBLETAP:
+ return FIELD_GET(BMA400_D_TAP_MSK,
+ data->tap_event_en_bitmask);
default:
return -EINVAL;
}
@@ -1101,6 +1239,80 @@ static int bma400_activity_event_en(struct bma400_data *data,
return 0;
}

+static int bma400_tap_event_en(struct bma400_data *data,
+ enum iio_event_direction dir, int state)
+{
+ int ret;
+ unsigned int mask, field_value;
+
+ /*
+ * Tap interrupts can be configured only in normal mode.
+ * See table in section 4.3 "Power modes - performance modes" of
+ * datasheet v1.2.
+ */
+ if (data->power_mode != POWER_MODE_NORMAL)
+ return -EINVAL;
+
+ /*
+ * Tap interrupts are operating with the data rate of 200Hz.
+ * See section 4.7 "Tap sensing interrupt" in datasheet v1.2.
+ */
+ if (data->sample_freq.hz != 200 && state) {
+ dev_err(data->dev, "Invalid data rate for tap interrupts.\n");
+ return -EINVAL;
+ }
+
+ ret = regmap_update_bits(data->regmap, BMA400_INT12_MAP_REG,
+ BMA400_S_TAP_MSK,
+ FIELD_PREP(BMA400_S_TAP_MSK, state));
+ if (ret)
+ return ret;
+
+ switch (dir) {
+ case IIO_EV_DIR_SINGLETAP:
+ mask = BMA400_S_TAP_MSK;
+ set_mask_bits(&field_value, BMA400_S_TAP_MSK,
+ FIELD_PREP(BMA400_S_TAP_MSK, state));
+ break;
+ case IIO_EV_DIR_DOUBLETAP:
+ mask = BMA400_D_TAP_MSK;
+ set_mask_bits(&field_value, BMA400_D_TAP_MSK,
+ FIELD_PREP(BMA400_D_TAP_MSK, state));
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG1_REG, mask,
+ field_value);
+ if (ret)
+ return ret;
+
+ set_mask_bits(&data->tap_event_en_bitmask, mask, field_value);
+
+ return 0;
+}
+
+static int bma400_disable_adv_interrupt(struct bma400_data *data)
+{
+ int ret;
+
+ ret = regmap_write(data->regmap, BMA400_INT_CONFIG0_REG, 0);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(data->regmap, BMA400_INT_CONFIG1_REG, 0);
+ if (ret)
+ return ret;
+
+ data->tap_event_en_bitmask = 0;
+ data->generic_event_en = 0;
+ data->step_event_en = false;
+ data->activity_event_en = false;
+
+ return 0;
+}
+
static int bma400_write_event_config(struct iio_dev *indio_dev,
const struct iio_chan_spec *chan,
enum iio_event_type type,
@@ -1111,10 +1323,20 @@ static int bma400_write_event_config(struct iio_dev *indio_dev,

switch (chan->type) {
case IIO_ACCEL:
- mutex_lock(&data->mutex);
- ret = bma400_activity_event_en(data, dir, state);
- mutex_unlock(&data->mutex);
- return ret;
+ switch (type) {
+ case IIO_EV_TYPE_MAG:
+ mutex_lock(&data->mutex);
+ ret = bma400_activity_event_en(data, dir, state);
+ mutex_unlock(&data->mutex);
+ return ret;
+ case IIO_EV_TYPE_GESTURE:
+ mutex_lock(&data->mutex);
+ ret = bma400_tap_event_en(data, dir, state);
+ mutex_unlock(&data->mutex);
+ return ret;
+ default:
+ return -EINVAL;
+ }
case IIO_STEPS:
mutex_lock(&data->mutex);
ret = bma400_steps_event_enable(data, state);
@@ -1157,10 +1379,13 @@ static int bma400_read_event_value(struct iio_dev *indio_dev,
int *val, int *val2)
{
struct bma400_data *data = iio_priv(indio_dev);
- int ret, reg;
+ int ret, reg, reg_val, raw;

- switch (chan->type) {
- case IIO_ACCEL:
+ if (chan->type != IIO_ACCEL)
+ return -EINVAL;
+
+ switch (type) {
+ case IIO_EV_TYPE_MAG:
reg = get_gen_config_reg(dir);
if (reg < 0)
return -EINVAL;
@@ -1196,6 +1421,39 @@ static int bma400_read_event_value(struct iio_dev *indio_dev,
default:
return -EINVAL;
}
+ case IIO_EV_TYPE_GESTURE:
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ ret = regmap_read(data->regmap, BMA400_TAP_CONFIG,
+ &reg_val);
+ if (ret)
+ return ret;
+
+ *val = FIELD_GET(BMA400_TAP_SEN_MSK, reg_val);
+ return IIO_VAL_INT;
+ case IIO_EV_INFO_RESET_TIMEOUT:
+ ret = regmap_read(data->regmap, BMA400_TAP_CONFIG1,
+ &reg_val);
+ if (ret)
+ return ret;
+
+ raw = FIELD_GET(BMA400_TAP_QUIET_MSK, reg_val);
+ *val = 0;
+ *val2 = tap_reset_timeout[raw];
+ return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_EV_INFO_TAP2_MIN_DELAY:
+ ret = regmap_read(data->regmap, BMA400_TAP_CONFIG1,
+ &reg_val);
+ if (ret)
+ return ret;
+
+ raw = FIELD_GET(BMA400_TAP_QUIETDT_MSK, reg_val);
+ *val = 0;
+ *val2 = double_tap2_min_delay[raw];
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
default:
return -EINVAL;
}
@@ -1209,10 +1467,13 @@ static int bma400_write_event_value(struct iio_dev *indio_dev,
int val, int val2)
{
struct bma400_data *data = iio_priv(indio_dev);
- int reg, ret;
+ int reg, ret, raw;

- switch (chan->type) {
- case IIO_ACCEL:
+ if (chan->type != IIO_ACCEL)
+ return -EINVAL;
+
+ switch (type) {
+ case IIO_EV_TYPE_MAG:
reg = get_gen_config_reg(dir);
if (reg < 0)
return -EINVAL;
@@ -1248,6 +1509,40 @@ static int bma400_write_event_value(struct iio_dev *indio_dev,
default:
return -EINVAL;
}
+ case IIO_EV_TYPE_GESTURE:
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ if (val < 0 || val > 7)
+ return -EINVAL;
+
+ return regmap_update_bits(data->regmap,
+ BMA400_TAP_CONFIG,
+ BMA400_TAP_SEN_MSK,
+ FIELD_PREP(BMA400_TAP_SEN_MSK,
+ val));
+ case IIO_EV_INFO_RESET_TIMEOUT:
+ raw = usec_to_tapreg_raw(val2, tap_reset_timeout);
+ if (raw < 0)
+ return -EINVAL;
+
+ return regmap_update_bits(data->regmap,
+ BMA400_TAP_CONFIG1,
+ BMA400_TAP_QUIET_MSK,
+ FIELD_PREP(BMA400_TAP_QUIET_MSK,
+ raw));
+ case IIO_EV_INFO_TAP2_MIN_DELAY:
+ raw = usec_to_tapreg_raw(val2, double_tap2_min_delay);
+ if (raw < 0)
+ return -EINVAL;
+
+ return regmap_update_bits(data->regmap,
+ BMA400_TAP_CONFIG1,
+ BMA400_TAP_QUIETDT_MSK,
+ FIELD_PREP(BMA400_TAP_QUIETDT_MSK,
+ raw));
+ default:
+ return -EINVAL;
+ }
default:
return -EINVAL;
}
@@ -1287,6 +1582,7 @@ static const struct iio_info bma400_info = {
.write_event_config = bma400_write_event_config,
.write_event_value = bma400_write_event_value,
.read_event_value = bma400_read_event_value,
+ .event_attrs = &bma400_event_attribute_group,
};

static const struct iio_trigger_ops bma400_trigger_ops = {
@@ -1350,6 +1646,32 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
if (ret || !data->status)
goto unlock_err;

+ /*
+ * Disable all advance interrupts if interrupt engine overrun occurs.
+ * See section 4.7 "Interrupt engine overrun" in datasheet v1.2.
+ */
+ if (FIELD_GET(BMA400_INT_ENG_OVRUN_MSK, le16_to_cpu(data->status))) {
+ bma400_disable_adv_interrupt(data);
+ dev_err(data->dev, "Interrupt engine overrun\n");
+ goto unlock_err;
+ }
+
+ if (FIELD_GET(BMA400_INT_S_TAP_MSK, le16_to_cpu(data->status)))
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+ IIO_MOD_X_OR_Y_OR_Z,
+ IIO_EV_TYPE_GESTURE,
+ IIO_EV_DIR_SINGLETAP),
+ timestamp);
+
+ if (FIELD_GET(BMA400_INT_D_TAP_MSK, le16_to_cpu(data->status)))
+ iio_push_event(indio_dev,
+ IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
+ IIO_MOD_X_OR_Y_OR_Z,
+ IIO_EV_TYPE_GESTURE,
+ IIO_EV_DIR_DOUBLETAP),
+ timestamp);
+
if (FIELD_GET(BMA400_INT_GEN1_MSK, le16_to_cpu(data->status)))
ev_dir = IIO_EV_DIR_RISING;

@@ -1467,5 +1789,6 @@ int bma400_probe(struct device *dev, struct regmap *regmap, int irq,
EXPORT_SYMBOL_NS(bma400_probe, IIO_BMA400);

MODULE_AUTHOR("Dan Robertson <[email protected]>");
+MODULE_AUTHOR("Jagath Jog J <[email protected]>");
MODULE_DESCRIPTION("Bosch BMA400 triaxial acceleration sensor core");
MODULE_LICENSE("GPL");
--
2.17.1

2022-08-25 20:23:42

by Jagath Jog J

[permalink] [raw]
Subject: [PATCH v3 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 | 69 +++++++++++++++++++++++++
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, 87 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index e81ba6f5e1c8..4035173030b7 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2038,3 +2038,72 @@ 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: 6.1
+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: 6.1
+Contact: [email protected]
+Description:
+ Specifies the threshold value that the device is comparing
+ against to generate the tap gesture event. The lower
+ threshold value increases the sensitivity of tap detection.
+ Units and the exact meaning of value are device-specific.
+
+What: /sys/.../events/in_accel_gesture_tap_value_available
+KernelVersion: 6.1
+Contact: [email protected]
+Description:
+ Lists all available threshold values which can be used to
+ modify the sensitivity of the tap detection.
+
+What: /sys/.../events/in_accel_gesture_singletap_reset_timeout
+What: /sys/.../events/in_accel_gesture_doubletap_reset_timeout
+KernelVersion: 6.1
+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
+ occurred. Basically the minimum quiet time between the two
+ single-tap's or two double-tap's.
+
+What: /sys/.../events/in_accel_gesture_tap_reset_timeout_available
+KernelVersion: 6.1
+Contact: [email protected]
+Description:
+ Lists all available tap reset timeout values. Units in seconds.
+
+What: /sys/.../events/in_accel_gesture_doubletap_tap2_min_delay
+KernelVersion: 6.1
+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_doubletap_tap2_min_delay_available
+KernelVersion: 6.1
+Contact: [email protected]
+Description:
+ Lists all available delay values between two taps in the double
+ tap. Units in seconds.
+
+What: /sys/.../events/in_accel_gesture_tap_maxtomin_time
+KernelVersion: 6.1
+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.
+
+What: /sys/.../events/in_accel_gesture_tap_maxtomin_time_available
+KernelVersion: 6.1
+Contact: [email protected]
+Description:
+ Lists all available time values between upper peak to lower
+ peak. Units in seconds.
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index b5e059e15b0a..65ef0fc3be23 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_TAP2_MIN_DELAY] = "tap2_min_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..4716a4b3d614 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_TAP2_MIN_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-25 20:27:25

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] iio: accel: bma400: Add support for single and double tap events

On Thu, Aug 25, 2022 at 10:46 PM Jagath Jog J <[email protected]> wrote:
>
> Add support for single and double tap events based on the tap threshold
> value, minimum quiet time before and after the tap and minimum time
> between the taps in the double tap. The INT1 pin is used to interrupt
> and the event is pushed to userspace.

...

> +static int tap_reset_timeout[] = {
> + 300000,
> + 400000,
> + 500000,
> + 600000

+ Comma and so on for the rest of the similar cases.

> +};

...

> +static int usec_to_tapreg_raw(int usec, const int *time_list)
> +{
> + int index;
> +
> + for (index = 0; index < 4; index++) {

Magic. Shouldn't be defined?

Also you may add it to each data structure in question.

> + if (usec == time_list[index])
> + return index;
> + }
> + return -EINVAL;
> +}

...

> + int ret;
> + unsigned int mask, field_value;

Reversed xmas tree order?

...

> + /*
> + * Tap interrupts are operating with the data rate of 200Hz.

a data

> + * See section 4.7 "Tap sensing interrupt" in datasheet v1.2.
> + */

--
With Best Regards,
Andy Shevchenko

2022-08-26 22:16:48

by Jagath Jog J

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] iio: accel: bma400: Add support for single and double tap events

Hi Andy,

On Fri, Aug 26, 2022 at 1:53 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Thu, Aug 25, 2022 at 10:46 PM Jagath Jog J <[email protected]> wrote:
> >
> > Add support for single and double tap events based on the tap threshold
> > value, minimum quiet time before and after the tap and minimum time
> > between the taps in the double tap. The INT1 pin is used to interrupt
> > and the event is pushed to userspace.
>
> ...
>
> > +static int tap_reset_timeout[] = {
> > + 300000,
> > + 400000,
> > + 500000,
> > + 600000
>
> + Comma and so on for the rest of the similar cases.

This is the terminator case so I have not added a comma in the last.
All three tap configurations have only 4 value options.

>
> > +};
>
> ...
>
> > +static int usec_to_tapreg_raw(int usec, const int *time_list)
> > +{
> > + int index;
> > +
> > + for (index = 0; index < 4; index++) {
>
> Magic. Shouldn't be defined?

All tap configuration value arrays are of size 4, I will define a
macro for that.

>
> Also you may add it to each data structure in question.

Do you mean storing these values in the device's private structure?

Tap configuration values are not stored in the device's private
structure because.
- I am directly accessing the device registers in _read_event_value()
and _write_event_value().
- These configuration values are not used in the other parts of
the driver.
- Two of these configurations have a default value so instead of
reading and storing these values in the device's private structure
during device init, I am directly accessing the device's register.

>
> > + if (usec == time_list[index])
> > + return index;
> > + }
> > + return -EINVAL;
> > +}
>
> ...
>
> > + int ret;
> > + unsigned int mask, field_value;
>
> Reversed xmas tree order?
>
> > + * Tap interrupts are operating with the data rate of 200Hz.
>
> a data

Sure, I will correct these in the next patch series.

Thank you
Jagath

>
> > + * See section 4.7 "Tap sensing interrupt" in datasheet v1.2.
> > + */
>
> --
> With Best Regards,
> Andy Shevchenko

2022-08-28 19:22:46

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] iio: accel: bma400: Add support for single and double tap events

On Sat, 27 Aug 2022 03:36:23 +0530
Jagath Jog J <[email protected]> wrote:

> Hi Andy,
>
> On Fri, Aug 26, 2022 at 1:53 AM Andy Shevchenko
> <[email protected]> wrote:
> >
> > On Thu, Aug 25, 2022 at 10:46 PM Jagath Jog J <[email protected]> wrote:
> > >
> > > Add support for single and double tap events based on the tap threshold
> > > value, minimum quiet time before and after the tap and minimum time
> > > between the taps in the double tap. The INT1 pin is used to interrupt
> > > and the event is pushed to userspace.
> >
> > ...
> >
> > > +static int tap_reset_timeout[] = {
> > > + 300000,
> > > + 400000,
> > > + 500000,
> > > + 600000
> >
> > + Comma and so on for the rest of the similar cases.
>
> This is the terminator case so I have not added a comma in the last.
> All three tap configurations have only 4 value options.
>
> >
> > > +};
> >
> > ...
> >
> > > +static int usec_to_tapreg_raw(int usec, const int *time_list)
> > > +{
> > > + int index;
> > > +
> > > + for (index = 0; index < 4; index++) {
> >
> > Magic. Shouldn't be defined?
>
> All tap configuration value arrays are of size 4, I will define a
> macro for that.
>
> >
> > Also you may add it to each data structure in question.
>
> Do you mean storing these values in the device's private structure?

I suspect Andy means making sure they are all 4 long via
+static int tap_reset_timeout[NEW_LENGTH_DEFINE] = {
etc.


>
> Tap configuration values are not stored in the device's private
> structure because.
> - I am directly accessing the device registers in _read_event_value()
> and _write_event_value().
> - These configuration values are not used in the other parts of
> the driver.
> - Two of these configurations have a default value so instead of
> reading and storing these values in the device's private structure
> during device init, I am directly accessing the device's register.
>
> >
> > > + if (usec == time_list[index])
> > > + return index;
> > > + }
> > > + return -EINVAL;
> > > +}
> >
> > ...
> >
> > > + int ret;
> > > + unsigned int mask, field_value;
> >
> > Reversed xmas tree order?
> >
> > > + * Tap interrupts are operating with the data rate of 200Hz.
> >
> > a data
>
> Sure, I will correct these in the next patch series.
>
> Thank you
> Jagath
>
> >
> > > + * See section 4.7 "Tap sensing interrupt" in datasheet v1.2.
> > > + */
> >
> > --
> > With Best Regards,
> > Andy Shevchenko

2022-08-29 17:49:32

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] iio: accel: bma400: Add support for single and double tap events

On Fri, 26 Aug 2022 01:16:04 +0530
Jagath Jog J <[email protected]> wrote:

> Add support for single and double tap events based on the tap threshold
> value, minimum quiet time before and after the tap and minimum time
> between the taps in the double tap. The INT1 pin is used to interrupt
> and the event is pushed to userspace.
>
> Signed-off-by: Jagath Jog J <[email protected]>

Nothing to add from me. I'll take a final look at v4 but assuming I've
not been missing anything until now should all be good to go once those
bits Andy raised are resolved subject to anyone else taking a look at
the ABI docs. Whilst I'd like review there, if we don't get it we can't
let that block this for ever so I aim to pick this up this cycle even
if we don't get input on that from anyone.

Thanks,

Jonathan

> ---
> drivers/iio/accel/bma400.h | 13 ++
> drivers/iio/accel/bma400_core.c | 343 +++++++++++++++++++++++++++++++-
> 2 files changed, 346 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/iio/accel/bma400.h b/drivers/iio/accel/bma400.h
> index e8f802a82300..d9e8eccb524b 100644
> --- a/drivers/iio/accel/bma400.h
> +++ b/drivers/iio/accel/bma400.h
> @@ -40,6 +40,7 @@
> #define BMA400_INT_STAT1_REG 0x0f
> #define BMA400_INT_STAT2_REG 0x10
> #define BMA400_INT12_MAP_REG 0x23
> +#define BMA400_INT_ENG_OVRUN_MSK BIT(4)
>
> /* Temperature register */
> #define BMA400_TEMP_DATA_REG 0x11
> @@ -105,6 +106,18 @@
> #define BMA400_INT_GEN2_MSK BIT(3)
> #define BMA400_GEN_HYST_MSK GENMASK(1, 0)
>
> +/* TAP config registers */
> +#define BMA400_TAP_CONFIG 0x57
> +#define BMA400_TAP_CONFIG1 0x58
> +#define BMA400_S_TAP_MSK BIT(2)
> +#define BMA400_D_TAP_MSK BIT(3)
> +#define BMA400_INT_S_TAP_MSK BIT(10)
> +#define BMA400_INT_D_TAP_MSK BIT(11)
> +#define BMA400_TAP_SEN_MSK GENMASK(2, 0)
> +#define BMA400_TAP_TICSTH_MSK GENMASK(1, 0)
> +#define BMA400_TAP_QUIET_MSK GENMASK(3, 2)
> +#define BMA400_TAP_QUIETDT_MSK GENMASK(5, 4)
> +
> /*
> * BMA400_SCALE_MIN macro value represents m/s^2 for 1 LSB before
> * converting to micro values for +-2g range.
> diff --git a/drivers/iio/accel/bma400_core.c b/drivers/iio/accel/bma400_core.c
> index c31bdd9b168e..1150778bda3a 100644
> --- a/drivers/iio/accel/bma400_core.c
> +++ b/drivers/iio/accel/bma400_core.c
> @@ -26,6 +26,7 @@
> #include <linux/iio/iio.h>
> #include <linux/iio/buffer.h>
> #include <linux/iio/events.h>
> +#include <linux/iio/sysfs.h>
> #include <linux/iio/trigger.h>
> #include <linux/iio/trigger_consumer.h>
> #include <linux/iio/triggered_buffer.h>
> @@ -47,6 +48,27 @@ static int bma400_sample_freqs[14];
>
> static const int bma400_osr_range[] = { 0, 1, 3 };
>
> +static int tap_reset_timeout[] = {
> + 300000,
> + 400000,
> + 500000,
> + 600000
> +};
> +
> +static int tap_max2min_time[] = {
> + 30000,
> + 45000,
> + 60000,
> + 90000
> +};
> +
> +static int double_tap2_min_delay[] = {
> + 20000,
> + 40000,
> + 60000,
> + 80000
> +};
> +
> /* See the ACC_CONFIG0 section of the datasheet */
> enum bma400_power_mode {
> POWER_MODE_SLEEP = 0x00,
> @@ -88,6 +110,7 @@ struct bma400_data {
> bool step_event_en;
> bool activity_event_en;
> unsigned int generic_event_en;
> + unsigned int tap_event_en_bitmask;
> /* Correct time stamp alignment */
> struct {
> __le16 buff[3];
> @@ -216,6 +239,115 @@ static const struct iio_event_spec bma400_accel_event[] = {
> BIT(IIO_EV_INFO_HYSTERESIS) |
> BIT(IIO_EV_INFO_ENABLE),
> },
> + {
> + .type = IIO_EV_TYPE_GESTURE,
> + .dir = IIO_EV_DIR_SINGLETAP,
> + .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE) |
> + BIT(IIO_EV_INFO_RESET_TIMEOUT),
> + },
> + {
> + .type = IIO_EV_TYPE_GESTURE,
> + .dir = IIO_EV_DIR_DOUBLETAP,
> + .mask_shared_by_type = BIT(IIO_EV_INFO_VALUE) |
> + BIT(IIO_EV_INFO_ENABLE) |
> + BIT(IIO_EV_INFO_RESET_TIMEOUT) |
> + BIT(IIO_EV_INFO_TAP2_MIN_DELAY),
> + },
> +};
> +
> +static int usec_to_tapreg_raw(int usec, const int *time_list)
> +{
> + int index;
> +
> + for (index = 0; index < 4; index++) {
> + if (usec == time_list[index])
> + return index;
> + }
> + return -EINVAL;
> +}
> +
> +static ssize_t in_accel_gesture_tap_maxtomin_time_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> + struct bma400_data *data = iio_priv(indio_dev);
> + int ret, reg_val, raw, vals[2];
> +
> + ret = regmap_read(data->regmap, BMA400_TAP_CONFIG1, &reg_val);
> + if (ret)
> + return ret;
> +
> + raw = FIELD_GET(BMA400_TAP_TICSTH_MSK, reg_val);
> + vals[0] = 0;
> + vals[1] = tap_max2min_time[raw];
> +
> + return iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, vals);
> +}
> +
> +static ssize_t in_accel_gesture_tap_maxtomin_time_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 bma400_data *data = iio_priv(indio_dev);
> + int ret, val_int, val_fract, raw;
> +
> + ret = iio_str_to_fixpoint(buf, 100000, &val_int, &val_fract);
> + if (ret)
> + return ret;
> +
> + raw = usec_to_tapreg_raw(val_fract, tap_max2min_time);
> + if (raw < 0)
> + return -EINVAL;
> +
> + ret = regmap_update_bits(data->regmap, BMA400_TAP_CONFIG1,
> + BMA400_TAP_TICSTH_MSK,
> + FIELD_PREP(BMA400_TAP_TICSTH_MSK, raw));
> + if (ret)
> + return ret;
> +
> + return len;
> +}
> +
> +static IIO_DEVICE_ATTR_RW(in_accel_gesture_tap_maxtomin_time, 0);
> +
> +/*
> + * Tap interrupts works with 200 Hz input data rate and the time based tap
> + * controls are in the terms of data samples so the below calculation is
> + * used to convert the configuration values into seconds.
> + * e.g.:
> + * 60 data samples * 0.005 ms = 0.3 seconds.
> + * 80 data samples * 0.005 ms = 0.4 seconds.
> + */
> +
> +/* quiet configuration values in seconds */
> +static IIO_CONST_ATTR(in_accel_gesture_tap_reset_timeout_available,
> + "0.3 0.4 0.5 0.6");
> +
> +/* tics_th configuration values in seconds */
> +static IIO_CONST_ATTR(in_accel_gesture_tap_maxtomin_time_available,
> + "0.03 0.045 0.06 0.09");
> +
> +/* quiet_dt configuration values in seconds */
> +static IIO_CONST_ATTR(in_accel_gesture_doubletap_tap2_min_delay_available,
> + "0.02 0.04 0.06 0.08");
> +
> +/* List of sensitivity values available to configure tap interrupts */
> +static IIO_CONST_ATTR(in_accel_gesture_tap_value_available, "0 1 2 3 4 5 6 7");
> +
> +static struct attribute *bma400_event_attributes[] = {
> + &iio_const_attr_in_accel_gesture_tap_value_available.dev_attr.attr,
> + &iio_const_attr_in_accel_gesture_tap_reset_timeout_available.dev_attr.attr,
> + &iio_const_attr_in_accel_gesture_tap_maxtomin_time_available.dev_attr.attr,
> + &iio_const_attr_in_accel_gesture_doubletap_tap2_min_delay_available.dev_attr.attr,
> + &iio_dev_attr_in_accel_gesture_tap_maxtomin_time.dev_attr.attr,
> + NULL
> +};
> +
> +static const struct attribute_group bma400_event_attribute_group = {
> + .attrs = bma400_event_attributes,
> };
>
> #define BMA400_ACC_CHANNEL(_index, _axis) { \
> @@ -1012,6 +1144,12 @@ static int bma400_read_event_config(struct iio_dev *indio_dev,
> case IIO_EV_DIR_FALLING:
> return FIELD_GET(BMA400_INT_GEN2_MSK,
> data->generic_event_en);
> + case IIO_EV_DIR_SINGLETAP:
> + return FIELD_GET(BMA400_S_TAP_MSK,
> + data->tap_event_en_bitmask);
> + case IIO_EV_DIR_DOUBLETAP:
> + return FIELD_GET(BMA400_D_TAP_MSK,
> + data->tap_event_en_bitmask);
> default:
> return -EINVAL;
> }
> @@ -1101,6 +1239,80 @@ static int bma400_activity_event_en(struct bma400_data *data,
> return 0;
> }
>
> +static int bma400_tap_event_en(struct bma400_data *data,
> + enum iio_event_direction dir, int state)
> +{
> + int ret;
> + unsigned int mask, field_value;
> +
> + /*
> + * Tap interrupts can be configured only in normal mode.
> + * See table in section 4.3 "Power modes - performance modes" of
> + * datasheet v1.2.
> + */
> + if (data->power_mode != POWER_MODE_NORMAL)
> + return -EINVAL;
> +
> + /*
> + * Tap interrupts are operating with the data rate of 200Hz.
> + * See section 4.7 "Tap sensing interrupt" in datasheet v1.2.
> + */
> + if (data->sample_freq.hz != 200 && state) {
> + dev_err(data->dev, "Invalid data rate for tap interrupts.\n");
> + return -EINVAL;
> + }
> +
> + ret = regmap_update_bits(data->regmap, BMA400_INT12_MAP_REG,
> + BMA400_S_TAP_MSK,
> + FIELD_PREP(BMA400_S_TAP_MSK, state));
> + if (ret)
> + return ret;
> +
> + switch (dir) {
> + case IIO_EV_DIR_SINGLETAP:
> + mask = BMA400_S_TAP_MSK;
> + set_mask_bits(&field_value, BMA400_S_TAP_MSK,
> + FIELD_PREP(BMA400_S_TAP_MSK, state));
> + break;
> + case IIO_EV_DIR_DOUBLETAP:
> + mask = BMA400_D_TAP_MSK;
> + set_mask_bits(&field_value, BMA400_D_TAP_MSK,
> + FIELD_PREP(BMA400_D_TAP_MSK, state));
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = regmap_update_bits(data->regmap, BMA400_INT_CONFIG1_REG, mask,
> + field_value);
> + if (ret)
> + return ret;
> +
> + set_mask_bits(&data->tap_event_en_bitmask, mask, field_value);
> +
> + return 0;
> +}
> +
> +static int bma400_disable_adv_interrupt(struct bma400_data *data)
> +{
> + int ret;
> +
> + ret = regmap_write(data->regmap, BMA400_INT_CONFIG0_REG, 0);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(data->regmap, BMA400_INT_CONFIG1_REG, 0);
> + if (ret)
> + return ret;
> +
> + data->tap_event_en_bitmask = 0;
> + data->generic_event_en = 0;
> + data->step_event_en = false;
> + data->activity_event_en = false;
> +
> + return 0;
> +}
> +
> static int bma400_write_event_config(struct iio_dev *indio_dev,
> const struct iio_chan_spec *chan,
> enum iio_event_type type,
> @@ -1111,10 +1323,20 @@ static int bma400_write_event_config(struct iio_dev *indio_dev,
>
> switch (chan->type) {
> case IIO_ACCEL:
> - mutex_lock(&data->mutex);
> - ret = bma400_activity_event_en(data, dir, state);
> - mutex_unlock(&data->mutex);
> - return ret;
> + switch (type) {
> + case IIO_EV_TYPE_MAG:
> + mutex_lock(&data->mutex);
> + ret = bma400_activity_event_en(data, dir, state);
> + mutex_unlock(&data->mutex);
> + return ret;
> + case IIO_EV_TYPE_GESTURE:
> + mutex_lock(&data->mutex);
> + ret = bma400_tap_event_en(data, dir, state);
> + mutex_unlock(&data->mutex);
> + return ret;
> + default:
> + return -EINVAL;
> + }
> case IIO_STEPS:
> mutex_lock(&data->mutex);
> ret = bma400_steps_event_enable(data, state);
> @@ -1157,10 +1379,13 @@ static int bma400_read_event_value(struct iio_dev *indio_dev,
> int *val, int *val2)
> {
> struct bma400_data *data = iio_priv(indio_dev);
> - int ret, reg;
> + int ret, reg, reg_val, raw;
>
> - switch (chan->type) {
> - case IIO_ACCEL:
> + if (chan->type != IIO_ACCEL)
> + return -EINVAL;
> +
> + switch (type) {
> + case IIO_EV_TYPE_MAG:
> reg = get_gen_config_reg(dir);
> if (reg < 0)
> return -EINVAL;
> @@ -1196,6 +1421,39 @@ static int bma400_read_event_value(struct iio_dev *indio_dev,
> default:
> return -EINVAL;
> }
> + case IIO_EV_TYPE_GESTURE:
> + switch (info) {
> + case IIO_EV_INFO_VALUE:
> + ret = regmap_read(data->regmap, BMA400_TAP_CONFIG,
> + &reg_val);
> + if (ret)
> + return ret;
> +
> + *val = FIELD_GET(BMA400_TAP_SEN_MSK, reg_val);
> + return IIO_VAL_INT;
> + case IIO_EV_INFO_RESET_TIMEOUT:
> + ret = regmap_read(data->regmap, BMA400_TAP_CONFIG1,
> + &reg_val);
> + if (ret)
> + return ret;
> +
> + raw = FIELD_GET(BMA400_TAP_QUIET_MSK, reg_val);
> + *val = 0;
> + *val2 = tap_reset_timeout[raw];
> + return IIO_VAL_INT_PLUS_MICRO;
> + case IIO_EV_INFO_TAP2_MIN_DELAY:
> + ret = regmap_read(data->regmap, BMA400_TAP_CONFIG1,
> + &reg_val);
> + if (ret)
> + return ret;
> +
> + raw = FIELD_GET(BMA400_TAP_QUIETDT_MSK, reg_val);
> + *val = 0;
> + *val2 = double_tap2_min_delay[raw];
> + return IIO_VAL_INT_PLUS_MICRO;
> + default:
> + return -EINVAL;
> + }
> default:
> return -EINVAL;
> }
> @@ -1209,10 +1467,13 @@ static int bma400_write_event_value(struct iio_dev *indio_dev,
> int val, int val2)
> {
> struct bma400_data *data = iio_priv(indio_dev);
> - int reg, ret;
> + int reg, ret, raw;
>
> - switch (chan->type) {
> - case IIO_ACCEL:
> + if (chan->type != IIO_ACCEL)
> + return -EINVAL;
> +
> + switch (type) {
> + case IIO_EV_TYPE_MAG:
> reg = get_gen_config_reg(dir);
> if (reg < 0)
> return -EINVAL;
> @@ -1248,6 +1509,40 @@ static int bma400_write_event_value(struct iio_dev *indio_dev,
> default:
> return -EINVAL;
> }
> + case IIO_EV_TYPE_GESTURE:
> + switch (info) {
> + case IIO_EV_INFO_VALUE:
> + if (val < 0 || val > 7)
> + return -EINVAL;
> +
> + return regmap_update_bits(data->regmap,
> + BMA400_TAP_CONFIG,
> + BMA400_TAP_SEN_MSK,
> + FIELD_PREP(BMA400_TAP_SEN_MSK,
> + val));
> + case IIO_EV_INFO_RESET_TIMEOUT:
> + raw = usec_to_tapreg_raw(val2, tap_reset_timeout);
> + if (raw < 0)
> + return -EINVAL;
> +
> + return regmap_update_bits(data->regmap,
> + BMA400_TAP_CONFIG1,
> + BMA400_TAP_QUIET_MSK,
> + FIELD_PREP(BMA400_TAP_QUIET_MSK,
> + raw));
> + case IIO_EV_INFO_TAP2_MIN_DELAY:
> + raw = usec_to_tapreg_raw(val2, double_tap2_min_delay);
> + if (raw < 0)
> + return -EINVAL;
> +
> + return regmap_update_bits(data->regmap,
> + BMA400_TAP_CONFIG1,
> + BMA400_TAP_QUIETDT_MSK,
> + FIELD_PREP(BMA400_TAP_QUIETDT_MSK,
> + raw));
> + default:
> + return -EINVAL;
> + }
> default:
> return -EINVAL;
> }
> @@ -1287,6 +1582,7 @@ static const struct iio_info bma400_info = {
> .write_event_config = bma400_write_event_config,
> .write_event_value = bma400_write_event_value,
> .read_event_value = bma400_read_event_value,
> + .event_attrs = &bma400_event_attribute_group,
> };
>
> static const struct iio_trigger_ops bma400_trigger_ops = {
> @@ -1350,6 +1646,32 @@ static irqreturn_t bma400_interrupt(int irq, void *private)
> if (ret || !data->status)
> goto unlock_err;
>
> + /*
> + * Disable all advance interrupts if interrupt engine overrun occurs.
> + * See section 4.7 "Interrupt engine overrun" in datasheet v1.2.
> + */
> + if (FIELD_GET(BMA400_INT_ENG_OVRUN_MSK, le16_to_cpu(data->status))) {
> + bma400_disable_adv_interrupt(data);
> + dev_err(data->dev, "Interrupt engine overrun\n");
> + goto unlock_err;
> + }
> +
> + if (FIELD_GET(BMA400_INT_S_TAP_MSK, le16_to_cpu(data->status)))
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> + IIO_MOD_X_OR_Y_OR_Z,
> + IIO_EV_TYPE_GESTURE,
> + IIO_EV_DIR_SINGLETAP),
> + timestamp);
> +
> + if (FIELD_GET(BMA400_INT_D_TAP_MSK, le16_to_cpu(data->status)))
> + iio_push_event(indio_dev,
> + IIO_MOD_EVENT_CODE(IIO_ACCEL, 0,
> + IIO_MOD_X_OR_Y_OR_Z,
> + IIO_EV_TYPE_GESTURE,
> + IIO_EV_DIR_DOUBLETAP),
> + timestamp);
> +
> if (FIELD_GET(BMA400_INT_GEN1_MSK, le16_to_cpu(data->status)))
> ev_dir = IIO_EV_DIR_RISING;
>
> @@ -1467,5 +1789,6 @@ int bma400_probe(struct device *dev, struct regmap *regmap, int irq,
> EXPORT_SYMBOL_NS(bma400_probe, IIO_BMA400);
>
> MODULE_AUTHOR("Dan Robertson <[email protected]>");
> +MODULE_AUTHOR("Jagath Jog J <[email protected]>");
> MODULE_DESCRIPTION("Bosch BMA400 triaxial acceleration sensor core");
> MODULE_LICENSE("GPL");

2022-08-29 18:36:12

by Jonathan Cameron

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

On Fri, 26 Aug 2022 01:16:03 +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]>
LGTM though I'm still keen for more review on this, particularly with an
eye on generality.

J
> ---
> Documentation/ABI/testing/sysfs-bus-iio | 69 +++++++++++++++++++++++++
> 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, 87 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index e81ba6f5e1c8..4035173030b7 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -2038,3 +2038,72 @@ 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: 6.1
> +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: 6.1
> +Contact: [email protected]
> +Description:
> + Specifies the threshold value that the device is comparing
> + against to generate the tap gesture event. The lower
> + threshold value increases the sensitivity of tap detection.
> + Units and the exact meaning of value are device-specific.
> +
> +What: /sys/.../events/in_accel_gesture_tap_value_available
> +KernelVersion: 6.1
> +Contact: [email protected]
> +Description:
> + Lists all available threshold values which can be used to
> + modify the sensitivity of the tap detection.
> +
> +What: /sys/.../events/in_accel_gesture_singletap_reset_timeout
> +What: /sys/.../events/in_accel_gesture_doubletap_reset_timeout
> +KernelVersion: 6.1
> +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
> + occurred. Basically the minimum quiet time between the two
> + single-tap's or two double-tap's.
> +
> +What: /sys/.../events/in_accel_gesture_tap_reset_timeout_available
> +KernelVersion: 6.1
> +Contact: [email protected]
> +Description:
> + Lists all available tap reset timeout values. Units in seconds.
> +
> +What: /sys/.../events/in_accel_gesture_doubletap_tap2_min_delay
> +KernelVersion: 6.1
> +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_doubletap_tap2_min_delay_available
> +KernelVersion: 6.1
> +Contact: [email protected]
> +Description:
> + Lists all available delay values between two taps in the double
> + tap. Units in seconds.
> +
> +What: /sys/.../events/in_accel_gesture_tap_maxtomin_time
> +KernelVersion: 6.1
> +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.
> +
> +What: /sys/.../events/in_accel_gesture_tap_maxtomin_time_available
> +KernelVersion: 6.1
> +Contact: [email protected]
> +Description:
> + Lists all available time values between upper peak to lower
> + peak. Units in seconds.
> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
> index b5e059e15b0a..65ef0fc3be23 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_TAP2_MIN_DELAY] = "tap2_min_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..4716a4b3d614 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_TAP2_MIN_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: