diff options
author | Daisuke Nojiri <dnojiri@chromium.org> | 2019-01-02 15:27:19 -0800 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2019-01-05 20:08:41 -0800 |
commit | aed008f87c3c880edecf7608ab24eaa4bee1bc46 (patch) | |
tree | ace789e46f1a72c4da399760b72eb281424000f7 | |
parent | ef15468753984a9d4ee71ea1c73fc7899e977802 (diff) | |
download | chrome-ec-aed008f87c3c880edecf7608ab24eaa4bee1bc46.tar.gz |
ectool: Don't acquire lock when dev interface is used
The /dev/cros_ec interface has a built-in mutex, thus we do not
need to use /run/lock to arbitrate access since we can assume other
tools (mosys, flashrom) also use the dev interface.
$ generate_logs
...
feedback/cbi_info
...
$ cat feedback/cbi_info
[0]
As integer: 1 (0x1)
As binary: 01 02
[1]
As integer: 3 (0x3)
As binary: 03
[2]
As integer: 103 (0x67)
As binary: 67 3a
Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org>
BUG=chromium:849399
BRANCH=none
TEST=Verify 'ectool version' runs on Nami. Verify lock is acquired
when '--interface=lpc' is specified. Verify debugd can run cbi_info.
Change-Id: Id94317472917a974218bb137bda11fe5618a4b88
Reviewed-on: https://chromium-review.googlesource.com/1393729
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>
-rw-r--r-- | util/comm-host.c | 56 | ||||
-rw-r--r-- | util/comm-host.h | 18 | ||||
-rw-r--r-- | util/ec_sb_firmware_update.c | 13 | ||||
-rw-r--r-- | util/ectool.c | 18 |
4 files changed, 60 insertions, 45 deletions
diff --git a/util/comm-host.c b/util/comm-host.c index e459bdfaa8..e9e7fb1f5f 100644 --- a/util/comm-host.c +++ b/util/comm-host.c @@ -82,10 +82,33 @@ int ec_command(int command, int version, indata, insize); } -int comm_init(int interfaces, const char *device_name) +int comm_init_alt(int interfaces, const char *device_name) +{ + if ((interfaces & COMM_SERVO) && comm_init_servo_spi && + !comm_init_servo_spi(device_name)) + return 0; + + /* Do not fallback to other communication methods if target is not a + * cros_ec device */ + if (!strcmp(CROS_EC_DEV_NAME, device_name)) { + /* Fallback to direct LPC on x86 */ + if ((interfaces & COMM_LPC) && !comm_init_lpc()) + return 0; + + /* Fallback to direct i2c on ARM */ + if ((interfaces & COMM_I2C) && !comm_init_i2c()) + return 0; + } + + /* Give up */ + fprintf(stderr, "Unable to establish host communication\n"); + return 1; +} + +int comm_init_buffer(void) { - struct ec_response_get_protocol_info info; int allow_large_buffer; + struct ec_response_get_protocol_info info; /* Default memmap access */ ec_readmem = fake_readmem; @@ -96,34 +119,6 @@ int comm_init(int interfaces, const char *device_name) return 1; } - /* Prefer new /dev method */ - if ((interfaces & COMM_DEV) && comm_init_dev && - !comm_init_dev(device_name)) - goto init_ok; - - if ((interfaces & COMM_SERVO) && comm_init_servo_spi && - !comm_init_servo_spi(device_name)) - goto init_ok; - - /* Do not fallback to other communication methods if target is not a - * cros_ec device */ - if (strcmp(CROS_EC_DEV_NAME, device_name)) - goto init_failed; - - /* Fallback to direct LPC on x86 */ - if ((interfaces & COMM_LPC) && comm_init_lpc && !comm_init_lpc()) - goto init_ok; - - /* Fallback to direct i2c on ARM */ - if ((interfaces & COMM_I2C) && comm_init_i2c && !comm_init_i2c()) - goto init_ok; - - init_failed: - /* Give up */ - fprintf(stderr, "Unable to establish host communication\n"); - return 1; - - init_ok: /* Allocate shared I/O buffers */ ec_outbuf = malloc(ec_max_outsize); ec_inbuf = malloc(ec_max_insize); @@ -154,5 +149,4 @@ int comm_init(int interfaces, const char *device_name) } return 0; - } diff --git a/util/comm-host.h b/util/comm-host.h index bf6921d17d..11f81b4950 100644 --- a/util/comm-host.h +++ b/util/comm-host.h @@ -35,13 +35,27 @@ enum comm_interface { }; /** - * Perform initializations needed for subsequent requests + * Initialize alternative interfaces * * @param interfaces Interfaces to try; use COMM_ALL to try all of them. * @param device_name For DEV option, the device file to use. * @return 0 in case of success, or error code. */ -int comm_init(int interfaces, const char *device_name); +int comm_init_alt(int interfaces, const char *device_name); + +/** + * Initialize dev interface + * + * @return 0 in case of success, or error code. + */ +int comm_init_dev(const char *device_name); + +/** + * Initialize input & output buffers + * + * @return 0 in case of success, or error code. + */ +int comm_init_buffer(void); /** * Send a command to the EC. Returns the length of output data returned (0 if diff --git a/util/ec_sb_firmware_update.c b/util/ec_sb_firmware_update.c index 54b7ff1567..09067b6928 100644 --- a/util/ec_sb_firmware_update.c +++ b/util/ec_sb_firmware_update.c @@ -748,7 +748,7 @@ void usage(char *argv[]) int main(int argc, char *argv[]) { - int rv = 0, interfaces = COMM_ALL; + int rv = 0; int op = OP_UNKNOWN; uint8_t val = 0; @@ -767,14 +767,14 @@ int main(int argc, char *argv[]) return -1; } - if (acquire_gec_lock(GEC_LOCK_TIMEOUT_SECS) < 0) { - printf("Could not acquire GEC lock.\n"); + if (comm_init_dev(NULL)) { + printf("Couldn't initialize /dev.\n"); return -1; } - if (comm_init(interfaces, NULL)) { - printf("Couldn't find EC\n"); - goto out; + if (comm_init_buffer()) { + fprintf(stderr, "Couldn't initialize buffers\n"); + return -1; } fw_update.flags = 0; @@ -836,7 +836,6 @@ int main(int argc, char *argv[]) if (fw_update.flags & F_POWERD_DISABLED) rv |= restore_power_management(); out: - release_gec_lock(); if (rv) return -1; else diff --git a/util/ectool.c b/util/ectool.c index 5eadc99bfb..a25b0d5294 100644 --- a/util/ectool.c +++ b/util/ectool.c @@ -8600,13 +8600,21 @@ int main(int argc, char *argv[]) exit(1); } - if (acquire_gec_lock(GEC_LOCK_TIMEOUT_SECS) < 0) { - fprintf(stderr, "Could not acquire GEC lock.\n"); - exit(1); + /* Prefer /dev method, which supports built-in mutex */ + if (!(interfaces & COMM_DEV) || comm_init_dev(device_name)) { + /* If dev is excluded or isn't supported, find alternative */ + if (acquire_gec_lock(GEC_LOCK_TIMEOUT_SECS) < 0) { + fprintf(stderr, "Could not acquire GEC lock.\n"); + exit(1); + } + if (comm_init_alt(interfaces, device_name)) { + fprintf(stderr, "Couldn't find EC\n"); + goto out; + } } - if (comm_init(interfaces, device_name)) { - fprintf(stderr, "Couldn't find EC\n"); + if (comm_init_buffer()) { + fprintf(stderr, "Couldn't initialize buffers\n"); goto out; } |