YH Lin has uploaded this change for review.

View Change

google_chromeec_command: makes the error status consistent

Currently various google_chromeec_command() implementation the error
can be returned differently (LPC: -1 for v3, 1 for v1, SPI: -1/1,
I2C: 1). This patch attempts to make it consistent across all
implementations by using only negative number as error.

All direct/indirect callers of google_chromeec_command() have also
been modified accordingly as well.

BRANCH=None
BUG=chromium:935038

Signed-off-by: YH Lin <yueherngl@google.com>
Change-Id: I39f12502bc6982670dd200dcef0724e7eed2ed29
---
M src/ec/google/chromeec/crosec_proto.c
M src/ec/google/chromeec/ec.c
M src/ec/google/chromeec/ec.h
M src/ec/google/chromeec/ec_i2c.c
M src/ec/google/chromeec/ec_lpc.c
M src/ec/google/chromeec/smihandler.c
M src/ec/google/chromeec/vboot_storage.c
M src/ec/google/chromeec/vstore.c
M src/mainboard/google/gru/mainboard.c
M src/security/vboot/vboot_logic.c
10 files changed, 35 insertions(+), 31 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/26/31626/1
diff --git a/src/ec/google/chromeec/crosec_proto.c b/src/ec/google/chromeec/crosec_proto.c
index 2043347..603f1df 100644
--- a/src/ec/google/chromeec/crosec_proto.c
+++ b/src/ec/google/chromeec/crosec_proto.c
@@ -246,7 +246,7 @@
int rv = send_command_proto3(cec_command, crosec_io, context);
if (rv < 0) {
cec_command->cmd_code = rv;
- return 1;
+ return -1;
}
return 0;
}
diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c
index 09357b0..897dceb 100644
--- a/src/ec/google/chromeec/ec.c
+++ b/src/ec/google/chromeec/ec.c
@@ -363,7 +363,7 @@
cmd.cmd_size_out = sizeof(rsp);
cmd.cmd_dev_index = 0;

- if (google_chromeec_command(&cmd) == 0)
+ if (google_chromeec_command(&cmd) >= 0)
return rsp.event_mask;
return 0;
}
@@ -404,7 +404,7 @@
cmd.cmd_size_out = sizeof(rsp);
cmd.cmd_dev_index = 0;

- if (google_chromeec_command(&cmd) == 0)
+ if (google_chromeec_command(&cmd) >= 0)
return rsp.event_mask;
return 0;
}
@@ -507,7 +507,7 @@
cmd.cmd_size_out = sizeof(r);
cmd.cmd_dev_index = 0;

- if (google_chromeec_command(&cmd) != 0)
+ if (google_chromeec_command(&cmd) < 0)
return -1;

if (feature >= 8 * sizeof(r.flags))
@@ -531,7 +531,7 @@
cmd.cmd_size_out = 0;
cmd.cmd_dev_index = 0;

- if (google_chromeec_command(&cmd) != 0)
+ if (google_chromeec_command(&cmd) < 0)
return -1;

return 0;
@@ -550,7 +550,7 @@
cmd.cmd_size_out = sizeof(r);
cmd.cmd_dev_index = 0;

- if (google_chromeec_command(&cmd) != 0)
+ if (google_chromeec_command(&cmd) < 0)
return -1;

return rtc_to_tm(r.time, time);
@@ -658,7 +658,7 @@
cmd.cmd_data_out = &board_v;
cmd.cmd_dev_index = 0;

- if (google_chromeec_command(&cmd))
+ if (google_chromeec_command(&cmd) < 0)
return -1;

*version = board_v.board_version;
@@ -677,7 +677,7 @@
cmd.cmd_data_out = &sku_v;
cmd.cmd_dev_index = 0;

- if (google_chromeec_command(&cmd) != 0)
+ if (google_chromeec_command(&cmd) < 0)
return 0;

return sku_v.sku_id;
@@ -708,7 +708,7 @@
if (!is_read)
memcpy(&cmd_vbnvcontext.block, data, EC_VBNV_BLOCK_SIZE);

