Move pending command logic into host_command

This logic doesn't really belong in drivers, since to enable another
driver (like SPI) we must repeat it all. This is tricky if we enable
both I2C and SPI.

Move the logic into host_command.

BUG=chrome-os-partner:10533
BRANCH=none
TEST=manual

Use U-Boot to test comms status functionality on snow:

SMDK5250 # mkbp write rw 40000000
SMDK5250 # mkbp erase rw
SMDK5250 # mkbp erase rw

Change-Id: I3f90aada80208cd0540be14525f73f980ad33292
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/32075
Reviewed-by: David Hendricks <dhendrix@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>

3 files changed, 98 insertions, 103 deletions

diff --git a/board/snow/board.h b/board/snow/board.h
index a9ef304701..86af96c01c 100644
--- a/board/snow/board.h
+++ b/board/snow/board.h
@@ -18,6 +18,7 @@
 #define CONFIG_I2C
 
 #define CONFIG_CONFIGURE_BOARD_LATE
+#define CONFIG_HOST_COMMAND_STATUS
 
 /* Debug features */
 #define CONFIG_PANIC_HELP
diff --git a/chip/stm32/i2c.c b/chip/stm32/i2c.c
index c5b1dd9018..2bc072f266 100644
--- a/chip/stm32/i2c.c
+++ b/chip/stm32/i2c.c
@@ -23,12 +23,6 @@
 #define CPUTS(outstr) cputs(CC_I2C, outstr)
 #define CPRINTF(format, args...) cprintf(CC_I2C, format, ## args)
 
-#ifdef DEBUG
-#define debug(f, a...) CPRINTF(f, ##a)
-#else
-#define debug(f, a...)
-#endif
-
 /* 8-bit I2C slave address */
 #define I2C_ADDRESS 0x3c
 
@@ -88,12 +82,6 @@ static struct host_cmd_handler_args host_cmd_args;
 /* Flag indicating if a command is currently in the buffer */
 static uint8_t rx_pending;
 
-/* Indicates that a command is in progress */
-static uint8_t command_pending;
-
-/* The result of the last 'slow' operation */
-static uint8_t saved_result = EC_RES_UNAVAILABLE;
-
 static inline void disable_i2c_interrupt(int port)
 {
 	STM32_I2C_CR2(port) &= ~(3 << 8);
@@ -172,45 +160,8 @@ static void i2c_send_response(struct host_cmd_handler_args *args)
 	const uint8_t *data = args->response;
 	int size = args->response_size;
 	uint8_t *out = host_buffer;
-	int watch_command_pending = !in_interrupt_context();
 	int sum = 0, i;
 
-	/*
-	 * TODO(sjg@chromium.org):
-	 * The logic here is a little painful since we are avoiding changing
-	 * host_command. If we got an 'in progress' previously, then this
-	 * must be the completion of that command, so stash the result
-	 * code. We can't send it back to the host now since we already sent
-	 * the in-progress response and the host is on to other things now.
-	 *
-	 * Of course, if we are in interrupt context, then we are just
-	 * handling a get_status response. We can't check that in
-	 * args->command of course because the original command value has
-	 * now been overwritten. This would be much easier to do in
-	 * host_command since it actually knows what is going on.
-	 *
-	 * When a EC_CMD_RESEND_RESPONSE arrives we will supply this response
-	 * to that command.
-	 *
-	 * We don't support stashing response data, so mark the response as
-	 * unavailable in that case.
-	 *
-	 * TODO(sjg@chromium): It would all be easier if drivers used a
-	 * stack variable for args and host_command was responsible for
-	 * saving the command before execution. Perhaps fast commands could
-	 * be executed in interrupt context anyway?
-	 */
-	if (command_pending && watch_command_pending) {
-		debug("pending complete, size=%d, result=%d\n",
-			args->response_size, args->result);
-		if (args->response_size != 0)
-			saved_result = EC_RES_UNAVAILABLE;
-		else
-			saved_result = args->result;
-		command_pending = 0;
-		return;
-	}
-
 	*out++ = args->result;
 	if (!args->i2c_old_response) {
 		*out++ = size;
@@ -225,12 +176,6 @@ static void i2c_send_response(struct host_cmd_handler_args *args)
 
 	/* send the answer to the AP */
 	i2c_write_raw_slave(I2C2, host_buffer, out - host_buffer);
-
-	if (watch_command_pending) {
-		command_pending = (args->result == EC_RES_IN_PROGRESS);
-		if (command_pending)
-			debug("Command pending\n");
-	}
 }
 
 /* Process the command in the i2c host buffer */
@@ -273,20 +218,7 @@ static void i2c_process_command(void)
 	args->response_max = EC_HOST_PARAM_SIZE;
 	args->response_size = 0;
 
-	/*
-	 * Special handling for GET_STATUS, which happens entirely outside
-	 * host_command.
-	 */
-	if (args->command == EC_CMD_GET_COMMS_STATUS) {
-		/*
-		 * Could do this directly, but then we get no logging
-		 * args->result = host_command_get_comms_status(args);
-		 */
-		args->result = host_command_process(args);
-		args->send_response(args);
-	} else {
-		host_command_received(args);
-	}
+	host_command_received(args);
 }
 
 static void i2c_event_handler(int port)
