[PATCH v2 0/2] iio: Add single and double tap events support

[PATCH v2 0/2] iio: Add single and double tap events support

[Jagath Jog J]

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 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 |  41 +++
 drivers/iio/accel/bma400.h              |  13 +
 drivers/iio/accel/bma400_core.c         | 337 +++++++++++++++++++++++-
 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, 399 insertions(+), 12 deletions(-)


base-commit: 180c6cb6b9b79c55b79e8414f4c0208f2463af7d
-- 
2.17.1

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

[Jagath Jog J]

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 <jagathjog1996@gmail.com>
---
 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:	linux-iio@vger.kernel.org
+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:	linux-iio@vger.kernel.org
+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:	linux-iio@vger.kernel.org
+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:	linux-iio@vger.kernel.org
+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:	linux-iio@vger.kernel.org
+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

[PATCH v2 2/2] iio: accel: bma400: Add support for single and double tap events

[Jagath Jog J]

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 <jagathjog1996@gmail.com>
---
 drivers/iio/accel/bma400.h      |  13 ++
 drivers/iio/accel/bma400_core.c | 337 +++++++++++++++++++++++++++++++-
 2 files changed, 340 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..eb995acdd45a 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_timeout[] = {
+	300000,
+	400000,
+	500000,
+	600000
+};
+
+static int tap_max2min_time[] = {
+	30000,
+	45000,
+	60000,
+	90000
+};
+
+static int doubletap_2min_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_TAP_2MIN_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_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_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_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_reset_timeout_available,
+		      "0.3 0.4 0.5 0.6");
+
+/* tics_th configuration values in seconds */
+static IIO_CONST_ATTR(in_accel_gesture_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_2mindelay_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_value_available, "0 1 2 3 4 5 6 7");
+
+static struct attribute *bma400_event_attributes[] = {
+	&iio_const_attr_in_accel_gesture_value_available.dev_attr.attr,
+	&iio_const_attr_in_accel_gesture_reset_timeout_available.dev_attr.attr,
+	&iio_const_attr_in_accel_gesture_maxtomin_time_available.dev_attr.attr,
+	&iio_const_attr_in_accel_gesture_doubletap_2mindelay_available.dev_attr.attr,
+	&iio_dev_attr_in_accel_gesture_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,10 @@ 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:
+	switch (type) {
+	case IIO_EV_TYPE_MAG:
 		reg = get_gen_config_reg(dir);
 		if (reg < 0)
 			return -EINVAL;
@@ -1196,6 +1418,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_timeout[raw];
+			return IIO_VAL_INT_PLUS_MICRO;
+		case IIO_EV_INFO_TAP_2MIN_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 = doubletap_2min_delay[raw];
+			return IIO_VAL_INT_PLUS_MICRO;
+		default:
+			return -EINVAL;
+		}
 	default:
 		return -EINVAL;
 	}
@@ -1209,10 +1464,10 @@ 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:
+	switch (type) {
+	case IIO_EV_TYPE_MAG:
 		reg = get_gen_config_reg(dir);
 		if (reg < 0)
 			return -EINVAL;
@@ -1248,6 +1503,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_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_TAP_2MIN_DELAY:
+			raw = usec_to_tapreg_raw(val2, doubletap_2min_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 +1576,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 +1640,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 +1783,6 @@ int bma400_probe(struct device *dev, struct regmap *regmap, int irq,
 EXPORT_SYMBOL_NS(bma400_probe, IIO_BMA400);
 
 MODULE_AUTHOR("Dan Robertson <dan@dlrobertson.com>");
+MODULE_AUTHOR("Jagath Jog J <jagathjog1996@gmail.com>");
 MODULE_DESCRIPTION("Bosch BMA400 triaxial acceleration sensor core");
 MODULE_LICENSE("GPL");
-- 
2.17.1

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

[Jonathan Cameron]

On Sat, 13 Aug 2022 12:48:02 +0530
Jagath Jog J <jagathjog1996@gmail.com> 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 <jagathjog1996@gmail.com>
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:	linux-iio@vger.kernel.org
> +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:	linux-iio@vger.kernel.org
> +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:	linux-iio@vger.kernel.org
> +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:	linux-iio@vger.kernel.org
> +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:	linux-iio@vger.kernel.org
> +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.

Re: [PATCH v2 2/2] iio: accel: bma400: Add support for single and double tap events

[Jonathan Cameron]

On Sat, 13 Aug 2022 12:48:03 +0530
Jagath Jog J <jagathjog1996@gmail.com> 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 <jagathjog1996@gmail.com>

Hi Jagath,

As, for this series, the difficult bit from review point of view is the ABI
I've only taken a quick look at the implementation.  Basically looks fine to me,
but one trivial thing below (and that might just be me failing to read a diff)

...
>  		ret = bma400_steps_event_enable(data, state);
> @@ -1157,10 +1379,10 @@ 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:

We've lost the check on ACCEL I think.  Might be worth putting
it back as
	if (chan->type != ACCEL)
		return -EINVAL;

> +	switch (type) {
> +	case IIO_EV_TYPE_MAG:
>  		reg = get_gen_config_reg(dir);
>  		if (reg < 0)
>  			return -EINVAL;
> @@ -1196,6 +1418,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_timeout[raw];
> +			return IIO_VAL_INT_PLUS_MICRO;
> +		case IIO_EV_INFO_TAP_2MIN_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 = doubletap_2min_delay[raw];
> +			return IIO_VAL_INT_PLUS_MICRO;
> +		default:
> +			return -EINVAL;
> +		}
>  	default:
>  		return -EINVAL;
>  	}

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

[Jagath Jog J]

Hi Jonathan,

On Sun, Aug 14, 2022 at 10:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sat, 13 Aug 2022 12:48:02 +0530
> Jagath Jog J <jagathjog1996@gmail.com> 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 <jagathjog1996@gmail.com>
> 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:     linux-iio@vger.kernel.org
> > +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:     linux-iio@vger.kernel.org
> > +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:     linux-iio@vger.kernel.org
> > +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:     linux-iio@vger.kernel.org
> > +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:     linux-iio@vger.kernel.org
> > +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.
>
>

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

[Andy Shevchenko]

On Mon, Aug 15, 2022 at 9:33 PM Jagath Jog J <jagathjog1996@gmail.com> wrote:
> On Sun, Aug 14, 2022 at 10:24 PM Jonathan Cameron <jic23@kernel.org> 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

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

[Jonathan Cameron]

On Tue, 16 Aug 2022 00:03:47 +0530
Jagath Jog J <jagathjog1996@gmail.com> wrote:

> Hi Jonathan,
> 
> On Sun, Aug 14, 2022 at 10:24 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Sat, 13 Aug 2022 12:48:02 +0530
> > Jagath Jog J <jagathjog1996@gmail.com> 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 <jagathjog1996@gmail.com>  
> > 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:     linux-iio@vger.kernel.org
> > > +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:     linux-iio@vger.kernel.org
> > > +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:     linux-iio@vger.kernel.org
> > > +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:     linux-iio@vger.kernel.org
> > > +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:     linux-iio@vger.kernel.org
> > > +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.
> >
> >