host_command: Add send_result() to the arg structure

It's a bit odd that the drivers package up a command to be processed
by host_command, but then host_command calls a global function to
pass the response back.

This adds ambiguity in the host_send_response() implementations as to
whether the command being responded to really is using the same
buffers that the driver set up.

Add a function pointer to the command, and have host_command call
that. Add status to the args structure also, which removes some of
the special case logic for error handling.

BUG=chrome-os-partner:11317
TEST=manual and a bit ad-hoc:
(note, this testing is not completed yet)
Check that snow and link still process commands correctly over I2C
from U-Boot. At this stage only the old interface is supported.
SMDK5250 # mkbp test
Old interface:

New interface:
Version 0: ec_command() returned error
Test failed with error -1
SMDK5250 #

Change-Id: Ic4afdcd7689666cc0f6af228abc6cffe41b0fcbf
Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/27468

7 files changed, 72 insertions, 57 deletions

diff --git a/chip/lm4/lpc.c b/chip/lm4/lpc.c
index 12d761e365..d40efbf26a 100644
--- a/chip/lm4/lpc.c
+++ b/chip/lm4/lpc.c
@@ -129,18 +129,19 @@ uint8_t *lpc_get_memmap_range(void)
 	return (uint8_t *)LPC_POOL_MEMMAP;
 }
 