@@ -557,38 +489,6 @@ static int i2c_init(void)
 DECLARE_HOOK(HOOK_INIT, i2c_init, HOOK_PRIO_DEFAULT);
 
 
-/* Returns current command status (busy or not) */
-static int host_command_get_comms_status(struct host_cmd_handler_args *args)
-{
-	struct ec_response_get_comms_status *r = args->response;
-
-	r->flags = command_pending ? EC_COMMS_STATUS_PROCESSING : 0;
-	args->response_size = sizeof(*r);
-
-	return EC_SUCCESS;
-}
-
-DECLARE_HOST_COMMAND(EC_CMD_GET_COMMS_STATUS,
-		     host_command_get_comms_status,
-		     EC_VER_MASK(0));
-
-/* Resend the last saved response */
-static int host_command_resend_response(struct host_cmd_handler_args *args)
-{
-	/* Handle resending response */
-	args->result = saved_result;
-	args->response_size = 0;
-
-	saved_result = EC_RES_UNAVAILABLE;
-
-	return EC_SUCCESS;
-}
-
-DECLARE_HOST_COMMAND(EC_CMD_RESEND_RESPONSE,
-		     host_command_resend_response,
-		     EC_VER_MASK(0));
-
-
 /*****************************************************************************/
 /* STM32 Host I2C */
 
diff --git a/common/host_command.c b/common/host_command.c
index 389bb537de..6a872de508 100644
--- a/common/host_command.c
+++ b/common/host_command.c
@@ -31,6 +31,17 @@ static uint8_t host_memmap[EC_MEMMAP_SIZE];
 
 static int hcdebug;  /* Enable extra host command debug output */
 
