From ffdb92b3c3d3373f37d69d0e91880789e093644a Mon Sep 17 00:00:00 2001 From: Mary Ruthven Date: Fri, 12 Nov 2021 11:49:52 -0600 Subject: Revert "common: add i2c tracing functionality" This reverts commit f8d6179a26bf512c43638d0916fde0fc966cc3fb. BUG=b:200823466 TEST=make buildall -j Change-Id: I8e7a4bbd01bc99bedd2abf77b0d482ed679de865 Signed-off-by: Mary Ruthven Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3285753 Reviewed-by: Vadim Sukhomlinov --- common/build.mk | 1 - common/i2c_controller.c | 16 ++-- common/i2c_trace.c | 200 ------------------------------------------------ docs/i2c-debugging.md | 48 ------------ include/i2c.h | 15 ---- 5 files changed, 6 insertions(+), 274 deletions(-) delete mode 100644 common/i2c_trace.c delete mode 100644 docs/i2c-debugging.md diff --git a/common/build.mk b/common/build.mk index cd244b3bab..506e1ba928 100644 --- a/common/build.mk +++ b/common/build.mk @@ -70,7 +70,6 @@ common-$(CONFIG_FMAP)+=fmap.o common-$(CONFIG_GESTURE_SW_DETECTION)+=gesture.o common-$(CONFIG_HOSTCMD_EVENTS)+=host_event_commands.o common-$(CONFIG_HOSTCMD_RTC)+=rtc.o -common-$(CONFIG_I2C_DEBUG)+=i2c_trace.o common-$(CONFIG_I2C_CONTROLLER)+=i2c_controller.o common-$(CONFIG_I2C_PERIPH)+=i2c_peripheral.o common-$(CONFIG_INDUCTIVE_CHARGING)+=inductive_charging.o diff --git a/common/i2c_controller.c b/common/i2c_controller.c index c12627c085..933cfa8dbb 100644 --- a/common/i2c_controller.c +++ b/common/i2c_controller.c @@ -79,11 +79,9 @@ static int chip_i2c_xfer_with_notify(const int port, int ret; uint16_t addr_flags = periph_addr_flags; - if (IS_ENABLED(CONFIG_I2C_DEBUG)) - i2c_trace_notify(port, periph_addr_flags, 0, out, out_size); - - if (IS_ENABLED(CONFIG_I2C_XFER_BOARD_CALLBACK)) - i2c_start_xfer_notify(port, periph_addr_flags); +#ifdef CONFIG_I2C_XFER_BOARD_CALLBACK + i2c_start_xfer_notify(port, periph_addr_flags); +#endif if (IS_ENABLED(CONFIG_SMBUS_PEC)) /* @@ -94,11 +92,9 @@ static int chip_i2c_xfer_with_notify(const int port, ret = chip_i2c_xfer(port, addr_flags, out, out_size, in, in_size, flags); - if (IS_ENABLED(CONFIG_I2C_XFER_BOARD_CALLBACK)) - i2c_end_xfer_notify(port, periph_addr_flags); - - if (IS_ENABLED(CONFIG_I2C_DEBUG)) - i2c_trace_notify(port, periph_addr_flags, 1, in, in_size); +#ifdef CONFIG_I2C_XFER_BOARD_CALLBACK + i2c_end_xfer_notify(port, periph_addr_flags); +#endif return ret; } diff --git a/common/i2c_trace.c b/common/i2c_trace.c deleted file mode 100644 index 0408751f53..0000000000 --- a/common/i2c_trace.c +++ /dev/null @@ -1,200 +0,0 @@ -/* 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 "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 periph_addr_lo; /* Inclusive */ - int periph_addr_hi; /* Inclusive */ -}; - -static struct i2c_trace_range trace_entries[8]; - -void i2c_trace_notify(int port, uint16_t periph_addr_flags, - int direction, const uint8_t *data, size_t size) -{ - size_t i; - uint16_t addr = I2C_GET_ADDR(periph_addr_flags); - - for (i = 0; i < ARRAY_SIZE(trace_entries); i++) - if (trace_entries[i].enabled - && trace_entries[i].port == port - && trace_entries[i].periph_addr_lo <= addr - && trace_entries[i].periph_addr_hi >= addr) - goto trace_enabled; - return; - -trace_enabled: - CPRINTF("i2c: %s %d:0x%X ", - direction ? "read" : "write", - port, - 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].periph_addr_lo); - if (trace_entries[i].periph_addr_hi - != trace_entries[i].periph_addr_lo) - ccprintf(" to 0x%X", - trace_entries[i].periph_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 periph_addr_lo, - int periph_addr_hi) -{ - struct i2c_trace_range *t; - struct i2c_trace_range *new_entry = NULL; - - if (port >= i2c_ports_used) - return EC_ERROR_PARAM2; - - if (periph_addr_lo > periph_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->periph_addr_lo <= periph_addr_lo && - t->periph_addr_hi >= periph_addr_hi) - return EC_SUCCESS; - - /* Extends existing range on both directions, replace */ - if (t->periph_addr_lo >= periph_addr_lo && - t->periph_addr_hi <= periph_addr_hi) { - t->enabled = 0; - return command_i2ctrace_enable( - port, periph_addr_lo, periph_addr_hi); - } - - /* Extends existing range below */ - if (t->periph_addr_lo - 1 <= periph_addr_hi && - t->periph_addr_hi >= periph_addr_hi) { - t->enabled = 0; - return command_i2ctrace_enable( - port, - periph_addr_lo, - t->periph_addr_hi); - } - - /* Extends existing range above */ - if (t->periph_addr_lo <= periph_addr_lo && - t->periph_addr_hi + 1 >= periph_addr_lo) { - t->enabled = 0; - return command_i2ctrace_enable( - port, - t->periph_addr_lo, - periph_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->periph_addr_lo = periph_addr_lo; - new_entry->periph_addr_hi = periph_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 | enable
| " - "enable ]", - "Trace I2C transactions"); diff --git a/docs/i2c-debugging.md b/docs/i2c-debugging.md deleted file mode 100644 index a1a56db285..0000000000 --- a/docs/i2c-debugging.md +++ /dev/null @@ -1,48 +0,0 @@ -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 - | enable
- | enable ] - -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 901c13bc77..6962ff2b5b 100644 --- a/include/i2c.h +++ b/include/i2c.h @@ -10,7 +10,6 @@ #include "common.h" #include "host_command.h" -#include "stddef.h" /* * I2C Peripheral Address encoding @@ -510,20 +509,6 @@ void i2c_start_xfer_notify(const int port, void i2c_end_xfer_notify(const int port, const uint16_t periph_addr_flags); -/** - * Defined in common/i2c_trace.c, used by i2c controller to notify tracing - * funcionality of transactions. - * - * @param port: I2C port number - * @param periph_addr: peripheral 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, uint16_t periph_addr_flags, - int direction, const uint8_t *data, size_t size); - /* * Interrupt handler of GPIO_MONITOR_I2CP_SDA. * Its role is to detect any transaction start during INT_AP_L -- cgit v1.2.1