Change in flashrom[master]: spi25.c: Improve spi_simple_write_cmd() diags and readability
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/47452 )
Change subject: spi25.c: Improve spi_simple_write_cmd() diags and readability ......................................................................
spi25.c: Improve spi_simple_write_cmd() diags and readability
Clearly initialise the cmds[] array of structures with explicit values and improve failure diagnostic line to inline failing op.
BUG=b:170690915 BRANCH=none TEST=shows opcode
Change-Id: Ib9fc82558035e511ee485fb5aab0bcd3e164c6ba Signed-off-by: Edward O'Callaghan quasisec@google.com --- M spi25.c 1 file changed, 5 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/47452/1
diff --git a/spi25.c b/spi25.c index 213273f..41fa3dd 100644 --- a/spi25.c +++ b/spi25.c @@ -324,20 +324,22 @@ { struct spi_command cmds[] = { { - .readarr = 0, .writecnt = JEDEC_WREN_OUTSIZE, .writearr = (const unsigned char[]){ JEDEC_WREN }, + .readcnt = 0, + .readarr = NULL, }, { - .readarr = 0, .writecnt = 1, .writearr = (const unsigned char[]){ op }, + .readcnt = 0, + .readarr = NULL, }, NULL_SPI_CMD, };
const int result = spi_send_multicommand(flash, cmds); if (result) - msg_cerr("%s failed during command execution\n", __func__); + msg_cerr("%s: opcode=0x%x failed during command execution\n", __func__, op);
const int status = poll_delay ? spi_poll_wip(flash, poll_delay) : 0;
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47452 )
Change subject: spi25.c: Improve spi_simple_write_cmd() diags and readability ......................................................................
Patch Set 1:
(1 comment)
I don't see the point of explicitly initializing the read members of a write command.
https://review.coreboot.org/c/flashrom/+/47452/1/spi25.c File spi25.c:
https://review.coreboot.org/c/flashrom/+/47452/1/spi25.c@354 PS1, Line 354: .readarr = 0, What about these?
Hello build bot (Jenkins), Patrick Georgi, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/47452
to look at the new patch set (#2).
Change subject: spi25.c: Improve spi_simple_write_cmd() diags and readability ......................................................................
spi25.c: Improve spi_simple_write_cmd() diags and readability
Clearly initialise the cmds[] array of structures with explicit values and improve failure diagnostic line to inline failing op.
BUG=b:170690915 BRANCH=none TEST=shows opcode
Change-Id: Ib9fc82558035e511ee485fb5aab0bcd3e164c6ba Signed-off-by: Edward O'Callaghan quasisec@google.com --- M spi25.c 1 file changed, 13 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/47452/2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47452 )
Change subject: spi25.c: Improve spi_simple_write_cmd() diags and readability ......................................................................
Patch Set 2:
(1 comment)
Patch Set 1:
(1 comment)
I don't see the point of explicitly initializing the read members of a write command.
It just makes the file more consistent to read as well as help static analyzers.
https://review.coreboot.org/c/flashrom/+/47452/1/spi25.c File spi25.c:
https://review.coreboot.org/c/flashrom/+/47452/1/spi25.c@354 PS1, Line 354: .readarr = 0,
What about these?
Done
Hello build bot (Jenkins), Patrick Georgi, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/47452
to look at the new patch set (#3).
Change subject: spi25.c: Improve spi cmd write ops diags and readability ......................................................................
spi25.c: Improve spi cmd write ops diags and readability
Clearly initialise the cmds[] array of structures with explicit values and improve failure diagnostic line to inline failing op.
BUG=b:170690915 BRANCH=none TEST=shows opcode
Change-Id: Ib9fc82558035e511ee485fb5aab0bcd3e164c6ba Signed-off-by: Edward O'Callaghan quasisec@google.com --- M spi25.c 1 file changed, 13 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/47452/3
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47452 )
Change subject: spi25.c: Improve spi cmd write ops diags and readability ......................................................................
Patch Set 3:
Patch Set 2:
(1 comment)
Patch Set 1:
(1 comment)
I don't see the point of explicitly initializing the read members of a write command.
It just makes the file more consistent to read as well as help static analyzers.
Wait, what? How can this help static analyzers? Why clutter the code to please mechanical eyes?
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)