accel: refined motion sense host command interface

Added enum for motion sensor ID's into ec_commands.h so that the host
can easily send host commands targeting the desired accelerometer.

Changed sensor present flag to just senosr flags, currently with only a
single mask defined for sensor present. This allows for easier future
expansion of various flags.

Also, added a motion sense module flags to the dump sub-command for flags
that represent all sensors, such as is the motion sense task active.

BUG=chrome-os-partner:27321
BRANCH=rambi
TEST=Manual test on a glimmer by testing ectool motionsense command

Change-Id: Iac052269a60db9ff4506f0490c3a0c6daad5b626
Signed-off-by: Alec Berg <alecaberg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/193122
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/193309

3 files changed, 94 insertions, 23 deletions

diff --git a/common/motion_sense.c b/common/motion_sense.c
index e94604fae5..1fb158b7c9 100644
--- a/common/motion_sense.c
+++ b/common/motion_sense.c
@@ -322,6 +322,27 @@ void accel_int_base(enum gpio_signal signal)
 /*****************************************************************************/
 /* Host commands */
 
+/**
+ * Temporary function to map host sensor IDs to EC sensor IDs.
+ *
+ * TODO(crosbug.com/p/27320): Eventually we need a board specific table
+ * specifying which motion sensors are attached and which driver to use to
+ * access that sensor. Once we have this, this function should be able to go
+ * away.
+ */
+static int host_sensor_id_to_ec_sensor_id(int host_id)
+{
+	switch (host_id) {
+	case EC_MOTION_SENSOR_ACCEL_BASE:
+		return ACCEL_BASE;
+	case EC_MOTION_SENSOR_ACCEL_LID:
+		return ACCEL_LID;
+	}
+
+	/* If no match then the EC currently doesn't support ID received. */
+	return -1;
+}
+
 static int host_cmd_motion_sense(struct host_cmd_handler_args *args)
 {
 	const struct ec_params_motion_sense *in = args->params;
@@ -335,9 +356,13 @@ static int host_cmd_motion_sense(struct host_cmd_handler_args *args)
 		 * use some motion_sense data structure from the board file to
 		 * help fill in this response.
 		 */
-		out->dump.sensor_presence[0] = 1;
-		out->dump.sensor_presence[1] = 1;
-		out->dump.sensor_presence[2] = 0;
+		out->dump.module_flags =
+			(*(host_get_memmap(EC_MEMMAP_ACC_STATUS)) &
+				EC_MEMMAP_ACC_STATUS_PRESENCE_BIT) ?
+					MOTIONSENSE_MODULE_FLAG_ACTIVE : 0;
+		out->dump.sensor_flags[0] = MOTIONSENSE_SENSOR_FLAG_PRESENT;
+		out->dump.sensor_flags[1] = MOTIONSENSE_SENSOR_FLAG_PRESENT;
+		out->dump.sensor_flags[2] = 0;
 		out->dump.data[0] = acc_base_host[X];
 		out->dump.data[1] = acc_base_host[Y];
 		out->dump.data[2] = acc_base_host[Z];
@@ -354,7 +379,11 @@ static int host_cmd_motion_sense(struct host_cmd_handler_args *args)
 		 * use some motion_sense data structure from the board file to
 		 * help fill in this response.
 		 */
-		switch (in->sensor_odr.sensor_num) {
+		id = host_sensor_id_to_ec_sensor_id(in->sensor_odr.sensor_num);
+		if (id < 0)
+			return EC_RES_INVALID_PARAM;
+
+		switch (id) {
 		case ACCEL_BASE:
 			out->info.type = MOTIONSENSE_TYPE_ACCEL;
 			out->info.location = MOTIONSENSE_LOC_BASE;
@@ -396,11 +425,10 @@ static int host_cmd_motion_sense(struct host_cmd_handler_args *args)
 
 	case MOTIONSENSE_CMD_SENSOR_ODR:
 		/* Verify sensor number is valid. */
-		if (in->sensor_odr.sensor_num >= ACCEL_COUNT)
+		id = host_sensor_id_to_ec_sensor_id(in->sensor_odr.sensor_num);
+		if (id < 0)
 			return EC_RES_INVALID_PARAM;
 
-		id = in->sensor_odr.sensor_num;
-
 		/* Set new datarate if the data arg has a value. */
 		if (in->sensor_odr.data != EC_MOTION_SENSE_NO_VALUE) {
 			if (accel_set_datarate(id, in->sensor_odr.data,
@@ -419,11 +447,10 @@ static int host_cmd_motion_sense(struct host_cmd_handler_args *args)
 
 	case MOTIONSENSE_CMD_SENSOR_RANGE:
 		/* Verify sensor number is valid. */
-		if (in->sensor_odr.sensor_num >= ACCEL_COUNT)
+		id = host_sensor_id_to_ec_sensor_id(in->sensor_odr.sensor_num);
+		if (id < 0)
 			return EC_RES_INVALID_PARAM;
 
-		id = in->sensor_odr.sensor_num;
-
 		/* Set new datarate if the data arg has a value. */
 		if (in->sensor_range.data != EC_MOTION_SENSE_NO_VALUE) {
 			if (accel_set_range(id, in->sensor_range.data,
diff --git a/include/ec_commands.h b/include/ec_commands.h
index 082c4d16c0..439e871d77 100644
--- a/include/ec_commands.h
+++ b/include/ec_commands.h
@@ -1170,8 +1170,8 @@ enum ec_vboot_hash_status {
 /* Motion sense commands */
 enum motionsense_command {
 	/*
-	 * Dump command returns all motion sensor data including a physical
-	 * presence bit for each potential sensor.
+	 * Dump command returns all motion sensor data including motion sense
+	 * module flags and individual sensor flags.
 	 */
 	MOTIONSENSE_CMD_DUMP = 0,
 
@@ -1204,26 +1204,45 @@ enum motionsense_command {
 	MOTIONSENSE_NUM_CMDS
 };
 
+enum motionsensor_id {
+	EC_MOTION_SENSOR_ACCEL_BASE = 0,
+	EC_MOTION_SENSOR_ACCEL_LID = 1,
+	EC_MOTION_SENSOR_GYRO = 2,
+
+	/*
+	 * Note, if more sensors are added and this count changes, the padding
+	 * in ec_response_motion_sense dump command must be modified.
+	 */
+	EC_MOTION_SENSOR_COUNT = 3
+};
+
 /* List of motion sensor types. */
 enum motionsensor_type {
-	MOTIONSENSE_TYPE_ACCEL,
-	MOTIONSENSE_TYPE_GYRO,
+	MOTIONSENSE_TYPE_ACCEL = 0,
+	MOTIONSENSE_TYPE_GYRO = 1,
 };
 
 /* List of motion sensor locations. */
 enum motionsensor_location {
-	MOTIONSENSE_LOC_BASE,
-	MOTIONSENSE_LOC_LID,
+	MOTIONSENSE_LOC_BASE = 0,
+	MOTIONSENSE_LOC_LID = 1,
 };
 
 /* List of motion sensor chips. */
 enum motionsensor_chip {
-	MOTIONSENSE_CHIP_KXCJ9,
+	MOTIONSENSE_CHIP_KXCJ9 = 0,
 };
 
+/* Module flag masks used for the dump sub-command. */
+#define MOTIONSENSE_MODULE_FLAG_ACTIVE (1<<0)
+
+/* Sensor flag masks used for the dump sub-command. */
+#define MOTIONSENSE_SENSOR_FLAG_PRESENT (1<<0)
+
 /*
  * Send this value for the data element to only perform a read. If you
- * send any other value, the EC will interpret it as data to set.
+ * send any other value, the EC will interpret it as data to set and will
+ * return the actual value set.
  */
 #define EC_MOTION_SENSE_NO_VALUE -1
 
@@ -1237,11 +1256,13 @@ struct ec_params_motion_sense {
 
 		/* Used for MOTIONSENSE_CMD_EC_RATE. */
 		struct {
+			/* Data to set or EC_MOTION_SENSE_NO_VALUE to read. */
 			int16_t data;
 		} ec_rate;
 
 		/* Used for MOTIONSENSE_CMD_INFO. */
 		struct {
+			/* Should be element of enum motionsensor_id. */
 			uint8_t sensor_num;
 		} info;
 
@@ -1250,9 +1271,15 @@ struct ec_params_motion_sense {
 		 * MOTIONSENSE_CMD_SENSOR_RANGE.
 		 */
 		struct {
+			/* Should be element of enum motionsensor_id. */
 			uint8_t sensor_num;
+
+			/* Rounding flag, true for round-up, false for down. */
 			uint8_t roundup;
+
 			uint16_t reserved;
+
+			/* Data to set or EC_MOTION_SENSE_NO_VALUE to read. */
 			int32_t data;
 		} sensor_odr, sensor_range;
 	};
@@ -1262,15 +1289,25 @@ struct ec_response_motion_sense {
 	union {
 		/* Used for MOTIONSENSE_CMD_DUMP. */
 		struct {
-			uint8_t sensor_presence[3];
-			uint8_t reserved;
-			int16_t data[9];
+			/* Flags representing the motion sensor module. */
+			uint8_t module_flags;
+
+			/* Flags for each sensor in enum motionsensor_id. */
+			uint8_t sensor_flags[EC_MOTION_SENSOR_COUNT];
+
+			/* Array of all sensor data. Each sensor is 3-axis. */
+			int16_t data[3*EC_MOTION_SENSOR_COUNT];
 		} dump;
 
 		/* Used for MOTIONSENSE_CMD_INFO. */
 		struct {
+			/* Should be element of enum motionsensor_type. */
 			uint8_t type;
+
+			/* Should be element of enum motionsensor_location. */
 			uint8_t location;
+
+			/* Should be element of enum motionsensor_chip. */
 			uint8_t chip;
 		} info;
 
@@ -1279,6 +1316,7 @@ struct ec_response_motion_sense {
 		 * and MOTIONSENSE_CMD_SENSOR_RANGE.
 		 */
 		struct {
+			/* Current value of the parameter queried. */
 			int32_t ret;
 		} ec_rate, sensor_odr, sensor_range;
 	};
diff --git a/util/ectool.c b/util/ectool.c
index 712f4fcf9d..af7b3f5c66 100644
--- a/util/ectool.c
+++ b/util/ectool.c
@@ -1739,9 +1739,15 @@ static int cmd_motionsense(int argc, char **argv)
 		if (rv < 0)
 			return rv;
 
-		for (i = 0; i < ARRAY_SIZE(resp.dump.sensor_presence); i++) {
+		if (resp.dump.module_flags & MOTIONSENSE_MODULE_FLAG_ACTIVE)
+			printf("Motion sensing active\n");
+		else
+			printf("Motion sensing inactive\n");
+
+		for (i = 0; i < EC_MOTION_SENSOR_COUNT; i++) {
 			printf("Sensor %d: ", i);
-			if (resp.dump.sensor_presence[i])
+			if (resp.dump.sensor_flags[i] &
+					MOTIONSENSE_SENSOR_FLAG_PRESENT)
 				printf("%d\t%d\t%d\n", resp.dump.data[3*i],
 							resp.dump.data[3*i+1],
 							resp.dump.data[3*i+2]);