i2c: Cleanup I2C tracing output

The output of the I2C tracing is hard to parse, especially for reads to
I2C registers. This change creates only a single I2C trace for each I2C
transfer (instead of 2 entries), and labels the write and read parts of
the I2C transaction clearly.

Example output (TCPC device during disconnect):
i2c: 1:0x20 wr 0x10   rd 0x01 0x00
i2c: 1:0x20 wr 0x10 0x01 0x00
i2c: 1:0x20 wr 0x1A   rd 0x1A
i2c: 1:0x20 wr 0x1D   rd 0x10
i2c: 1:0x20 wr 0x1C   rd 0x70
i2c: 1:0x20 wr 0x2F 0x21
i2c: 1:0x20 wr 0x1C 0x70
i2c: 1:0x20 wr 0x2F 0x00
i2c: 1:0x20 wr 0x1C   rd 0x70
i2c: 1:0x20 wr 0x1C 0x60

BUG=none
BRANCH=none
TEST=make buildall
TEST=Enable CONFIG_I2C_DEBUG and verify output.

Signed-off-by: Keith Short <keithshort@chromium.org>
Change-Id: I077196e70ae3abb6c462cf08a3f944b43fdcf82a
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2091573
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
Reviewed-by: Jett Rink <jettrink@chromium.org>

4 files changed, 57 insertions, 48 deletions

diff --git a/common/i2c_master.c b/common/i2c_master.c
index ccb5a34492..b7b490b7dd 100644
--- a/common/i2c_master.c
+++ b/common/i2c_master.c
@@ -98,9 +98,6 @@ static int chip_i2c_xfer_with_notify(const int port,
 	uint16_t addr_flags = slave_addr_flags;
 	const struct i2c_port_t *i2c_port = get_i2c_port(port);
 
-	if (IS_ENABLED(CONFIG_I2C_DEBUG))
-		i2c_trace_notify(port, slave_addr_flags, 0, out, out_size);
-
 	if (IS_ENABLED(CONFIG_I2C_XFER_BOARD_CALLBACK))
 		i2c_start_xfer_notify(port, slave_addr_flags);
 
@@ -120,8 +117,10 @@ static int chip_i2c_xfer_with_notify(const int port,
 	if (IS_ENABLED(CONFIG_I2C_XFER_BOARD_CALLBACK))
 		i2c_end_xfer_notify(port, slave_addr_flags);
 
-	if (IS_ENABLED(CONFIG_I2C_DEBUG))
-		i2c_trace_notify(port, slave_addr_flags, 1, in, in_size);
+	if (IS_ENABLED(CONFIG_I2C_DEBUG)) {
+		i2c_trace_notify(port, slave_addr_flags, out, out_size,
+				 in, in_size);
+	}
 
 	return ret;
 }
diff --git a/common/i2c_trace.c b/common/i2c_trace.c
index d25f89a00c..6645b9a303 100644
--- a/common/i2c_trace.c
+++ b/common/i2c_trace.c
@@ -24,14 +24,12 @@ struct i2c_trace_range {
 static struct i2c_trace_range trace_entries[8];
 
 void i2c_trace_notify(int port, uint16_t slave_addr_flags,
-		      int direction, const uint8_t *data, size_t size)
+		      const uint8_t *out_data, size_t out_size,
+		      const uint8_t *in_data, size_t in_size)
 {
 	size_t i;
 	uint16_t addr = I2C_GET_ADDR(slave_addr_flags);
 
-	if (size == 0)
-		return;
-
 	for (i = 0; i < ARRAY_SIZE(trace_entries); i++)
 		if (trace_entries[i].enabled
 		    && trace_entries[i].port == port
@@ -41,12 +39,17 @@ void i2c_trace_notify(int port, uint16_t slave_addr_flags,
 	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("i2c: %d:0x%X ", port, addr);
+	if (out_size) {
+		CPRINTF("wr ");
+		for (i = 0; i < out_size; i++)
+			CPRINTF("0x%02X ", out_data[i]);
+	}
+	if (in_size) {
+		CPRINTF("  rd ");
+		for (i = 0; i < in_size; i++)
+			CPRINTF("0x%02X ", in_data[i]);
+	}
 	CPRINTF("\n");
 }
 
diff --git a/docs/i2c-debugging.md b/docs/i2c-debugging.md
index a1a56db285..7ab84f9d8d 100644
--- a/docs/i2c-debugging.md
+++ b/docs/i2c-debugging.md
@@ -11,38 +11,44 @@ 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>]
+```
+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
+```
+> i2ctrace enable 0 0x10 0x30
+> i2ctrace enable 1 0x20
+> i2ctrace list
+id port address
+-- ---- -------
+0     0 0x10 to 0x30
+1     1 0x40 to 0x50
+... debug spam may follow ...
+i2c: 1:0x20 wr 0x10   rd 0x01 0x00
+i2c: 1:0x20 wr 0x10 0x01 0x00
+...
+> 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
+```
+> 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
+```
\ No newline at end of file
diff --git a/include/i2c.h b/include/i2c.h
index b9feac13d8..3557d8e32b 100644
--- a/include/i2c.h
+++ b/include/i2c.h
@@ -551,17 +551,18 @@ void i2c_end_xfer_notify(const int port,
 
 /**
  * Defined in common/i2c_trace.c, used by i2c master to notify tracing
- * funcionality of transactions.
+ * functionality 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
+ * @param out_data: pointer to data written
+ * @param out_size: size of data written
+ * @param in_data: pointer to data read
+ * @param in_size: size of data read
  */
 void i2c_trace_notify(int port, uint16_t slave_addr_flags,
-		      int direction, const uint8_t *data, size_t size);
+		      const uint8_t *out_data, size_t out_size,
+		      const uint8_t *in_data, size_t in_size);
 
 /**
  * Set bus speed. Only support for ports with I2C_PORT_FLAG_DYNAMIC_SPEED