- if (google_chromeec_command(&cec_cmd)) {
+ if (google_chromeec_command(&cec_cmd) < 0) {
printk(BIOS_ERR, "ERROR: failed to %s vbnv_ec context: %d\n",
is_read ? "read" : "write", (int)cec_cmd.cmd_code);
mdelay(10); /* just in case */
@@ -792,7 +792,7 @@
cmd.cmd_size_out = sizeof(*r) + read_len;
cmd.cmd_dev_index = 0;
rv = google_chromeec_command(&cmd);
- if (rv != 0)
+ if (rv < 0)
return rv;

/* Parse response */
@@ -875,7 +875,7 @@
};
struct usb_chg_measures m;
int rv = google_chromeec_command(&cmd);
- if (rv != 0)
+ if (rv < 0)
return rv;
/* values are given in milliAmps and milliVolts */
*type = rsp.type;
diff --git a/src/ec/google/chromeec/ec.h b/src/ec/google/chromeec/ec.h
index 7a38336..51124f6 100644
--- a/src/ec/google/chromeec/ec.h
+++ b/src/ec/google/chromeec/ec.h
@@ -108,7 +108,7 @@
*
* @param type charger type
* @param max_watts charger max wattage
- * @return non-zero for error, otherwise 0.
+ * @return 0 on success or negative integer for errors.
*/
int google_chromeec_get_usb_pd_power_info(enum usb_chg_type *type,
u32 *max_watts);
@@ -118,7 +118,7 @@
*
* @param current_lim Max current in mA
* @param voltage_lim Max voltage in mV
- * @return non-zero for error, otherwise 0.
+ * @return 0 on success or negative integer for errors.
*/
int google_chromeec_override_dedicated_charger_limit(u16 current_lim,
u16 voltage_lim);
@@ -150,6 +150,8 @@
* The lower level transport works on the buffers handed out to the
* upper level. Therefore, only the size of the request and response
* are required.
+ *
+ * @return 0 on success or negative integer for errors.
*/
typedef int (*crosec_io_t)(size_t req_size, size_t resp_size, void *context);
int crosec_command_proto(struct chromeec_command *cec_command,
@@ -159,7 +161,7 @@
* Send a command to a CrOS EC
*
* @param cec_command: CrOS EC command to send
- * @return 0 for success. Non-zero for error.
+ * @return 0 on success or negative integer for errors.
*/
int google_chromeec_command(struct chromeec_command *cec_command);

diff --git a/src/ec/google/chromeec/ec_i2c.c b/src/ec/google/chromeec/ec_i2c.c
index 22b491a..7f2ead5 100644
--- a/src/ec/google/chromeec/ec_i2c.c
+++ b/src/ec/google/chromeec/ec_i2c.c
@@ -205,7 +205,7 @@
__func__, cec_command->cmd_size_in,
cec_command->cmd_size_out);
cec_command->cmd_code = EC_RES_INVALID_PARAM;
- return 1;
+ return -1;
}

/* Construct command. */
@@ -221,13 +221,13 @@
printk(BIOS_ERR, "%s: Cannot complete write to i2c-%d:%#x\n",
__func__, bus, chip);
cec_command->cmd_code = EC_RES_ERROR;
- return 1;
+ return -1;
}
if (i2c_read_raw(bus, chip, (uint8_t *)&resp, size_i2c_resp) != 0) {
printk(BIOS_ERR, "%s: Cannot complete read from i2c-%d:%#x\n",
__func__, bus, chip);
cec_command->cmd_code = EC_RES_ERROR;
- return 1;
+ return -1;
}

/* Verify and return response */
@@ -235,17 +235,17 @@
if (resp.response != EC_RES_SUCCESS) {
printk(BIOS_DEBUG, "%s: Received bad result code %d\n",
__func__, (int)resp.response);
- return 1;
+ return -1;
}
if (resp.length > cec_command->cmd_size_out) {
printk(BIOS_ERR, "%s: Received len %#02x too large\n",
__func__, (int)resp.length);
cec_command->cmd_code = EC_RES_INVALID_RESPONSE;
- return 1;
+ return -1;
}
if (!ec_verify_checksum(&resp)) {
cec_command->cmd_code = EC_RES_INVALID_CHECKSUM;
- return 1;
+ return -1;
}
cec_command->cmd_size_out = resp.length;
memcpy(cec_command->cmd_data_out, resp.data, resp.length);
diff --git a/src/ec/google/chromeec/ec_lpc.c b/src/ec/google/chromeec/ec_lpc.c
index 61005d3..097b1b4 100644
--- a/src/ec/google/chromeec/ec_lpc.c
+++ b/src/ec/google/chromeec/ec_lpc.c
@@ -330,13 +330,13 @@
if (google_chromeec_wait_ready(EC_LPC_ADDR_HOST_CMD)) {
printk(BIOS_ERR, "Timeout waiting for EC process command %d!\n",
cec_command->cmd_code);
- return 1;
+ return -1;
}

/* Check result */
cec_command->cmd_code = read_byte(EC_LPC_ADDR_HOST_DATA);
if (cec_command->cmd_code)
- return 1;
+ return -1;

