motion: remove load_fifo

To prevent invalid timestamping, call load_fifo only when we get a FIFO
interrupt.
In consequence, remove load_fifo entry point and only process fifo
inside the IRQ.

Add helper function to know when we are in forced mode (the EC needs to
periodically read sensor data or interrupt driven).

BUG=b:73557414
BRANCH=master
TEST=compile

Change-Id: I959e476f3f7215be95424c07223f7421e8b13da1
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/938146
Commit-Ready: Alexandru M Stan <amstan@chromium.org>
Tested-by: Alexandru M Stan <amstan@chromium.org>
Reviewed-by: Alexandru M Stan <amstan@chromium.org>

4 files changed, 180 insertions, 173 deletions

diff --git a/driver/accel_lis2dh.c b/driver/accel_lis2dh.c
index 18a0ade444..6cb2ff87ce 100644
--- a/driver/accel_lis2dh.c
+++ b/driver/accel_lis2dh.c
@@ -398,9 +398,6 @@ const struct accelgyro_drv lis2dh_drv = {
 	.set_offset = st_set_offset,
 	.get_offset = st_get_offset,
 	.perform_calib = NULL,
-#ifdef CONFIG_ACCEL_FIFO
-	.load_fifo = load_fifo,
-#endif /* CONFIG_ACCEL_FIFO */
 #ifdef CONFIG_ACCEL_INTERRUPTS
 	.irq_handler = irq_handler,
 #endif /* CONFIG_ACCEL_INTERRUPTS */
diff --git a/driver/accelgyro_bmi160.c b/driver/accelgyro_bmi160.c
index 8c9a8e4af4..384357bf29 100644
--- a/driver/accelgyro_bmi160.c
+++ b/driver/accelgyro_bmi160.c
@@ -766,169 +766,6 @@ int list_activities(const struct motion_sensor_t *s,
 #endif
 
 #ifdef CONFIG_ACCEL_INTERRUPTS
-/**
- * bmi160_interrupt - called when the sensor activates the interrupt line.
- *
- * This is a "top half" interrupt handler, it just asks motion sense ask
- * to schedule the "bottom half", ->irq_handler().
- */
-void bmi160_interrupt(enum gpio_signal signal)
-{
-	task_set_event(TASK_ID_MOTIONSENSE,
-		       CONFIG_ACCELGYRO_BMI160_INT_EVENT, 0);
-}
-
-
-static int config_interrupt(const struct motion_sensor_t *s)
-{
-	int ret, tmp;
-
-	if (s->type != MOTIONSENSE_TYPE_ACCEL)
-		return EC_SUCCESS;
-
-	mutex_lock(s->mutex);
-	raw_write8(s->port, s->addr, BMI160_CMD_REG, BMI160_CMD_FIFO_FLUSH);
-	raw_write8(s->port, s->addr, BMI160_CMD_REG, BMI160_CMD_INT_RESET);
-
-#ifdef CONFIG_GESTURE_SENSOR_BATTERY_TAP
-	raw_write8(s->port, s->addr, BMI160_INT_TAP_0,
-		BMI160_TAP_DUR(s, CONFIG_GESTURE_TAP_MAX_INTERSTICE_T));
-	ret = raw_write8(s->port, s->addr, BMI160_INT_TAP_1,
-		BMI160_TAP_TH(s, CONFIG_GESTURE_TAP_THRES_MG));
-#endif
-#ifdef CONFIG_BMI160_ORIENTATION_SENSOR
-	/* only use orientation sensor on the lid sensor */
-	if (s->location == MOTIONSENSE_LOC_LID) {
-		ret = raw_write8(s->port, s->addr, BMI160_INT_ORIENT_0,
-			BMI160_INT_ORIENT_0_INIT_VAL);
-		ret = raw_write8(s->port, s->addr, BMI160_INT_ORIENT_1,
-			BMI160_INT_ORIENT_1_INIT_VAL);
-	}
-#endif
-
-	/*
-	 * Set a 5ms latch to be sure the EC can read the interrupt register
-	 * properly, even when it is running more slowly.
-	 */
-#ifdef CONFIG_ACCELGYRO_BMI160_INT2_OUTPUT
-	ret = raw_write8(s->port, s->addr, BMI160_INT_LATCH, BMI160_LATCH_5MS);
-#else
-	/* Also, configure int2 as an external input. */
-	ret = raw_write8(s->port, s->addr, BMI160_INT_LATCH,
-		BMI160_INT2_INPUT_EN | BMI160_LATCH_5MS);
-#endif
-
-	/* configure int1 as an interrupt */
-	ret = raw_write8(s->port, s->addr, BMI160_INT_OUT_CTRL,
-		BMI160_INT_CTRL(1, OUTPUT_EN));
-
-	/* Map activity interrupt to int 1 */
-	tmp = 0;
-#ifdef CONFIG_GESTURE_SIGMO
-	tmp |= BMI160_INT_ANYMOTION;
-#endif
-#ifdef CONFIG_GESTURE_SENSOR_BATTERY_TAP
-	tmp |= BMI160_INT_D_TAP;
-#endif
-#ifdef CONFIG_BMI160_ORIENTATION_SENSOR
-	/* enable orientation interrupt for lid sensor only */
-	if (s->location == MOTIONSENSE_LOC_LID)
-		tmp |= BMI160_INT_ORIENT;
-#endif
-	ret = raw_write8(s->port, s->addr, BMI160_INT_MAP_REG(1), tmp);
-
-#ifdef CONFIG_ACCEL_FIFO
-	/* map fifo water mark to int 1 */
-	ret = raw_write8(s->port, s->addr, BMI160_INT_FIFO_MAP,
-			BMI160_INT_MAP(1, FWM) |
-			BMI160_INT_MAP(1, FFULL));
-
-	/* configure fifo watermark to int whenever there's any data in there */
-	ret = raw_write8(s->port, s->addr, BMI160_FIFO_CONFIG_0, 1);
-#ifdef CONFIG_ACCELGYRO_BMI160_INT2_OUTPUT
-	ret = raw_write8(s->port, s->addr, BMI160_FIFO_CONFIG_1,
-			BMI160_FIFO_HEADER_EN);
-#else
-	ret = raw_write8(s->port, s->addr, BMI160_FIFO_CONFIG_1,
-			BMI160_FIFO_TAG_INT2_EN |
-			BMI160_FIFO_HEADER_EN);
-#endif
-
-	/* Set fifo*/
-	ret = raw_read8(s->port, s->addr, BMI160_INT_EN_1, &tmp);
-	tmp |= BMI160_INT_FWM_EN | BMI160_INT_FFUL_EN;
-	ret = raw_write8(s->port, s->addr, BMI160_INT_EN_1, tmp);
-#endif
-	mutex_unlock(s->mutex);
-	return ret;
-}
-
-/**
- * irq_handler - bottom half of the interrupt stack.
- * Ran from the motion_sense task, finds the events that raised the interrupt.
- *
- * For now, we just print out. We should set a bitmask motion sense code will
- * act upon.
- */
-static int irq_handler(struct motion_sensor_t *s, uint32_t *event)
-{
-	int interrupt;
-#ifdef CONFIG_BMI160_ORIENTATION_SENSOR
-	int shifted_masked_orientation;
-#endif
-
-	if ((s->type != MOTIONSENSE_TYPE_ACCEL) ||
-	    (!(*event & CONFIG_ACCELGYRO_BMI160_INT_EVENT)))
-		return EC_ERROR_NOT_HANDLED;
-
-	raw_read32(s->port, s->addr, BMI160_INT_STATUS_0, &interrupt);
-
-#ifdef CONFIG_GESTURE_SENSOR_BATTERY_TAP
-	if (interrupt & BMI160_D_TAP_INT)
-		*event |= CONFIG_GESTURE_TAP_EVENT;
-#endif
-#ifdef CONFIG_GESTURE_SIGMO
-	if (interrupt & BMI160_SIGMOT_INT)
-		*event |= CONFIG_GESTURE_SIGMO_EVENT;
-#endif
-#ifdef CONFIG_BMI160_ORIENTATION_SENSOR
-	shifted_masked_orientation = (interrupt >> 24) & BMI160_ORIENT_XY_MASK;
-	if (BMI160_GET_DATA(s)->raw_orientation != shifted_masked_orientation) {
-		enum motionsensor_orientation orientation =
-				MOTIONSENSE_ORIENTATION_UNKNOWN;
-
-		BMI160_GET_DATA(s)->raw_orientation =
-				shifted_masked_orientation;
-
-		switch (shifted_masked_orientation) {
-		case BMI160_ORIENT_PORTRAIT:
-			orientation = MOTIONSENSE_ORIENTATION_PORTRAIT;
-			break;
-		case BMI160_ORIENT_PORTRAIT_INVERT:
-			orientation =
-				MOTIONSENSE_ORIENTATION_UPSIDE_DOWN_PORTRAIT;
-			break;
-		case BMI160_ORIENT_LANDSCAPE:
-			orientation = MOTIONSENSE_ORIENTATION_LANDSCAPE;
-			break;
-		case BMI160_ORIENT_LANDSCAPE_INVERT:
-			orientation =
-				MOTIONSENSE_ORIENTATION_UPSIDE_DOWN_LANDSCAPE;
-			break;
-		default:
-			break;
-		}
-		orientation = motion_sense_remap_orientation(s, orientation);
-		SET_ORIENTATION(s, orientation);
-	}
-#endif
-	/*
-	 * No need to read the FIFO here, motion sense task is
-	 * doing it on every interrupt.
-	 */
-	return EC_SUCCESS;
-}
-#endif  /* CONFIG_ACCEL_INTERRUPTS */
 
 #ifdef CONFIG_ACCEL_FIFO
 enum fifo_state {
@@ -993,6 +830,17 @@ static int bmi160_decode_header(struct motion_sensor_t *s,
 	}
 }
 
+/**
+ * Retrieve hardware FIFO from sensor,
+ * - put data in Sensor Hub fifo.
+ * - update sensor raw_xyz vector with the last information.
+ * We put raw data in hub fifo and process data from there.
+ * @s Pointer to sensor data.
+ *
+ * NOTE: If a new driver supports this function, be sure to add a check
+ * for spoof_mode in order to load the sensor stack with the spoofed
+ * data.  See accelgyro_bmi160.c::load_fifo for an example.
+ */
 static int load_fifo(struct motion_sensor_t *s)
 {
 	int done = 0;
@@ -1099,6 +947,174 @@ static int load_fifo(struct motion_sensor_t *s)
 }
 #endif  /* CONFIG_ACCEL_FIFO */
 
+/**
+ * bmi160_interrupt - called when the sensor activates the interrupt line.
+ *
+ * This is a "top half" interrupt handler, it just asks motion sense ask
+ * to schedule the "bottom half", ->irq_handler().
+ */
+void bmi160_interrupt(enum gpio_signal signal)
+{
+	task_set_event(TASK_ID_MOTIONSENSE,
+		       CONFIG_ACCELGYRO_BMI160_INT_EVENT, 0);
+}
+
+
+static int config_interrupt(const struct motion_sensor_t *s)
+{
+	int ret, tmp;
+
+	if (s->type != MOTIONSENSE_TYPE_ACCEL)
+		return EC_SUCCESS;
+
+	mutex_lock(s->mutex);
+	raw_write8(s->port, s->addr, BMI160_CMD_REG, BMI160_CMD_FIFO_FLUSH);
+	raw_write8(s->port, s->addr, BMI160_CMD_REG, BMI160_CMD_INT_RESET);
+
+#ifdef CONFIG_GESTURE_SENSOR_BATTERY_TAP
+	raw_write8(s->port, s->addr, BMI160_INT_TAP_0,
+		BMI160_TAP_DUR(s, CONFIG_GESTURE_TAP_MAX_INTERSTICE_T));
+	ret = raw_write8(s->port, s->addr, BMI160_INT_TAP_1,
+		BMI160_TAP_TH(s, CONFIG_GESTURE_TAP_THRES_MG));
+#endif
+#ifdef CONFIG_BMI160_ORIENTATION_SENSOR
+	/* only use orientation sensor on the lid sensor */
+	if (s->location == MOTIONSENSE_LOC_LID) {
+		ret = raw_write8(s->port, s->addr, BMI160_INT_ORIENT_0,
+			BMI160_INT_ORIENT_0_INIT_VAL);
+		ret = raw_write8(s->port, s->addr, BMI160_INT_ORIENT_1,
+			BMI160_INT_ORIENT_1_INIT_VAL);
+	}
+#endif
+
+	/*
+	 * Set a 5ms latch to be sure the EC can read the interrupt register
+	 * properly, even when it is running more slowly.
+	 */
+#ifdef CONFIG_ACCELGYRO_BMI160_INT2_OUTPUT
+	ret = raw_write8(s->port, s->addr, BMI160_INT_LATCH, BMI160_LATCH_5MS);
+#else
+	/* Also, configure int2 as an external input. */
+	ret = raw_write8(s->port, s->addr, BMI160_INT_LATCH,
+		BMI160_INT2_INPUT_EN | BMI160_LATCH_5MS);
+#endif
+
+	/* configure int1 as an interrupt */
+	ret = raw_write8(s->port, s->addr, BMI160_INT_OUT_CTRL,
+		BMI160_INT_CTRL(1, OUTPUT_EN));
+
+	/* Map activity interrupt to int 1 */
+	tmp = 0;
+#ifdef CONFIG_GESTURE_SIGMO
+	tmp |= BMI160_INT_ANYMOTION;
+#endif
+#ifdef CONFIG_GESTURE_SENSOR_BATTERY_TAP
+	tmp |= BMI160_INT_D_TAP;
+#endif
+#ifdef CONFIG_BMI160_ORIENTATION_SENSOR
+	/* enable orientation interrupt for lid sensor only */
+	if (s->location == MOTIONSENSE_LOC_LID)
+		tmp |= BMI160_INT_ORIENT;
+#endif
+	ret = raw_write8(s->port, s->addr, BMI160_INT_MAP_REG(1), tmp);
+
+#ifdef CONFIG_ACCEL_FIFO
+	/* map fifo water mark to int 1 */
+	ret = raw_write8(s->port, s->addr, BMI160_INT_FIFO_MAP,
+			BMI160_INT_MAP(1, FWM) |
+			BMI160_INT_MAP(1, FFULL));
+
+	/* configure fifo watermark to int whenever there's any data in there */
+	ret = raw_write8(s->port, s->addr, BMI160_FIFO_CONFIG_0, 1);
+#ifdef CONFIG_ACCELGYRO_BMI160_INT2_OUTPUT
+	ret = raw_write8(s->port, s->addr, BMI160_FIFO_CONFIG_1,
+			BMI160_FIFO_HEADER_EN);
+#else
+	ret = raw_write8(s->port, s->addr, BMI160_FIFO_CONFIG_1,
+			BMI160_FIFO_TAG_INT2_EN |
+			BMI160_FIFO_HEADER_EN);
+#endif
+
+	/* Set fifo*/
+	ret = raw_read8(s->port, s->addr, BMI160_INT_EN_1, &tmp);
+	tmp |= BMI160_INT_FWM_EN | BMI160_INT_FFUL_EN;
+	ret = raw_write8(s->port, s->addr, BMI160_INT_EN_1, tmp);
+#endif
+	mutex_unlock(s->mutex);
+	return ret;
+}
+
+/**
+ * irq_handler - bottom half of the interrupt stack.
+ * Ran from the motion_sense task, finds the events that raised the interrupt.
+ *
+ * For now, we just print out. We should set a bitmask motion sense code will
+ * act upon.
+ */
+static int irq_handler(struct motion_sensor_t *s, uint32_t *event)
+{
+	int interrupt;
+#ifdef CONFIG_BMI160_ORIENTATION_SENSOR
+	int shifted_masked_orientation;
+#endif
+
+	if ((s->type != MOTIONSENSE_TYPE_ACCEL) ||
+			(!(*event & CONFIG_ACCELGYRO_BMI160_INT_EVENT)))
+		return EC_ERROR_NOT_HANDLED;
+
+	raw_read32(s->port, s->addr, BMI160_INT_STATUS_0, &interrupt);
+
+#ifdef CONFIG_GESTURE_SENSOR_BATTERY_TAP
+	if (interrupt & BMI160_D_TAP_INT)
+		*event |= CONFIG_GESTURE_TAP_EVENT;
+#endif
+#ifdef CONFIG_GESTURE_SIGMO
+	if (interrupt & BMI160_SIGMOT_INT)
+		*event |= CONFIG_GESTURE_SIGMO_EVENT;
+#endif
+#ifdef CONFIG_ACCEL_FIFO
+	if (interrupt & (BMI160_FWM_INT | BMI160_FFULL_INT))
+		load_fifo(s);
+#endif
+#ifdef CONFIG_BMI160_ORIENTATION_SENSOR
+	shifted_masked_orientation = (interrupt >> 24) & BMI160_ORIENT_XY_MASK;
+	if (BMI160_GET_DATA(s)->raw_orientation != shifted_masked_orientation) {
+		enum motionsensor_orientation orientation =
+				MOTIONSENSE_ORIENTATION_UNKNOWN;
+
+		BMI160_GET_DATA(s)->raw_orientation =
+				shifted_masked_orientation;
+
+		switch (shifted_masked_orientation) {
+		case BMI160_ORIENT_PORTRAIT:
+			orientation = MOTIONSENSE_ORIENTATION_PORTRAIT;
+			break;
+		case BMI160_ORIENT_PORTRAIT_INVERT:
+			orientation =
+				MOTIONSENSE_ORIENTATION_UPSIDE_DOWN_PORTRAIT;
+			break;
+		case BMI160_ORIENT_LANDSCAPE:
+			orientation = MOTIONSENSE_ORIENTATION_LANDSCAPE;
+			break;
+		case BMI160_ORIENT_LANDSCAPE_INVERT:
+			orientation =
+				MOTIONSENSE_ORIENTATION_UPSIDE_DOWN_LANDSCAPE;
+			break;
+		default:
+			break;
+		}
+		orientation = motion_sense_remap_orientation(s, orientation);
+		SET_ORIENTATION(s, orientation);
+	}
+#endif
+	/*
+	 * No need to read the FIFO here, motion sense task is
+	 * doing it on every interrupt.
+	 */
+	return EC_SUCCESS;
+}
+#endif  /* CONFIG_ACCEL_INTERRUPTS */
+
 
 static int read(const struct motion_sensor_t *s, vector_3_t v)
 {
@@ -1293,9 +1309,6 @@ const struct accelgyro_drv bmi160_drv = {
 #ifdef CONFIG_ACCEL_INTERRUPTS
 	.irq_handler = irq_handler,
 #endif
-#ifdef CONFIG_ACCEL_FIFO
-	.load_fifo = load_fifo,
-#endif
 #ifdef CONFIG_GESTURE_HOST_DETECTION
 	.manage_activity = manage_activity,
 	.list_activities = list_activities,
diff --git a/driver/accelgyro_lsm6ds0.c b/driver/accelgyro_lsm6ds0.c
index 67f3fffd85..86232f93e6 100644
--- a/driver/accelgyro_lsm6ds0.c
+++ b/driver/accelgyro_lsm6ds0.c
@@ -363,7 +363,7 @@ static int read(const struct motion_sensor_t *s, vector_3_t v)
 	}
 
 	for (i = X; i <= Z; i++)
-		v[i] = ((int16_t)((raw[i * 2 + 1] << 8) | raw[i * 2]));
+		v[i] = (int16_t)((raw[i * 2 + 1] << 8) | raw[i * 2]);
 
 	rotate(v, *s->rot_standard_ref, v);
 
diff --git a/driver/als_si114x.c b/driver/als_si114x.c
index 3b4cf39daa..5d9e595f07 100644
--- a/driver/als_si114x.c
+++ b/driver/als_si114x.c
@@ -581,7 +581,4 @@ const struct accelgyro_drv si114x_drv = {
 #ifdef CONFIG_ACCEL_INTERRUPTS
 	.irq_handler = irq_handler,
 #endif
-#ifdef CONFIG_ACCEL_FIFO
-	.load_fifo = NULL,
-#endif
 };