Stefan Reinauer has uploaded this change for review. ( https://review.coreboot.org/c/em100/+/48728 )
Change subject: Add a brief mode for trace output ......................................................................
Add a brief mode for trace output
The log with -b|--brief will look like this:
0x5a @ 0x00000000 (read SFDP) 0x5a @ 0x00000008 (read SFDP) 0x5a @ 0x00000030 (read SFDP) 0xb7 (enter 4b mode) 0x03 @ 0x00000010 (read) 0x03 @ 0x00000014 (read) 0x03 @ 0x00000030 (read) 0x03 @ 0x00000040 (read)
Instead of this:
Time: 000000.00000000 command # 1 : 0x5a - read SFDP Table 00000000 : 53 46 44 50 00 01 01 ff Time: 000000.00000575 command # 2 : 0x5a - read SFDP Table 00000008 : 00 00 01 09 30 00 00 ff Time: 000000.00001149 command # 3 : 0x5a - read SFDP Table 00000030 : e5 20 f3 ff ff ff ff 0f 44 eb 08 6b 08 3b 04 bb Time: 000000.00002053 command # 4 : 0xb7 - enter 4-byte address mode Time: 000000.00002687 command # 5 : 0x03 - read 00000000 : 10 5a a5 f0 0f Time: 000000.00003088 command # 6 : 0x03 - read 00000000 : 14 03 00 04 05 06 03 10 15 20 01 21 26 Time: 000000.00003825 command # 7 : 0x03 - read 00000000 : 30 f5 00 00 00 00 00 00 00 00 00 00 00 Time: 000000.00004569 command # 8 : 0x03 - read 00000000 : 40 00 00 00 00 00 08 ff 0f 23 00 ff 07 01 00 02 00000010 : 00 03 00 12 00 13 00 22 00 ff 7f 00 00
Signed-off-by: Stefan Reinauer stefan.reinauer@coreboot.org Change-Id: Icdaa3444bba03be8bf122239fd6da805cf840e52 --- M em100.c M em100.h M trace.c 3 files changed, 53 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/em100 refs/changes/28/48728/1
diff --git a/em100.c b/em100.c index 67ac43c..7a6f910 100644 --- a/em100.c +++ b/em100.c @@ -790,6 +790,7 @@ {"terminal", 0, 0, 'T'}, {"traceconsole", 0, 0, 'R'}, {"length", 1, 0, 'L'}, + {"brief", 0, 0, 'b'}, {"compatible", 0, 0, 'C'}, {NULL, 0, 0, 0} }; @@ -812,6 +813,7 @@ " -T|--terminal: terminal mode\n" " -R|--traceconsole: trace console mode\n" " -L|--length HEX_VAL: length of buffer for traceconsole mode\n" + " -b|--brief: brief mode for traces\n" " -F|--firmware-update FILE|auto: update EM100pro firmware (dangerous)\n" " -f|--firmware-dump FILE: export raw EM100pro firmware to file\n" " -g|--firmware-write FILE: export EM100pro firmware to DPFW file\n" @@ -848,7 +850,7 @@ struct sigaction signal_action; struct em100 *em100 = &em100_state;
- while ((opt = getopt_long(argc, argv, "c:d:a:m:u:rsvtO:F:f:g:S:V:p:DCx:lUhTRL:", + while ((opt = getopt_long(argc, argv, "c:d:a:m:u:rsvtO:F:f:g:S:V:p:DCx:lUhTRL:b", longopts, &idx)) != -1) { switch (opt) { case 'c': @@ -930,6 +932,9 @@ case 'L': sscanf(optarg, "%lx", &address_length); break; + case 'b': + trace_brief = 1; + break; case 'C': compatibility = 1; break; diff --git a/em100.h b/em100.h index bf1bb87..4ba78a9 100644 --- a/em100.h +++ b/em100.h @@ -126,6 +126,7 @@ int set_led(struct em100 *em100, led_state_t led_state);
/* trace.c */ +extern int trace_brief; #define EM100_SPECIFIC_CMD 0x11 #define EM100_MSG_SIGNATURE 0x47364440
diff --git a/trace.c b/trace.c index df3fdd9..c04afcc 100644 --- a/trace.c +++ b/trace.c @@ -15,6 +15,8 @@ #include <string.h> #include "em100.h"
+int trace_brief = 0; + /* SPI Trace related operations */
/** @@ -282,44 +284,62 @@ j = MAX_TRACE_BLOCKLENGTH; }
- printf("\nTime: %06lld.%08lld", + if (trace_brief) { + if (start_timestamp) + start_timestamp = 0; + + if (spi_cmd_vals->address_type != ADDR_NONE) + printf("0x%02x @ 0x%08lx (%s)\n", + spi_command, address, spi_cmd_vals->cmd_name); + else + printf("0x%02x (%s)\n", + spi_command, spi_cmd_vals->cmd_name); + } else { + printf("\nTime: %06lld.%08lld", (timestamp - start_timestamp) / 100000000, (timestamp - start_timestamp) % 100000000); - printf(" command # %-6d : 0x%02x - %s", + printf(" command # %-6d : 0x%02x - %s", ++counter, spi_command, spi_cmd_vals->cmd_name); + } + curpos = 0; outbytes = 0; }
- /* this exploits 8bit wrap around in curpos */ - unsigned char blocklen = (data[2 + i*8 + 1] - curpos); - blocklen /= 8; + if (trace_brief) { + if (outbytes) + outbytes++; + } else { + /* this exploits 8bit wrap around in curpos */ + unsigned char blocklen = (data[2 + i*8 + 1] - curpos); + blocklen /= 8;
- for (; j < blocklen; j++) { - if (outbytes == 0) { - switch (spi_cmd_vals->address_type) { - case ADDR_DYNAMIC: - case ADDR_3B: - case ADDR_4B: - printf("\n%08lx : ", - addr_offset + address); - break; - case ADDR_NO_OFF_3B: - printf("\n%08lx : ", address); - break; - case ADDR_NONE: - printf("\n : "); - break; + for (; j < blocklen; j++) { + if (outbytes == 0) { + switch (spi_cmd_vals->address_type) { + case ADDR_DYNAMIC: + case ADDR_3B: + case ADDR_4B: + printf("\n%08lx : ", + addr_offset + address); + break; + case ADDR_NO_OFF_3B: + printf("\n%08lx : ", address); + break; + case ADDR_NONE: + printf("\n : "); + break; + } } - } - printf("%02x ", data[i * 8 + 4 + j]); - outbytes++; - if (outbytes == 16) { - outbytes = 0; - address += 16; + printf("%02x ", data[i * 8 + 4 + j]); + outbytes++; + if (outbytes == 16) { + outbytes = 0; + address += 16; + } } } // this is because the em100 counts funny
ron minnich has posted comments on this change. ( https://review.coreboot.org/c/em100/+/48728 )
Change subject: Add a brief mode for trace output ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/em100/+/48728/1/trace.c File trace.c:
https://review.coreboot.org/c/em100/+/48728/1/trace.c@18 PS1, Line 18: int trace_brief = 0; technically, you don't need the initialization here.
Hello build bot (Jenkins), ron minnich,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/em100/+/48728
to look at the new patch set (#2).
Change subject: Add a brief mode for trace output ......................................................................
Add a brief mode for trace output
The log with -b|--brief will look like this:
0x5a @ 0x00000000 (read SFDP) 0x5a @ 0x00000008 (read SFDP) 0x5a @ 0x00000030 (read SFDP) 0xb7 (enter 4b mode) 0x03 @ 0x00000010 (read) 0x03 @ 0x00000014 (read) 0x03 @ 0x00000030 (read) 0x03 @ 0x00000040 (read)
Instead of this:
Time: 000000.00000000 command # 1 : 0x5a - read SFDP Table 00000000 : 53 46 44 50 00 01 01 ff Time: 000000.00000575 command # 2 : 0x5a - read SFDP Table 00000008 : 00 00 01 09 30 00 00 ff Time: 000000.00001149 command # 3 : 0x5a - read SFDP Table 00000030 : e5 20 f3 ff ff ff ff 0f 44 eb 08 6b 08 3b 04 bb Time: 000000.00002053 command # 4 : 0xb7 - enter 4-byte address mode Time: 000000.00002687 command # 5 : 0x03 - read 00000000 : 10 5a a5 f0 0f Time: 000000.00003088 command # 6 : 0x03 - read 00000000 : 14 03 00 04 05 06 03 10 15 20 01 21 26 Time: 000000.00003825 command # 7 : 0x03 - read 00000000 : 30 f5 00 00 00 00 00 00 00 00 00 00 00 Time: 000000.00004569 command # 8 : 0x03 - read 00000000 : 40 00 00 00 00 00 08 ff 0f 23 00 ff 07 01 00 02 00000010 : 00 03 00 12 00 13 00 22 00 ff 7f 00 00
Signed-off-by: Stefan Reinauer stefan.reinauer@coreboot.org Change-Id: Icdaa3444bba03be8bf122239fd6da805cf840e52 --- M em100.c M em100.h M trace.c 3 files changed, 53 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/em100 refs/changes/28/48728/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/em100/+/48728 )
Change subject: Add a brief mode for trace output ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/em100/+/48728/2/trace.c File trace.c:
https://review.coreboot.org/c/em100/+/48728/2/trace.c@313 PS2, Line 313: outbytes This should not ever be true with trace_brief, right?
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/em100/+/48728 )
Change subject: Add a brief mode for trace output ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/em100/+/48728/2/trace.c File trace.c:
https://review.coreboot.org/c/em100/+/48728/2/trace.c@313 PS2, Line 313: outbytes
This should not ever be true with trace_brief, right?
That's a good point. I copied this blindly from Duncan's original patch but I might have gotten it wrong (or it was never needed). Let me check the original code again and hopefully remove this if it's not needed.
Stefan Reinauer has submitted this change. ( https://review.coreboot.org/c/em100/+/48728 )
Change subject: Add a brief mode for trace output ......................................................................
Add a brief mode for trace output
The log with -b|--brief will look like this:
0x5a @ 0x00000000 (read SFDP) 0x5a @ 0x00000008 (read SFDP) 0x5a @ 0x00000030 (read SFDP) 0xb7 (enter 4b mode) 0x03 @ 0x00000010 (read) 0x03 @ 0x00000014 (read) 0x03 @ 0x00000030 (read) 0x03 @ 0x00000040 (read)
Instead of this:
Time: 000000.00000000 command # 1 : 0x5a - read SFDP Table 00000000 : 53 46 44 50 00 01 01 ff Time: 000000.00000575 command # 2 : 0x5a - read SFDP Table 00000008 : 00 00 01 09 30 00 00 ff Time: 000000.00001149 command # 3 : 0x5a - read SFDP Table 00000030 : e5 20 f3 ff ff ff ff 0f 44 eb 08 6b 08 3b 04 bb Time: 000000.00002053 command # 4 : 0xb7 - enter 4-byte address mode Time: 000000.00002687 command # 5 : 0x03 - read 00000000 : 10 5a a5 f0 0f Time: 000000.00003088 command # 6 : 0x03 - read 00000000 : 14 03 00 04 05 06 03 10 15 20 01 21 26 Time: 000000.00003825 command # 7 : 0x03 - read 00000000 : 30 f5 00 00 00 00 00 00 00 00 00 00 00 Time: 000000.00004569 command # 8 : 0x03 - read 00000000 : 40 00 00 00 00 00 08 ff 0f 23 00 ff 07 01 00 02 00000010 : 00 03 00 12 00 13 00 22 00 ff 7f 00 00
Signed-off-by: Stefan Reinauer stefan.reinauer@coreboot.org Change-Id: Icdaa3444bba03be8bf122239fd6da805cf840e52 Reviewed-on: https://review.coreboot.org/c/em100/+/48728 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: ron minnich rminnich@gmail.com --- M em100.c M em100.h M trace.c 3 files changed, 53 insertions(+), 27 deletions(-)
Approvals: build bot (Jenkins): Verified ron minnich: Looks good to me, approved
diff --git a/em100.c b/em100.c index 67ac43c..7a6f910 100644 --- a/em100.c +++ b/em100.c @@ -790,6 +790,7 @@ {"terminal", 0, 0, 'T'}, {"traceconsole", 0, 0, 'R'}, {"length", 1, 0, 'L'}, + {"brief", 0, 0, 'b'}, {"compatible", 0, 0, 'C'}, {NULL, 0, 0, 0} }; @@ -812,6 +813,7 @@ " -T|--terminal: terminal mode\n" " -R|--traceconsole: trace console mode\n" " -L|--length HEX_VAL: length of buffer for traceconsole mode\n" + " -b|--brief: brief mode for traces\n" " -F|--firmware-update FILE|auto: update EM100pro firmware (dangerous)\n" " -f|--firmware-dump FILE: export raw EM100pro firmware to file\n" " -g|--firmware-write FILE: export EM100pro firmware to DPFW file\n" @@ -848,7 +850,7 @@ struct sigaction signal_action; struct em100 *em100 = &em100_state;
- while ((opt = getopt_long(argc, argv, "c:d:a:m:u:rsvtO:F:f:g:S:V:p:DCx:lUhTRL:", + while ((opt = getopt_long(argc, argv, "c:d:a:m:u:rsvtO:F:f:g:S:V:p:DCx:lUhTRL:b", longopts, &idx)) != -1) { switch (opt) { case 'c': @@ -930,6 +932,9 @@ case 'L': sscanf(optarg, "%lx", &address_length); break; + case 'b': + trace_brief = 1; + break; case 'C': compatibility = 1; break; diff --git a/em100.h b/em100.h index bf1bb87..4ba78a9 100644 --- a/em100.h +++ b/em100.h @@ -126,6 +126,7 @@ int set_led(struct em100 *em100, led_state_t led_state);
/* trace.c */ +extern int trace_brief; #define EM100_SPECIFIC_CMD 0x11 #define EM100_MSG_SIGNATURE 0x47364440
diff --git a/trace.c b/trace.c index df3fdd9..c04afcc 100644 --- a/trace.c +++ b/trace.c @@ -15,6 +15,8 @@ #include <string.h> #include "em100.h"
+int trace_brief = 0; + /* SPI Trace related operations */
/** @@ -282,44 +284,62 @@ j = MAX_TRACE_BLOCKLENGTH; }
- printf("\nTime: %06lld.%08lld", + if (trace_brief) { + if (start_timestamp) + start_timestamp = 0; + + if (spi_cmd_vals->address_type != ADDR_NONE) + printf("0x%02x @ 0x%08lx (%s)\n", + spi_command, address, spi_cmd_vals->cmd_name); + else + printf("0x%02x (%s)\n", + spi_command, spi_cmd_vals->cmd_name); + } else { + printf("\nTime: %06lld.%08lld", (timestamp - start_timestamp) / 100000000, (timestamp - start_timestamp) % 100000000); - printf(" command # %-6d : 0x%02x - %s", + printf(" command # %-6d : 0x%02x - %s", ++counter, spi_command, spi_cmd_vals->cmd_name); + } + curpos = 0; outbytes = 0; }
- /* this exploits 8bit wrap around in curpos */ - unsigned char blocklen = (data[2 + i*8 + 1] - curpos); - blocklen /= 8; + if (trace_brief) { + if (outbytes) + outbytes++; + } else { + /* this exploits 8bit wrap around in curpos */ + unsigned char blocklen = (data[2 + i*8 + 1] - curpos); + blocklen /= 8;
- for (; j < blocklen; j++) { - if (outbytes == 0) { - switch (spi_cmd_vals->address_type) { - case ADDR_DYNAMIC: - case ADDR_3B: - case ADDR_4B: - printf("\n%08lx : ", - addr_offset + address); - break; - case ADDR_NO_OFF_3B: - printf("\n%08lx : ", address); - break; - case ADDR_NONE: - printf("\n : "); - break; + for (; j < blocklen; j++) { + if (outbytes == 0) { + switch (spi_cmd_vals->address_type) { + case ADDR_DYNAMIC: + case ADDR_3B: + case ADDR_4B: + printf("\n%08lx : ", + addr_offset + address); + break; + case ADDR_NO_OFF_3B: + printf("\n%08lx : ", address); + break; + case ADDR_NONE: + printf("\n : "); + break; + } } - } - printf("%02x ", data[i * 8 + 4 + j]); - outbytes++; - if (outbytes == 16) { - outbytes = 0; - address += 16; + printf("%02x ", data[i * 8 + 4 + j]); + outbytes++; + if (outbytes == 16) { + outbytes = 0; + address += 16; + } } } // this is because the em100 counts funny
-
Furquan Shaikh (Code Review) -
ron minnich (Code Review)
-
Stefan Reinauer (Code Review)