lsm6ds0: Cache ODR and range on EC.

For the driver functions get_range and get_data_rate, each call would
end up executing an i2c transaction even if the value had not
changed. Therefore, I modified the lsm6ds0 driver to cache the output
data rate as well as the range. This prevents unecessary i2c
transactions from occuring.

BUG=chromium:476226
TEST=Flashed EC on samus and verified that the accelrange and accelrate
commands still worked and that the sensors were functional.
TEST=Verified Double Tap still worked.
TEST=make -j buildall tests
BRANCH=none

Change-Id: Ie432979266dc4e4892978005de5d1df62cc0654f
Signed-off-by: Aseda Aboagye <aaboagye@google.com>
Reviewed-on: https://chromium-review.googlesource.com/265933
Reviewed-by: Alec Berg <alecaberg@chromium.org>
Commit-Queue: Aseda Aboagye <aaboagye@chromium.org>
Tested-by: Aseda Aboagye <aaboagye@chromium.org>

5 files changed, 108 insertions, 46 deletions

diff --git a/board/ryu_sh/board.c b/board/ryu_sh/board.c
index 2266e7d95a..4e2f68f18a 100644
--- a/board/ryu_sh/board.c
+++ b/board/ryu_sh/board.c
@@ -37,26 +37,53 @@ const unsigned int i2c_ports_used = ARRAY_SIZE(i2c_ports);
 /* Sensor mutex */
 static struct mutex g_mutex;
 
+/* lsm6ds0 local sensor data (per-sensor) */
+struct lsm6ds0_data g_lsm6ds0_data[2];
+
 struct motion_sensor_t motion_sensors[] = {
 
 	/*
 	 * Note: lsm6ds0: supports accelerometer and gyro sensor
-	 * Requriement: accelerometer sensor must init before gyro sensor
+	 * Requirement: accelerometer sensor must init before gyro sensor
 	 * DO NOT change the order of the following table.
 	 */
-	{SENSOR_ACTIVE_S0_S3, "Accel", MOTIONSENSE_CHIP_LSM6DS0,
-		MOTIONSENSE_TYPE_ACCEL, MOTIONSENSE_LOC_LID,
-		&lsm6ds0_drv, &g_mutex, NULL,
-		LSM6DS0_ADDR1, NULL, 119000, 2},
+	{.name = "Accel",
+	 .active_mask = SENSOR_ACTIVE_S0_S3,
+	 .chip = MOTIONSENSE_CHIP_LSM6DS0,
+	 .type = MOTIONSENSE_TYPE_ACCEL,
+	 .location = MOTIONSENSE_LOC_LID,
+	 .drv = &lsm6ds0_drv,
+	 .mutex = &g_mutex,
+	 .drv_data = &g_lsm6ds0_data[0],
+	 .i2c_addr = LSM6DS0_ADDR1,
+	 .rot_standard_ref = NULL,
+	 .default_odr = 119000,
+	 .default_range = 2
+	},
 
-	{SENSOR_ACTIVE_S0_S3, "Gyro", MOTIONSENSE_CHIP_LSM6DS0,
-		MOTIONSENSE_TYPE_GYRO, MOTIONSENSE_LOC_LID,
-		&lsm6ds0_drv, &g_mutex, NULL,
-		LSM6DS0_ADDR1, NULL, 119000, 2000},
+	{.name = "Gyro",
+	 .active_mask = SENSOR_ACTIVE_S0_S3,
+	 .chip = MOTIONSENSE_CHIP_LSM6DS0,
+	 .type = MOTIONSENSE_TYPE_GYRO,
+	 .location = MOTIONSENSE_LOC_LID,
+	 .drv = &lsm6ds0_drv,
+	 .mutex = &g_mutex,
+	 .drv_data = &g_lsm6ds0_data[1],
+	 .i2c_addr = LSM6DS0_ADDR1,
+	 .rot_standard_ref = NULL,
+	 .default_odr = 119000,
+	 .default_range = 2000
+	},
 
 };
 const unsigned int motion_sensor_count = ARRAY_SIZE(motion_sensors);
 
