From 39ab7a039676ef3738171ed00dc3c7f61802e7b3 Mon Sep 17 00:00:00 2001 From: Jonathan Brandmeyer Date: Thu, 9 Aug 2018 12:58:03 -0600 Subject: i2c: Split i2c_xfer into locked/unlocked versions. Rename i2c_xfer to i2c_xfer_unlocked. Audit all users of i2c_xfer to see if they can use simple locking semantics or require their own locking. Since locked accesses are only safe for I2C_XFER_SINGLE transactions, remove the flags parameter from i2c_xfer. This also makes the audit a bit easier. Some remaining applications hold the bus locked across several transactions that would otherwise be atomic, and a few others implement complex I2C transactions in multiple commands. Add a (nondeterministic) verification on the I2C port locking correctness. This will catch all statically incorrect locking patterns, although dynamically incorrect locking patterns may not be caught. Related: Revise the i2c_port_active_count to be a bitmap of the active ports instead of a count of the active ports. The EC's mutex does not provide an is_locked() primitive, and we would prefer not to have one. - board/glados: Custom locking for battery reset - board/mchpevb1: Custom locking for battery reset - board/oak: Custom locking for battery reset - board/samus: Custom locking for lightbar reset - board/sweetberry: simple locking - board/servo_micro: Custom locking for funky i2c transfers - capsense: simple locking - host_command_master: multi-command transactions - lb_common: manual locking to support samus power sequence - smbus: multi-command transactions - usb_i2c: simple locking - driver/tcpm/fusb302: simple locking and multi-command transactions - driver/tcpm/tcpi: Forward _unlocked and implicitly locked interface to fusb302 - driver/touchpad_elan: simple locking BUG=chromium:871851 BRANCH=none TEST=buildall; very careful audit TEST=grunt clamshell and Coral clamshell, test boot, battery charging, PD communication, and TCPC port low-power mode. Signed-off-by: Jonathan Brandmeyer Change-Id: Ieabf22bcab42780bdb994fca3ced5d8c62519d56 Reviewed-on: https://chromium-review.googlesource.com/1169913 Commit-Ready: Jonathan Brandmeyer Tested-by: Jonathan Brandmeyer Reviewed-by: Randall Spangler Reviewed-by: Jett Rink --- driver/tcpm/fusb302.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'driver/tcpm/fusb302.c') diff --git a/driver/tcpm/fusb302.c b/driver/tcpm/fusb302.c index f89f47896e..da5d32e206 100644 --- a/driver/tcpm/fusb302.c +++ b/driver/tcpm/fusb302.c @@ -322,9 +322,7 @@ static int fusb302_send_message(int port, uint16_t header, const uint32_t *data, buf[buf_pos++] = FUSB302_TKN_TXON; /* burst write for speed! */ - tcpc_lock(port, 1); - rv = tcpc_xfer(port, buf, buf_pos, 0, 0, I2C_XFER_SINGLE); - tcpc_lock(port, 0); + rv = tcpc_xfer(port, buf, buf_pos, 0, 0); return rv; } @@ -727,7 +725,7 @@ static int fusb302_tcpm_get_message(int port, uint32_t *payload, int *head) * PART 1 OF BURST READ: Write in register address. * Issue a START, no STOP. */ - rv = tcpc_xfer(port, buf, 1, 0, 0, I2C_XFER_START); + rv = tcpc_xfer_unlocked(port, buf, 1, 0, 0, I2C_XFER_START); /* * PART 2 OF BURST READ: Read up to the header. @@ -736,7 +734,7 @@ static int fusb302_tcpm_get_message(int port, uint32_t *payload, int *head) * and determine how many more bytes we need to read. * TODO: Check token to ensure valid packet. */ - rv |= tcpc_xfer(port, 0, 0, buf, 3, I2C_XFER_START); + rv |= tcpc_xfer_unlocked(port, 0, 0, buf, 3, I2C_XFER_START); /* Grab the header */ *head = (buf[1] & 0xFF); @@ -750,7 +748,7 @@ static int fusb302_tcpm_get_message(int port, uint32_t *payload, int *head) * No START, but do issue a STOP at the end. * add 4 to len to read CRC out */ - rv |= tcpc_xfer(port, 0, 0, buf, len+4, I2C_XFER_STOP); + rv |= tcpc_xfer_unlocked(port, 0, 0, buf, len+4, I2C_XFER_STOP); tcpc_lock(port, 0); } while (!rv && PACKET_IS_GOOD_CRC(*head) && -- cgit v1.2.1