-void host_send_response(enum ec_status result)
+static void lpc_send_response(struct host_cmd_handler_args *args)
 {
 	uint8_t *out;
-	int size = host_cmd_args.response_size;
+	int size = args->response_size;
 	int max_size;
 
 	/* Handle negative size */
 	if (size < 0) {
-		result = EC_RES_INVALID_RESPONSE;
+		args->result = EC_RES_INVALID_RESPONSE;
 		size = 0;
 	}
 
+	/* TODO(sjg@chromium.org): Really shold put flags in args too */
 	if (lpc_host_args->flags & EC_HOST_ARGS_FLAG_FROM_HOST) {
 		/* New-style response */
 		int csum;
@@ -155,12 +156,11 @@ void host_send_response(enum ec_status result)
 
 		lpc_host_args->data_size = size;
 
-		csum = host_cmd_args.command + lpc_host_args->flags +
-			lpc_host_args->command_version +
-			lpc_host_args->data_size;
+		csum = args->command + lpc_host_args->flags +
+			args->version + args->params_size;
 
 		for (i = 0; i < size; i++)
-			csum += host_cmd_args.response[i];
+			csum += args->response[i];
 
 		lpc_host_args->checksum = (uint8_t)csum;
 	} else {
@@ -172,9 +172,9 @@ void host_send_response(enum ec_status result)
 
 	/* Fail if response doesn't fit in the param buffer */
 	if (size > max_size)
-		result = EC_RES_INVALID_RESPONSE;
+		args->result = EC_RES_INVALID_RESPONSE;
 	else if (host_cmd_args.response != out)
-		memcpy(out, host_cmd_args.response, size);
+		memcpy(out, args->response, size);
 
 	/*
 	 * Write result to the data byte.  This sets the TOH bit in the
@@ -184,7 +184,7 @@ void host_send_response(enum ec_status result)
 	 * TODO: (crosbug.com/p/7496) or it would, if we actually set up host
 	 * IRQs
 	 */
-	LPC_POOL_CMD[1] = result;
+	LPC_POOL_CMD[1] = args->result;
 
 	/* Clear the busy bit */
 	task_disable_irq(LM4_IRQ_LPC);
@@ -369,6 +369,8 @@ static void handle_acpi_command(void)
 static void handle_host_command(int cmd)
 {
 	host_cmd_args.command = cmd;
+	host_cmd_args.result = EC_RES_SUCCESS;
+	host_cmd_args.send_response = lpc_send_response;
 
 	/* See if we have an old or new style command */
 	if (lpc_host_args->flags & EC_HOST_ARGS_FLAG_FROM_HOST) {
@@ -385,23 +387,20 @@ static void handle_host_command(int cmd)
 
 		/* Verify params size */
 		if (size > EC_HOST_PARAM_SIZE) {
-			host_send_response(EC_RES_INVALID_PARAM);
-			return;
+			host_cmd_args.result = EC_RES_INVALID_PARAM;
+		} else {
+			/* Verify checksum */
+			csum = host_cmd_args.command +
+				lpc_host_args->flags +
+				lpc_host_args->command_version +
+				lpc_host_args->data_size;
+
+			for (i = 0; i < size; i++)
+				csum += cmd_params[i];
+
+			if ((uint8_t)csum != lpc_host_args->checksum)
+				host_cmd_args.result = EC_RES_INVALID_CHECKSUM;
 		}
-
-		/* Verify checksum */
-		csum = host_cmd_args.command + lpc_host_args->flags +
-			lpc_host_args->command_version +
-			lpc_host_args->data_size;
-
-		for (i = 0; i < size; i++)
-			csum += cmd_params[i];
-
-		if ((uint8_t)csum != lpc_host_args->checksum) {
-			host_send_response(EC_RES_INVALID_CHECKSUM);
-			return;
-		}
-
 	} else {
 		/* Old style command */
 		host_cmd_args.version = 0;
diff --git a/chip/lm4/mock_lpc.c b/chip/lm4/mock_lpc.c
index 5b31ec4bc2..b1d12efead 100644
--- a/chip/lm4/mock_lpc.c
+++ b/chip/lm4/mock_lpc.c
@@ -65,13 +65,6 @@ void lpc_comx_put_char(int c)
 }
 
 
-void host_send_response(enum ec_status result, const uint8_t *data, int size)
-{
-	/* Not implemented */
-	return;
-}
-
-
 uint8_t *lpc_get_memmap_range(void)
 {
 	return (uint8_t *)LPC_POOL_CMD_DATA + EC_HOST_PARAM_SIZE * 2;
diff --git a/chip/stm32/i2c.c b/chip/stm32/i2c.c
index 137dc48485..248eed7237 100644
--- a/chip/stm32/i2c.c
+++ b/chip/stm32/i2c.c
@@ -122,14 +122,14 @@ static int i2c_write_raw(int port, void *buf, int len)
 	return len;
 }
 
-void host_send_response(enum ec_status result)
+static void i2c_send_response(struct host_cmd_handler_args *args)
 {
-	const uint8_t *data = host_cmd_args.response;
-	int size = host_cmd_args.response_size;
+	const uint8_t *data = args->response;
+	int size = args->response_size;
 	uint8_t *out = host_buffer;
 	int sum, i;
 
-	*out++ = result;
+	*out++ = args->result;
 	for (i = 0, sum = 0; i < size; i++, data++, out++) {
 		if (data != out)
 			*out = *data;
@@ -176,6 +176,9 @@ static void i2c_event_handler(int port)
 			if (rx_index) {
 				/* we have an available command : execute it */
 				host_cmd_args.command = host_buffer[0];
+				host_cmd_args.result = EC_RES_SUCCESS;
+				host_cmd_args.send_response =
+					i2c_send_response;
 				host_cmd_args.version = 0;
 				host_cmd_args.params = host_buffer + 1;
 				host_cmd_args.params_size = EC_HOST_PARAM_SIZE;
diff --git a/chip/stm32/spi.c b/chip/stm32/spi.c
index 219b82311a..5b59208317 100644
--- a/chip/stm32/spi.c
+++ b/chip/stm32/spi.c
@@ -147,6 +147,11 @@ static void reply(int port, char *msg, int msg_len)
 	dma_start_tx(dmac, msg_len, (void *)&STM32_SPI_DR(port), out_msg);
 }
 
+/* dummy handler for SPI - will be filled in later */
+static void spi_send_response(struct host_cmd_handler_args *args)
+{
+}
+
 /**
  * Handles an interrupt on the specified port.
  *
@@ -187,6 +192,8 @@ static void spi_interrupt(int port)
 	 * current devel board.
 	 */
 	args.command = cmd;
+	args.result = EC_RES_SUCCESS;
+	args.send_response = spi_send_response;
 	args.version = 0;
 	args.params = out_msg + SPI_MSG_HEADER_BYTES + 1;
 	args.params_size = sizeof(out_msg) - SPI_MSG_PROTO_BYTES;
diff --git a/common/host_command.c b/common/host_command.c
index 1420e79434..1a1c21fd40 100644
--- a/common/host_command.c
+++ b/common/host_command.c
@@ -49,15 +49,19 @@ void host_command_received(struct host_cmd_handler_args *args)
 	if (args->command == EC_CMD_REBOOT) {
 		system_reset(SYSTEM_RESET_HARD);
 		/* Reset should never return; if it does, post an error */
-		host_send_response(EC_RES_ERROR);
-		return;
+		args->result = EC_RES_ERROR;
 	}
 
-	/* Save the command */
-	pending_args = args;
+	/* If the driver has signalled an error, send the response now */
+	if (args->result) {
+		args->send_response(args);
+	} 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);
+		/* Wake up the task to handle the command */
+		task_set_event(TASK_ID_HOSTCMD, TASK_EVENT_CMD_PENDING, 0);
+	}
 }
 
 /*
@@ -224,8 +228,9 @@ void host_command_task(void)
 		int evt = task_wait_event(-1);
 		/* process it */
 		if ((evt & TASK_EVENT_CMD_PENDING) && pending_args) {
-			enum ec_status res = host_command_process(pending_args);
-			host_send_response(res);
+			pending_args->result =
+					host_command_process(pending_args);
+			pending_args->send_response(pending_args);
 		}
 	}
 }
