diff options
author | Jack Rosenthal <jrosenth@chromium.org> | 2019-07-11 18:17:34 -0600 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2019-07-17 21:45:18 +0000 |
commit | f8d6179a26bf512c43638d0916fde0fc966cc3fb (patch) | |
tree | d4df87ff4a738060d20f874eaa75cc3b2d24cdb9 | |
parent | 21a255ea953e8ac64d05147ad7f11491db126cf4 (diff) | |
download | chrome-ec-f8d6179a26bf512c43638d0916fde0fc966cc3fb.tar.gz |
common: add i2c tracing functionality
crbug.com/982442 requests a way for developers to enable tracing of
i2c commands when debugging. This adds a new debug feature, the
i2ctrace command, which provides that. The command is guarded by
CONFIG_I2C_DEBUG.
BUG=chromium:982442
BRANCH=none
TEST=enabled CONFIG_I2C_DEBUG on arcada_ish, made sure that command
functioned as it says on the tin
Change-Id: I9c762271237cbf131e5ef7c0f605c89af4f209fd
Signed-off-by: Jack Rosenthal <jrosenth@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1699347
Reviewed-by: Denis Brockus <dbrockus@chromium.org>
Reviewed-by: Jett Rink <jettrink@chromium.org>
Commit-Queue: Denis Brockus <dbrockus@chromium.org>
-rw-r--r-- | common/build.mk | 1 | ||||
-rw-r--r-- | common/i2c_master.c | 16 | ||||
-rw-r--r-- | common/i2c_trace.c | 201 | ||||
-rw-r--r-- | docs/i2c-debugging.md | 48 | ||||
-rw-r--r-- | include/i2c.h | 15 |
5 files changed, 275 insertions, 6 deletions
diff --git a/common/build.mk b/common/build.mk index 61f64fca35..a7b93361a3 100644 --- a/common/build.mk +++ b/common/build.mk @@ -77,6 +77,7 @@ common-$(CONFIG_GESTURE_SW_DETECTION)+=gesture.o common-$(CONFIG_HOSTCMD_EVENTS)+=host_event_commands.o common-$(CONFIG_HOSTCMD_PD)+=host_command_master.o common-$(CONFIG_HOSTCMD_RTC)+=rtc.o +common-$(CONFIG_I2C_DEBUG)+=i2c_trace.o common-$(CONFIG_I2C_MASTER)+=i2c_master.o common-$(CONFIG_I2C_SLAVE)+=i2c_slave.o common-$(CONFIG_I2C_VIRTUAL_BATTERY)+=virtual_battery.o diff --git a/common/i2c_master.c b/common/i2c_master.c index 79ee897d25..2efd6e9450 100644 --- a/common/i2c_master.c +++ b/common/i2c_master.c @@ -76,16 +76,20 @@ static int chip_i2c_xfer_with_notify(int port, int slave_addr, { int ret; -#ifdef CONFIG_I2C_XFER_BOARD_CALLBACK - i2c_start_xfer_notify(port, slave_addr); -#endif + if (IS_ENABLED(CONFIG_I2C_DEBUG)) + i2c_trace_notify(port, slave_addr, 0, out, out_size); + + if (IS_ENABLED(CONFIG_I2C_XFER_BOARD_CALLBACK)) + i2c_start_xfer_notify(port, slave_addr); ret = chip_i2c_xfer(port, slave_addr, out, out_size, in, in_size, flags); -#ifdef CONFIG_I2C_XFER_BOARD_CALLBACK - i2c_end_xfer_notify(port, slave_addr); -#endif + if (IS_ENABLED(CONFIG_I2C_XFER_BOARD_CALLBACK)) + i2c_end_xfer_notify(port, slave_addr); + + if (IS_ENABLED(CONFIG_I2C_DEBUG)) + i2c_trace_notify(port, slave_addr, 1, in, in_size); return ret; } diff --git a/common/i2c_trace.c b/common/i2c_trace.c new file mode 100644 index 0000000000..52603c7244 --- /dev/null +++ b/common/i2c_trace.c @@ -0,0 +1,201 @@ +/* Copyright 2019 The Chromium OS Authors. All rights reserved. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "common.h" +#include "console.h" +#include "i2c.h" +#include "stddef.h" +#include "stdbool.h" +#include "util.h" + +#define CPUTS(outstr) cputs(CC_I2C, outstr) +#define CPRINTS(format, args...) cprints(CC_I2C, format, ## args) +#define CPRINTF(format, args...) cprintf(CC_I2C, format, ## args) + +struct i2c_trace_range { + bool enabled; + int port; + int slave_addr_lo; /* Inclusive */ + int slave_addr_hi; /* Inclusive */ +}; + +static struct i2c_trace_range trace_entries[8]; + +void i2c_trace_notify(int port, int slave_addr, int direction, + const uint8_t *data, size_t size) +{ + size_t i; + + for (i = 0; i < ARRAY_SIZE(trace_entries); i++) + if (trace_entries[i].enabled + && trace_entries[i].port == port + && trace_entries[i].slave_addr_lo <= slave_addr + && trace_entries[i].slave_addr_hi >= slave_addr) + goto trace_enabled; + return; + +trace_enabled: + CPRINTF("i2c: %s %d:0x%X ", + direction ? "read" : "write", + port, + slave_addr); + for (i = 0; i < size; i++) + CPRINTF("%02X ", data[i]); + CPRINTF("\n"); +} + +static int command_i2ctrace_list(void) +{ + size_t i; + + ccprintf("id port address\n"); + ccprintf("-- ---- -------\n"); + + for (i = 0; i < ARRAY_SIZE(trace_entries); i++) { + if (trace_entries[i].enabled) { + ccprintf("%2d %4d 0x%X", + i, + trace_entries[i].port, + trace_entries[i].slave_addr_lo); + if (trace_entries[i].slave_addr_hi + != trace_entries[i].slave_addr_lo) + ccprintf(" to 0x%X", + trace_entries[i].slave_addr_hi); + ccprintf("\n"); + } + } + + return EC_SUCCESS; +} + +static int command_i2ctrace_disable(size_t id) +{ + if (id >= ARRAY_SIZE(trace_entries)) + return EC_ERROR_PARAM2; + + trace_entries[id].enabled = 0; + return EC_SUCCESS; +} + +static int command_i2ctrace_enable(int port, int slave_addr_lo, + int slave_addr_hi) +{ + struct i2c_trace_range *t; + struct i2c_trace_range *new_entry = NULL; + + if (port >= i2c_ports_used) + return EC_ERROR_PARAM2; + + if (slave_addr_lo > slave_addr_hi) + return EC_ERROR_PARAM3; + + /* + * Scan thru existing entries to see if there is one we can + * extend instead of making a new entry + */ + for (t = trace_entries; + t < trace_entries + ARRAY_SIZE(trace_entries); + t++) { + if (t->enabled && t->port == port) { + /* Subset of existing range, do nothing */ + if (t->slave_addr_lo <= slave_addr_lo && + t->slave_addr_hi >= slave_addr_hi) + return EC_SUCCESS; + + /* Extends exising range on both directions, replace */ + if (t->slave_addr_lo >= slave_addr_lo && + t->slave_addr_hi <= slave_addr_hi) { + t->enabled = 0; + return command_i2ctrace_enable( + port, slave_addr_lo, slave_addr_hi); + } + + /* Extends existing range below */ + if (t->slave_addr_lo - 1 <= slave_addr_hi && + t->slave_addr_hi >= slave_addr_hi) { + t->enabled = 0; + return command_i2ctrace_enable( + port, + slave_addr_lo, + t->slave_addr_hi); + } + + /* Extends existing range above */ + if (t->slave_addr_lo <= slave_addr_lo && + t->slave_addr_hi + 1 >= slave_addr_lo) { + t->enabled = 0; + return command_i2ctrace_enable( + port, + t->slave_addr_lo, + slave_addr_hi); + } + } else if (!t->enabled && !new_entry) { + new_entry = t; + } + } + + /* We need to allocate a new entry */ + if (new_entry) { + new_entry->enabled = 1; + new_entry->port = port; + new_entry->slave_addr_lo = slave_addr_lo; + new_entry->slave_addr_hi = slave_addr_hi; + return EC_SUCCESS; + } + + ccprintf("No space to allocate new trace entry. Delete some first.\n"); + return EC_ERROR_MEMORY_ALLOCATION; +} + + +static int command_i2ctrace(int argc, char **argv) +{ + int id_or_port; + int address_low; + int address_high; + char *end; + + if (argc < 2) + return EC_ERROR_PARAM_COUNT; + + if (!strcasecmp(argv[1], "list") && argc == 2) + return command_i2ctrace_list(); + + if (argc < 3) + return EC_ERROR_PARAM_COUNT; + + id_or_port = strtoi(argv[2], &end, 0); + if (*end || id_or_port < 0) + return EC_ERROR_PARAM2; + + if (!strcasecmp(argv[1], "disable") && argc == 3) + return command_i2ctrace_disable(id_or_port); + + if (!strcasecmp(argv[1], "enable")) { + address_low = strtoi(argv[3], &end, 0); + if (*end || address_low < 0) + return EC_ERROR_PARAM3; + + if (argc == 4) { + address_high = address_low; + } else if (argc == 5) { + address_high = strtoi(argv[4], &end, 0); + if (*end || address_high < 0) + return EC_ERROR_PARAM4; + } else { + return EC_ERROR_PARAM_COUNT; + } + + return command_i2ctrace_enable( + id_or_port, address_low, address_high); + } + + return EC_ERROR_PARAM1; +} +DECLARE_CONSOLE_COMMAND(i2ctrace, + command_i2ctrace, + "[list | disable <id> | enable <port> <address> | " + "enable <port> <address-low> <address-high>]", + "Trace I2C transactions"); diff --git a/docs/i2c-debugging.md b/docs/i2c-debugging.md new file mode 100644 index 0000000000..a1a56db285 --- /dev/null +++ b/docs/i2c-debugging.md @@ -0,0 +1,48 @@ +I²C Debugging Tips +================== + +The EC codebase has functionality to help you debug I²C errors without +pulling out the scope. Some of the debug functionality is disabled by +default to save space, but can be enabled with the `CONFIG_I2C_DEBUG` +option. + +Tracing +------- + +You can use the `i2ctrace` command to monitor (ranges of) addresses: + + i2ctrace [list + | disable <id> + | enable <port> <address> + | enable <port> <address-low> <address-high>] + +For example: + + > i2ctrace enable 0 0x10 0x30 + > i2ctrace enable 0 0x40 0x50 + > i2ctrace list + id port address + -- ---- ------- + 0 0 0x10 to 0x30 + 1 0 0x40 to 0x50 + ... debug spam may follow ... + i2c: write 0:0x50 A8 + i2c: read 0:0x50 00 02 00 06 00 44 + ... + > i2ctrace disable 1 + > i2ctrace list + id port address + -- ---- ------- + 0 0 0x10 to 0x30 + +A maximum of 8 debug entries are supported at a single time. + +Note that `i2ctrace enable` will merge debug entries when possible: + + > i2ctrace enable 0 0x10 0x30 + > i2ctrace enable 0 0x40 0x50 + > i2ctrace enable 0 0x31 0x3f + > i2ctrace list + id port address + -- ---- ------- + 0 0 0x10 to 0x50 diff --git a/include/i2c.h b/include/i2c.h index 057f38f2e8..566ae7b402 100644 --- a/include/i2c.h +++ b/include/i2c.h @@ -11,6 +11,7 @@ #include "common.h" #include "gpio.h" #include "host_command.h" +#include "stddef.h" /* Flags for slave address field, in addition to the 8-bit address */ #define I2C_FLAG_BIG_ENDIAN 0x100 /* 16 byte values are MSB-first */ @@ -426,4 +427,18 @@ void i2c_start_xfer_notify(int port, int slave_addr); */ void i2c_end_xfer_notify(int port, int slave_addr); +/** + * Defined in common/i2c_trace.c, used by i2c master to notify tracing + * funcionality of transactions. + * + * @param port: I2C port number + * @param slave_addr: slave device address + * @param direction: 0 for write, + * 1 for read + * @param data: pointer to data read or written + * @param size: size of data read or written + */ +void i2c_trace_notify(int port, int slave_addr, int direction, + const uint8_t *data, size_t size); + #endif /* __CROS_EC_I2C_H */ |