From 949d525e2a4de87da034f7fd89c6cb0f04d8f1c4 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sun, 15 Jul 2012 02:30:59 +0100 Subject: 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 Reviewed-on: https://gerrit.chromium.org/gerrit/27468 --- chip/lm4/lpc.c | 51 +++++++++++++++++++++++++------------------------- chip/lm4/mock_lpc.c | 7 ------- chip/stm32/i2c.c | 11 +++++++---- chip/stm32/spi.c | 7 +++++++ common/host_command.c | 21 +++++++++++++-------- common/system_common.c | 3 ++- include/host_command.h | 29 +++++++++++++++++----------- 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 \ -- cgit v1.2.1