Revert "host_command: Add send_result() to the arg structure"

This reverts commit 18db93b25b05c871826fd1853a33a560e64ed247

Breaks link checksums.

Change-Id: Ieeb278b7d4da0600bdc9ced1476b67f23abce1a1
Reviewed-on: https://gerrit.chromium.org/gerrit/28136
Commit-Ready: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Tested-by: Randall Spangler <rspangler@chromium.org>

chip/lm4/lpc.c

@@ -129,19 +129,18 @@ uint8_t *lpc_get_memmap_range(void)
 	return (uint8_t *)LPC_POOL_MEMMAP;
 }
 
-static void lpc_send_response(struct host_cmd_handler_args *args)
+void host_send_response(enum ec_status result)
 {
 	uint8_t *out;
-	int size = args->response_size;
+	int size = host_cmd_args.response_size;
 	int max_size;
 
 	/* Handle negative size */
 	if (size < 0) {
-		args->result = EC_RES_INVALID_RESPONSE;
+		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;
@@ -156,11 +155,12 @@ static void lpc_send_response(struct host_cmd_handler_args *args)
 
 		lpc_host_args->data_size = size;
 
-		csum = args->command + lpc_host_args->flags +
-			args->version + args->params_size;
+		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 += args->response[i];
+			csum += host_cmd_args.response[i];
 
 		lpc_host_args->checksum = (uint8_t)csum;
 	} else {
@@ -172,9 +172,9 @@ static void lpc_send_response(struct host_cmd_handler_args *args)
 
 	/* Fail if response doesn't fit in the param buffer */
 	if (size > max_size)
-		args->result = EC_RES_INVALID_RESPONSE;
+		result = EC_RES_INVALID_RESPONSE;
 	else if (host_cmd_args.response != out)
-		memcpy(out, args->response, size);
+		memcpy(out, host_cmd_args.response, size);
 
 	/*
 	 * Write result to the data byte.  This sets the TOH bit in the
@@ -184,7 +184,7 @@ static void lpc_send_response(struct host_cmd_handler_args *args)
 	 * TODO: (crosbug.com/p/7496) or it would, if we actually set up host
 	 * IRQs
 	 */
-	LPC_POOL_CMD[1] = args->result;
+	LPC_POOL_CMD[1] = result;
 
 	/* Clear the busy bit */
 	task_disable_irq(LM4_IRQ_LPC);
@@ -369,8 +369,6 @@ 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) {
@@ -387,20 +385,23 @@ static void handle_host_command(int cmd)
 
 		/* Verify params size */
 		if (size > EC_HOST_PARAM_SIZE) {
-			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;
+			host_send_response(EC_RES_INVALID_PARAM);
+			return;
 		}
+
+		/* 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;

chip/lm4/mock_lpc.c

@@ -65,6 +65,13 @@ 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;

chip/stm32/i2c.c

@@ -122,14 +122,14 @@ static int i2c_write_raw(int port, void *buf, int len)
 	return len;
 }
 
-static void i2c_send_response(struct host_cmd_handler_args *args)
+void host_send_response(enum ec_status result)
 {
-	const uint8_t *data = args->response;
-	int size = args->response_size;
+	const uint8_t *data = host_cmd_args.response;
+	int size = host_cmd_args.response_size;
 	uint8_t *out = host_buffer;
 	int sum, i;
 
-	*out++ = args->result;
+	*out++ = result;
 	for (i = 0, sum = 0; i < size; i++, data++, out++) {
 		if (data != out)
 			*out = *data;
@@ -176,9 +176,6 @@ 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;

chip/stm32/spi.c

@@ -147,11 +147,6 @@ 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.
  *
@@ -192,8 +187,6 @@ 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;

common/host_command.c

@@ -49,19 +49,15 @@ 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 */
-		args->result = EC_RES_ERROR;
+		host_send_response(EC_RES_ERROR);
+		return;
 	}
 
-	/* 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;
+	/* 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);
 }
 
 /*
@@ -228,9 +224,8 @@ void host_command_task(void)
 		int evt = task_wait_event(-1);
 		/* process it */
 		if ((evt & TASK_EVENT_CMD_PENDING) && pending_args) {
-			pending_args->result =
-					host_command_process(pending_args);
-			pending_args->send_response(pending_args);
+			enum ec_status res = host_command_process(pending_args);
+			host_send_response(res);
 		}
 	}
 }

common/system_common.c

@@ -795,8 +795,7 @@ int host_command_reboot(struct host_cmd_handler_args *args)
 
 #ifdef CONFIG_TASK_HOSTCMD
 	/* Clean busy bits on host */
-	args->result = EC_RES_SUCCESS;
-	args->send_response(args);
+	host_send_response(EC_RES_SUCCESS);
 #endif
 
 	CPRINTF("[%T Executing host reboot command %d]\n", p.cmd);

include/host_command.h

@@ -13,16 +13,10 @@
 
 /* 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) */
-	uint8_t params_size;   /* Size of input parameters in bytes */
 	const uint8_t *params; /* Input parameters */
+	uint8_t params_size;   /* Size of input parameters in bytes */
 	/*
 	 * Pointer to output response data buffer.  On input to the handler,
 	 * points to a buffer of size response_max.  Command handler can change
@@ -37,17 +31,6 @@ 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 */
@@ -117,6 +100,16 @@ 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			\