+#ifdef CONFIG_HOST_COMMAND_STATUS
+/*
+ * Indicates that a 'slow' command has sent EC_RES_IN_PROGRESS but hasn't
+ * sent a final status (i.e. it is in progress)
+ */
+static uint8_t command_pending;
+
+/* The result of the last 'slow' operation */
+static uint8_t saved_result = EC_RES_UNAVAILABLE;
+#endif
+
 uint8_t *host_get_memmap(int offset)
 {
 #ifdef CONFIG_LPC
@@ -42,6 +53,49 @@ uint8_t *host_get_memmap(int offset)
 
 void host_send_response(struct host_cmd_handler_args *args)
 {
+#ifdef CONFIG_HOST_COMMAND_STATUS
+	/*
+	 * TODO(sjg@chromium.org):
+	 * If we got an 'in progress' previously, then this
+	 * must be the completion of that command, so stash the result
+	 * code. We can't send it back to the host now since we already sent
+	 * the in-progress response and the host is on to other things now.
+	 *
+	 * If we are in interrupt context, then we are handling a
+	 * get_status response or an immediate error which prevented us
+	 * from processing the command. Note we can't check for the
+	 * GET_COMMS_STATUS command in args->command because the original
+	 * command value has now been overwritten.
+	 *
+	 * When a EC_CMD_RESEND_RESPONSE arrives we will supply this response
+	 * to that command.
+	 *
+	 * We don't support stashing response data, so mark the response as
+	 * unavailable in that case.
+	 *
+	 * TODO(sjg@chromium.org): If we stashed the command in host_command
+	 * before processing it, then it would not get overwritten by a
+	 * subsequent command and we could simplify the logic here by adding
+	 * a flag to host_cmd_handler_args to indicate that the command had
+	 * an interim response. We would have to make this stashing dependent
+	 * on CONFIG_HOST_COMMAND_STATUS also.
+	 */
+	if (!in_interrupt_context()) {
+		if (command_pending) {
+			CPRINTF("pending complete, size=%d, result=%d\n",
+				args->response_size, args->result);
+			if (args->response_size != 0)
+				saved_result = EC_RES_UNAVAILABLE;
+			else
+				saved_result = args->result;
+			command_pending = 0;
+			return;
+		} else if (args->result == EC_RES_IN_PROGRESS) {
+			command_pending = 1;
+			CPRINTF("Command pending\n");
+		}
+	}
+#endif
 	args->send_response(args);
 }
 
@@ -60,16 +114,23 @@ void host_command_received(struct host_cmd_handler_args *args)
 		args->result = EC_RES_ERROR;
 	}
 
-	/* If the driver has signalled an error, send the response now */
 	if (args->result) {
-		host_send_response(args);
+		; /* driver has signalled an error, respond now */
+#ifdef CONFIG_HOST_COMMAND_STATUS
+	} else if (args->command == EC_CMD_GET_COMMS_STATUS) {
+		args->result = host_command_process(args);
+#endif
 	} else {
 		/* Save the command */
 		pending_args = args;
 
 		/* Wake up the task to handle the command */
 		task_set_event(TASK_ID_HOSTCMD, TASK_EVENT_CMD_PENDING, 0);
+		return;
 	}
+
+	/* Send the response now */
+	host_send_response(args);
 }
 
 /*
@@ -214,6 +275,39 @@ enum ec_status host_command_process(struct host_cmd_handler_args *args)
 	return rv;
 }
 
+#ifdef CONFIG_HOST_COMMAND_STATUS
+/* Returns current command status (busy or not) */
+static int host_command_get_comms_status(struct host_cmd_handler_args *args)
+{
+	struct ec_response_get_comms_status *r = args->response;
+
+	r->flags = command_pending ? EC_COMMS_STATUS_PROCESSING : 0;
+	args->response_size = sizeof(*r);
+
+	return EC_SUCCESS;
+}
+
+DECLARE_HOST_COMMAND(EC_CMD_GET_COMMS_STATUS,
+		     host_command_get_comms_status,
+		     EC_VER_MASK(0));
+
+/* Resend the last saved response */
+static int host_command_resend_response(struct host_cmd_handler_args *args)
+{
+	/* Handle resending response */
+	args->result = saved_result;
+	args->response_size = 0;
+
+	saved_result = EC_RES_UNAVAILABLE;
+
+	return EC_SUCCESS;
+}
+
+DECLARE_HOST_COMMAND(EC_CMD_RESEND_RESPONSE,
+		     host_command_resend_response,
+		     EC_VER_MASK(0));
+#endif /* CONFIG_HOST_COMMAND_STATUS */
+
 /*****************************************************************************/
 /* Initialization / task */