+/*
+ * Note: If a new sensor driver is added, make sure to update the following
+ * assert.
+ */
+BUILD_ASSERT(ARRAY_SIZE(motion_sensors) == ARRAY_SIZE(g_lsm6ds0_data));
+
 void board_config_pre_init(void)
 {
 	/*
diff --git a/board/samus/board.c b/board/samus/board.c
index 9002e0e92f..1222528959 100644
--- a/board/samus/board.c
+++ b/board/samus/board.c
@@ -261,6 +261,9 @@ static struct mutex g_lid_mutex;
 /* kxcj9 local/private data */
 struct kxcj9_data g_kxcj9_data;
 
+/* lsm6ds0 local sensor data (per-sensor) */
+struct lsm6ds0_data g_lsm6ds0_data[2];
+
 /* Four Motion sensors */
 /* Matrix to rotate accelrator into standard reference frame */
 const matrix_3x3_t base_standard_ref = {
@@ -276,26 +279,52 @@ const matrix_3x3_t lid_standard_ref = {
 };
 
 struct motion_sensor_t motion_sensors[] = {
-
 	/*
 	 * Note: lsm6ds0: supports accelerometer and gyro sensor
-	 * Requriement: accelerometer sensor must init before gyro sensor
+	 * Requirement: accelerometer sensor must init before gyro sensor
 	 * DO NOT change the order of the following table.
 	 */
-	{SENSOR_ACTIVE_S0_S3_S5, "Base", MOTIONSENSE_CHIP_LSM6DS0,
-		MOTIONSENSE_TYPE_ACCEL, MOTIONSENSE_LOC_BASE,
-		&lsm6ds0_drv, &g_base_mutex, NULL,
-		LSM6DS0_ADDR1, &base_standard_ref, 119000, 2},
-
-	{SENSOR_ACTIVE_S0, "Lid",  MOTIONSENSE_CHIP_KXCJ9,
-		MOTIONSENSE_TYPE_ACCEL, MOTIONSENSE_LOC_LID,
-		&kxcj9_drv, &g_lid_mutex, &g_kxcj9_data,
-		KXCJ9_ADDR0, &lid_standard_ref, 100000, 2},
-
-	{SENSOR_ACTIVE_S0, "Base Gyro", MOTIONSENSE_CHIP_LSM6DS0,
-		MOTIONSENSE_TYPE_GYRO, MOTIONSENSE_LOC_BASE,
-		&lsm6ds0_drv, &g_base_mutex, NULL,
-		LSM6DS0_ADDR1, NULL, 119000, 2000},
+	{.name = "Base",
+	 .active_mask = SENSOR_ACTIVE_S0_S3_S5,
+	 .chip = MOTIONSENSE_CHIP_LSM6DS0,
+	 .type = MOTIONSENSE_TYPE_ACCEL,
+	 .location = MOTIONSENSE_LOC_BASE,
+	 .drv = &lsm6ds0_drv,
+	 .mutex = &g_base_mutex,
+	 .drv_data = &g_lsm6ds0_data[0],
+	 .i2c_addr = LSM6DS0_ADDR1,
+	 .rot_standard_ref = &base_standard_ref,
+	 .default_odr = 119000,
+	 .default_range = 2
+	},
+
+	{.name = "Lid",
+	 .active_mask = SENSOR_ACTIVE_S0,
+	 .chip = MOTIONSENSE_CHIP_KXCJ9,
+	 .type = MOTIONSENSE_TYPE_ACCEL,
+	 .location = MOTIONSENSE_LOC_LID,
+	 .drv = &kxcj9_drv,
+	 .mutex = &g_lid_mutex,
+	 .drv_data = &g_kxcj9_data,
+	 .i2c_addr = KXCJ9_ADDR0,
+	 .rot_standard_ref = &lid_standard_ref,
+	 .default_odr = 100000,
+	 .default_range = 2
+	},
+
+	{.name = "Base Gyro",
+	 .active_mask = SENSOR_ACTIVE_S0,
+	 .chip = MOTIONSENSE_CHIP_LSM6DS0,
+	 .type = MOTIONSENSE_TYPE_GYRO,
+	 .location = MOTIONSENSE_LOC_BASE,
+	 .drv = &lsm6ds0_drv,
+	 .mutex = &g_base_mutex,
+	 .drv_data = &g_lsm6ds0_data[1],
+	 .i2c_addr = LSM6DS0_ADDR1,
+	 .rot_standard_ref = NULL,
+	 .default_odr = 119000,
+	 .default_range = 2000
+	},
 
 };
 const unsigned int motion_sensor_count = ARRAY_SIZE(motion_sensors);
diff --git a/common/motion_sense.c b/common/motion_sense.c
index f303853e72..0c1841b502 100644
--- a/common/motion_sense.c
+++ b/common/motion_sense.c
@@ -456,7 +456,7 @@ static int host_cmd_motion_sense(struct host_cmd_handler_args *args)
 		if (sensor == NULL)
 			return EC_RES_INVALID_PARAM;
 
-		/* Set new data rate if the data arg has a value. */
+		/* Set new range if the data arg has a value. */
 		if (in->sensor_range.data != EC_MOTION_SENSE_NO_VALUE) {
 			if (sensor->drv->set_range(sensor,
 						in->sensor_range.data,
diff --git a/driver/accelgyro_lsm6ds0.c b/driver/accelgyro_lsm6ds0.c
index bb8f6d8fc9..a93166c1de 100644
--- a/driver/accelgyro_lsm6ds0.c
+++ b/driver/accelgyro_lsm6ds0.c
@@ -169,6 +169,7 @@ static int set_range(const struct motion_sensor_t *s,
 	int ret, ctrl_val, range_tbl_size;
 	uint8_t ctrl_reg, reg_val;
 	const struct accel_param_pair *ranges;
+	struct lsm6ds0_data *data = (struct lsm6ds0_data *)s->drv_data;
 
 	ctrl_reg = get_ctrl_reg(s->type);
 	ranges = get_range_table(s->type, &range_tbl_size);
@@ -188,6 +189,11 @@ static int set_range(const struct motion_sensor_t *s,
 	ctrl_val = (ctrl_val & ~LSM6DS0_RANGE_MASK) | reg_val;
 	ret = raw_write8(s->i2c_addr, ctrl_reg, ctrl_val);
 
+	/* Now that we have set the range, update the driver's value. */
+	if (ret == EC_SUCCESS)
+		data->sensor_range = get_engineering_val(reg_val, ranges,
+							 range_tbl_size);
+
 accel_cleanup:
 	mutex_unlock(s->mutex);
 	return EC_SUCCESS;
@@ -196,15 +202,10 @@ accel_cleanup:
 static int get_range(const struct motion_sensor_t *s,
 				int *range)
 {
-	int ret, ctrl_val, range_tbl_size;
-	uint8_t ctrl_reg;
-	const struct accel_param_pair *ranges;
-	ranges = get_range_table(s->type, &range_tbl_size);
-	ctrl_reg = get_ctrl_reg(s->type);
-	ret = raw_read8(s->i2c_addr, ctrl_reg, &ctrl_val);
-	*range = get_engineering_val(ctrl_val & LSM6DS0_RANGE_MASK,
-		ranges, range_tbl_size);
-	return ret;
+	struct lsm6ds0_data *data = (struct lsm6ds0_data *)s->drv_data;
+
+	*range = data->sensor_range;
+	return EC_SUCCESS;
 }
 
 static int set_resolution(const struct motion_sensor_t *s,
@@ -229,6 +230,7 @@ static int set_data_rate(const struct motion_sensor_t *s,
 	int ret, val, odr_tbl_size;
 	uint8_t ctrl_reg, reg_val;
 	const struct accel_param_pair *data_rates;
+	struct lsm6ds0_data *data = s->drv_data;
 
 	ctrl_reg = get_ctrl_reg(s->type);
 	data_rates = get_odr_table(s->type, &odr_tbl_size);
@@ -247,6 +249,11 @@ static int set_data_rate(const struct motion_sensor_t *s,
 	val = (val & ~LSM6DS0_ODR_MASK) | reg_val;
 	ret = raw_write8(s->i2c_addr, ctrl_reg, val);
 
+	/* Now that we have set the odr, update the driver's value. */
+	if (ret == EC_SUCCESS)
+		data->sensor_odr = get_engineering_val(reg_val, data_rates,
+						       odr_tbl_size);
+
 	/* CTRL_REG3_G 12h
 	 * [7] low-power mode = 0;
 	 * [6] high pass filter disabled;
@@ -273,18 +280,9 @@ accel_cleanup:
 static int get_data_rate(const struct motion_sensor_t *s,
 				int *rate)
 {
-	int ret, ctrl_val, odr_tbl_size;
-	uint8_t ctrl_reg;
-	const struct accel_param_pair *data_rates;
-	ctrl_reg = get_ctrl_reg(s->type);
-
-	ret = raw_read8(s->i2c_addr, ctrl_reg, &ctrl_val);
-	if (ret != EC_SUCCESS)
-		return EC_ERROR_UNKNOWN;
+	struct lsm6ds0_data *data = s->drv_data;
 
-	data_rates = get_odr_table(s->type, &odr_tbl_size);
-	*rate = get_engineering_val(ctrl_val & LSM6DS0_ODR_MASK,
-			data_rates, odr_tbl_size);
+	*rate = data->sensor_odr;
 	return EC_SUCCESS;
 }
 
diff --git a/driver/accelgyro_lsm6ds0.h b/driver/accelgyro_lsm6ds0.h
index c71fc37767..9f8953395d 100644
--- a/driver/accelgyro_lsm6ds0.h
+++ b/driver/accelgyro_lsm6ds0.h
@@ -116,6 +116,14 @@ enum lsm6ds0_bdu {
 /* Sensor resolution in number of bits. This sensor has fixed resolution. */
 #define LSM6DS0_RESOLUTION      16
 
+/* Run-time configurable parameters */
+struct lsm6ds0_data {
+	/* Current range */
+	int sensor_range;
+	/* Current output data rate */
+	int sensor_odr;
+};
+
 extern const struct accelgyro_drv lsm6ds0_drv;
 
 #endif /* __CROS_EC_ACCEL_LSM6DS0_H */