/* Read back args */
read_bytes(EC_LPC_ADDR_HOST_ARGS, sizeof(args), (u8*)&args, NULL);
@@ -348,12 +348,12 @@
*/
if (!(args.flags & EC_HOST_ARGS_FLAG_TO_HOST)) {
printk(BIOS_ERR, "EC protocol mismatch\n");
- return 1;
+ return -1;
}

if (args.data_size > cec_command->cmd_size_out) {
printk(BIOS_ERR, "EC returned too much data\n");
- return 1;
+ return -1;
}
cec_command->cmd_size_out = args.data_size;

@@ -369,7 +369,7 @@
/* Verify checksum */
if (args.checksum != csum) {
printk(BIOS_ERR, "EC response has invalid checksum\n");
- return 1;
+ return -1;
}

return 0;
diff --git a/src/ec/google/chromeec/smihandler.c b/src/ec/google/chromeec/smihandler.c
index cab7192..3f703f9 100644
--- a/src/ec/google/chromeec/smihandler.c
+++ b/src/ec/google/chromeec/smihandler.c
@@ -54,7 +54,7 @@
;

printk(BIOS_DEBUG,"Clearing pending EC events. Error code 1 is expected.\n");
- while (google_chromeec_get_mkbp_event(&mkbp_event) == 0)
+ while (google_chromeec_get_mkbp_event(&mkbp_event) >= 0)
;
}

diff --git a/src/ec/google/chromeec/vboot_storage.c b/src/ec/google/chromeec/vboot_storage.c
index f47c2f1..08af2f6 100644
--- a/src/ec/google/chromeec/vboot_storage.c
+++ b/src/ec/google/chromeec/vboot_storage.c
@@ -30,7 +30,7 @@
/* Ensure the digests being saved match the EC's slot size. */
assert(digest_size == EC_VSTORE_SLOT_SIZE);

- if (google_chromeec_vstore_write(slot, digest, digest_size))
+ if (google_chromeec_vstore_write(slot, digest, digest_size) < 0)
return -1;

/* Assert the slot is locked on successful write. */
diff --git a/src/ec/google/chromeec/vstore.c b/src/ec/google/chromeec/vstore.c
index 28c2603..39e59f8 100644
--- a/src/ec/google/chromeec/vstore.c
+++ b/src/ec/google/chromeec/vstore.c
@@ -42,7 +42,7 @@
.cmd_data_out = &info,
};

- if (google_chromeec_command(&cmd) != 0)
+ if (google_chromeec_command(&cmd) < 0)
return 0;

if (locked)
diff --git a/src/mainboard/google/gru/mainboard.c b/src/mainboard/google/gru/mainboard.c
index 41307c7..245a70e 100644
--- a/src/mainboard/google/gru/mainboard.c
+++ b/src/mainboard/google/gru/mainboard.c
@@ -257,12 +257,14 @@

static void usb_power_cycle(int port)
{
- if (google_chromeec_set_usb_pd_role(port, USB_PD_CTRL_ROLE_FORCE_SINK))
+ if (google_chromeec_set_usb_pd_role(port, USB_PD_CTRL_ROLE_FORCE_SINK)
+ < 0)
printk(BIOS_ERR, "ERROR: Cannot force USB%d PD sink\n", port);

mdelay(10); /* Make sure USB stick is fully depowered. */

- if (google_chromeec_set_usb_pd_role(port, USB_PD_CTRL_ROLE_TOGGLE_ON))
+ if (google_chromeec_set_usb_pd_role(port, USB_PD_CTRL_ROLE_TOGGLE_ON)
+ < 0)
printk(BIOS_ERR, "ERROR: Cannot restore USB%d PD mode\n", port);
}

diff --git a/src/security/vboot/vboot_logic.c b/src/security/vboot/vboot_logic.c
index 00bbae6..c379463 100644
--- a/src/security/vboot/vboot_logic.c
+++ b/src/security/vboot/vboot_logic.c
@@ -141,7 +141,7 @@

printk(BIOS_DEBUG, "Platform is resuming.\n");

- if (vboot_retrieve_hash(saved_hash, saved_hash_sz)) {
+ if (vboot_retrieve_hash(saved_hash, saved_hash_sz) < 0) {
printk(BIOS_ERR, "Couldn't retrieve saved hash.\n");
return -1;
}

To view, visit change 31626. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I39f12502bc6982670dd200dcef0724e7eed2ed29
Gerrit-Change-Number: 31626
Gerrit-PatchSet: 1
Gerrit-Owner: YH Lin <yueherngl@google.com>
Gerrit-MessageType: newchange