summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDuncan Laurie <dlaurie@chromium.org>2013-08-09 13:45:41 -0700
committerChromeBot <chrome-bot@google.com>2013-08-09 13:52:00 -0700
commite8b1473f7bde1fc40e2ea185a8378e55aa4c2b6f (patch)
tree8f36bc3cb5db4d1aeed545bf96a94de3732f06ca
parent3cd5e4cf7d6a5f5ffe55700e0d0fc6895c91bb5c (diff)
downloadchrome-ec-e8b1473f7bde1fc40e2ea185a8378e55aa4c2b6f.tar.gz
Host commands don't set the response pointer
Remove support for allowing host command handlers to set the response pointer. This is just one more thing that can break (and did, on SPI protocol V2). No commands other than the trivial read-memory-map command in host_command.c and flash read made use of this capability, and the savings in performance was negligible. BUG=chrome-os-partner:21576 BRANCH=none TEST=boot pit; still boots Original-Change-Id: I0a9bcf57dbea7155318fc389d7b47d3742a1a00a Signed-off-by: Randall Spangler <rspangler@chromium.org> Reviewed-on: https://gerrit.chromium.org/gerrit/64236 (cherry picked from commit c6dfa7e03b4c672e1031b4af9230b9da77e50f1a) Change-Id: Ib14042190989aa32eb32f024d9b616baaf339231 Signed-off-by: Duncan Laurie <dlaurie@chromium.org> Reviewed-on: https://gerrit.chromium.org/gerrit/65367
-rw-r--r--chip/lm4/lpc.c2
-rw-r--r--chip/stm32/spi.c6
-rw-r--r--common/flash_common.c8
-rw-r--r--common/host_command.c21
-rw-r--r--common/test_util.c8
-rw-r--r--include/host_command.h16
6 files changed, 20 insertions, 41 deletions
diff --git a/chip/lm4/lpc.c b/chip/lm4/lpc.c
index 7712d13cd4..c3cc908ed7 100644
--- a/chip/lm4/lpc.c
+++ b/chip/lm4/lpc.c
@@ -204,8 +204,6 @@ static void lpc_send_response(struct host_cmd_handler_args *args)
/* Fail if response doesn't fit in the param buffer */
if (size > EC_PROTO2_MAX_PARAM_SIZE)
args->result = EC_RES_INVALID_RESPONSE;
- else if (host_cmd_args.response != cmd_params)
- memcpy(cmd_params, args->response, size);
/*
* Write result to the data byte. This sets the TOH bit in the
diff --git a/chip/stm32/spi.c b/chip/stm32/spi.c
index bb2bfaafe6..07285e27ef 100644
--- a/chip/stm32/spi.c
+++ b/chip/stm32/spi.c
@@ -238,13 +238,9 @@ static void spi_send_response(struct host_cmd_handler_args *args)
if (!active)
return;
- if (args->response_size > EC_PROTO2_MAX_PARAM_SIZE)
+ if (args->response_size > args->response_max)
result = EC_RES_INVALID_RESPONSE;
- if ((uint8_t *)args->response >= out_msg &&
- (uint8_t *)args->response < out_msg + sizeof(out_msg))
- ASSERT(args->response == out_msg + SPI_PROTO2_OFFSET);
-
/* Transmit the reply */
txdma = dma_get_channel(STM32_DMAC_SPI1_TX);
reply(txdma, result, args->response, args->response_size);
diff --git a/common/flash_common.c b/common/flash_common.c
index 6f79f544a0..082f64fb75 100644
--- a/common/flash_common.c
+++ b/common/flash_common.c
@@ -522,11 +522,15 @@ DECLARE_HOST_COMMAND(EC_CMD_FLASH_INFO,
static int flash_command_read(struct host_cmd_handler_args *args)
{
const struct ec_params_flash_read *p = args->params;
+ const char *src;
- if (flash_dataptr(p->offset, p->size, 1,
- (const char **)&args->response) < 0)
+ if (flash_dataptr(p->offset, p->size, 1, &src) < 0)
return EC_RES_ERROR;
+ if (p->size > args->response_max)
+ return EC_RES_OVERFLOW;
+
+ memcpy(args->response, src, p->size);
args->response_size = p->size;
return EC_RES_SUCCESS;
diff --git a/common/host_command.c b/common/host_command.c
index 489ae7b35f..55f27d9199 100644
--- a/common/host_command.c
+++ b/common/host_command.c
@@ -182,22 +182,9 @@ void host_packet_respond(struct host_cmd_handler_args *args)
for (i = sizeof(*r); i > 0; i--)
csum += *out++;
- /* Checksum and copy response data, if any */
- if (!args->response_size) {
- /* No data to copy */
- } else if (args->response != out) {
- /* Copy and checksum */
- const uint8_t *outr = (const uint8_t *)args->response;
-
- for (i = args->response_size; i > 0; i--) {
- *out = *outr++;
- csum += *out++;
- }
- } else {
- /* Response already in right place; just checksum it */
- for (i = args->response_size; i > 0; i--)
- csum += *out++;
- }
+ /* Checksum response data, if any */
+ for (i = args->response_size; i > 0; i--)
+ csum += *out++;
/* Write checksum field so the entire packet sums to 0 */
r->checksum = (uint8_t)(-csum);
@@ -451,7 +438,7 @@ static int host_command_read_memmap(struct host_cmd_handler_args *args)
offset + size > EC_MEMMAP_SIZE)
return EC_RES_INVALID_PARAM;
- args->response = host_get_memmap(offset);
+ memcpy(args->response, host_get_memmap(offset), size);
args->response_size = size;
return EC_RES_SUCCESS;
diff --git a/common/test_util.c b/common/test_util.c
index 10e718399f..c133ac9d24 100644
--- a/common/test_util.c
+++ b/common/test_util.c
@@ -70,7 +70,6 @@ int test_send_host_command(int command, int version, const void *params,
int params_size, void *resp, int resp_size)
{
struct host_cmd_handler_args args;
- int rv;
args.version = version;
args.command = command;
@@ -80,12 +79,7 @@ int test_send_host_command(int command, int version, const void *params,
args.response_max = resp_size;
args.response_size = 0;
- rv = host_command_process(&args);
-
- if (args.response != resp)
- memcpy(resp, args.response, args.response_size);
-
- return rv;
+ return host_command_process(&args);
}
#endif /* TASK_HAS_HOSTCMD */
diff --git a/include/host_command.h b/include/host_command.h
index 53e088e2e0..96096bc699 100644
--- a/include/host_command.h
+++ b/include/host_command.h
@@ -27,18 +27,18 @@ struct host_cmd_handler_args {
/*
* Pointer to output response data buffer. On input to the handler,
- * points to a buffer of size response_max. Command handler can change
- * this to point to a different location instead of memcpy()'ing data
- * into the provided buffer.
+ * points to a buffer of size response_max.
*/
void *response;
+
+ /* Maximum size of response buffer provided to command handler */
+ uint16_t response_max;
+
/*
- * Maximum size of response buffer provided to command handler. If the
- * handler changes response to point to its own larger buffer, it may
- * return a response_size greater than response_max.
+ * Size of data pointed to by response. Defaults to 0, so commands
+ * which do not produce response data do not need to set this.
*/
- uint16_t response_max;
- uint16_t response_size; /* Size of data pointed to by response */
+ uint16_t response_size;
/*
* This is the result returned by command and therefore the status to