diff --git a/common/system_common.c b/common/system_common.c
index 3251a79a56..5ede75b913 100644
--- a/common/system_common.c
+++ b/common/system_common.c
@@ -795,7 +795,8 @@ int host_command_reboot(struct host_cmd_handler_args *args)
 
 #ifdef CONFIG_TASK_HOSTCMD
 	/* Clean busy bits on host */
-	host_send_response(EC_RES_SUCCESS);
+	args->result = EC_RES_SUCCESS;
+	args->send_response(args);
 #endif
 
 	CPRINTF("[%T Executing host reboot command %d]\n", p.cmd);
diff --git a/include/host_command.h b/include/host_command.h
index 65cf849c6a..a88761a6e2 100644
--- a/include/host_command.h
+++ b/include/host_command.h
@@ -13,10 +13,16 @@
 
 /* Args for host command handler */
 struct host_cmd_handler_args {
+	/*
+	 * The driver that receives the command sets up the send_response()
+	 * handler. Once the command is processed this handler is called to
+	 * send the response back to the host.
+	 */
+	void (*send_response)(struct host_cmd_handler_args *args);
 	uint8_t command;       /* Command (e.g., EC_CMD_FLASH_GET_INFO) */
 	uint8_t version;       /* Version of command (0-31) */
-	const uint8_t *params; /* Input parameters */
 	uint8_t params_size;   /* Size of input parameters in bytes */
+	const uint8_t *params; /* Input parameters */
 	/*
 	 * Pointer to output response data buffer.  On input to the handler,
 	 * points to a buffer of size response_max.  Command handler can change
@@ -31,6 +37,17 @@ struct host_cmd_handler_args {
 	 */
 	uint8_t response_max;
 	uint8_t response_size; /* Size of data pointed to by resp_ptr */
+
+	/*
+	 * This is the result returned by command and therefore the status to
+	 * be reported from the command execution to the host. The driver
+	 * should set this to EC_RES_SUCCESS on receipt of a valid command.
+	 * It is then passed back to the driver via send_response() when
+	 * command execution is complete. The driver may still override this
+	 * when sending the response back to the host if it detects an error
+	 * in the response or in its own operation.
+	 */
+	enum ec_status result;
 };
 
 /* Host command */
@@ -100,16 +117,6 @@ uint32_t host_get_events(void);
  */
 void host_command_received(struct host_cmd_handler_args *args);
 
-/**
- * Send a successful result code to a host command.
- *
- * Response data, if any, has been stored in the args passed to
- * host_command_received().
- *
- * @param result        Result code for the command (EC_RES_...)
- */
-void host_send_response(enum ec_status result);
-
 /* Register a host command handler */
 #define DECLARE_HOST_COMMAND(command, routine, version_mask)		\
 	const struct host_command __host_cmd_##command			\