Change in flashrom[master]: pickit2_spi: Remove dead assignment

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/34403 )

Change subject: pickit2_spi: Remove dead assignment
......................................................................

pickit2_spi: Remove dead assignment

We never read the first 'ret', so remove it.
Also, print the version only when the command succeeded.

Change-Id: I4aac5e1f3bd0604b079e1fdd9b7f09f1f4fc2d7f
Signed-off-by: Elyes HAOUAS 
---
M pickit2_spi.c
1 file changed, 2 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/34403/1

diff --git a/pickit2_spi.c b/pickit2_spi.c
index 6d9b28f..4e9ca1d 100644
--- a/pickit2_spi.c
+++ b/pickit2_spi.c
@@ -93,17 +93,16 @@
 {
    int ret;
    uint8_t command[CMD_LENGTH] = {CMD_GET_VERSION, CMD_END_OF_BUFFER};
-
    int transferred;
-	ret = libusb_interrupt_transfer(pickit2_handle, ENDPOINT_OUT, command, CMD_LENGTH, &transferred, DFLT_TIMEOUT);
+
    ret = libusb_interrupt_transfer(pickit2_handle, ENDPOINT_IN, command, CMD_LENGTH, &transferred, DFLT_TIMEOUT);

-	msg_pdbg("PICkit2 Firmware Version: %d.%d\n", (int)command[0], (int)command[1]);
    if (ret != 0) {
    	msg_perr("Command Get Firmware Version failed!\n");
    	return 1;
    }

+	msg_pdbg("PICkit2 Firmware Version: %d.%d\n", (int)command[0], (int)command[1]);
    return 0;
 }

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34403 )

Change subject: pickit2_spi: Remove dead assignment
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/flashrom/+/34403/1/pickit2_spi.c 
File pickit2_spi.c:

https://review.coreboot.org/c/flashrom/+/34403/1/pickit2_spi.c@a98 
PS1, Line 98: 
That we didn't check the return value doesn't mean that the line doesn't do
something. It sends the `command` to the Pickit. Without that, we can't expect
an answer in the next line.

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34403 )

Change subject: pickit2_spi: Remove dead assignment
......................................................................

Patch Set 1:

What we can do: Add a second `ret` variable and check them both:

ret = libusb_interrupt_transfer(..., ENDPOINT_OUT, ...);
    ret2 = libusb_interrupt_transfer(..., ENDPOINT_IN, ...);
    if (ret != 0 || ret2 != 0)
           ...

or check them individually, so we wouldn't run into the timeout
for the second transfer if the first already failed:

ret = libusb_interrupt_transfer(..., ENDPOINT_OUT, ...);
    if (ret != 0)
           ...
    ret = libusb_interrupt_transfer(..., ENDPOINT_IN, ...);
    if (ret != 0)
           ...

Change in flashrom[master]: pickit2_spi: Fix "dead" assignment

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/34403

to look at the new patch set (#2).

Change subject: pickit2_spi: Fix "dead" assignment
......................................................................

pickit2_spi: Fix "dead" assignment

We never read the first 'ret'. Let's check the first 'ret'
and exit if it failed.
Also, print the version only when the command succeeded.

Found-by scan-build: 7.0.1-8
Change-Id: I4aac5e1f3bd0604b079e1fdd9b7f09f1f4fc2d7f
Signed-off-by: Elyes HAOUAS 
---
M pickit2_spi.c
1 file changed, 10 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/34403/2

Change in flashrom[master]: pickit2_spi: Fix "dead" assignment

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34403 )

Change subject: pickit2_spi: Fix "dead" assignment
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/flashrom/+/34403/1/pickit2_spi.c 
File pickit2_spi.c:

https://review.coreboot.org/c/flashrom/+/34403/1/pickit2_spi.c@a98 
PS1, Line 98:
> That we didn't check the return value doesn't mean that the line doesn't do […]

Done.
Thank you

Change in flashrom[master]: pickit2_spi: Fix "dead" assignment

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34403 )

Change subject: pickit2_spi: Fix "dead" assignment
......................................................................

Patch Set 2: Code-Review+2

(2 comments)

Thanks!

https://review.coreboot.org/c/flashrom/+/34403/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/34403/2//COMMIT_MSG@10 
PS2, Line 10: and exit if it failed.
Please separate paragraphs with an empty line (or remove the line
break).

https://review.coreboot.org/c/flashrom/+/34403/2//COMMIT_MSG@13 
PS2, Line 13: Found-by
I guess `Found-by` should be followed by a colon.

Change in flashrom[master]: pickit2_spi: Fix "dead" assignment

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/34403

to look at the new patch set (#3).

Change subject: pickit2_spi: Fix "dead" assignment
......................................................................

pickit2_spi: Fix "dead" assignment

We never read the first 'ret'. Let's check the first 'ret'
and exit if it failed.
Also, print the version only when the command succeeded.

Found-by: scan-build 7.0.1-8

Change-Id: I4aac5e1f3bd0604b079e1fdd9b7f09f1f4fc2d7f
Signed-off-by: Elyes HAOUAS 
---
M pickit2_spi.c
1 file changed, 10 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/34403/3

Change in flashrom[master]: pickit2_spi: Fix "dead" assignment

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/34403

to look at the new patch set (#4).

Change subject: pickit2_spi: Fix "dead" assignment
......................................................................

pickit2_spi: Fix "dead" assignment

We never read the first 'ret'. Let's check the first 'ret'
and exit if it failed.

Also, print the version only when the command succeeded.

Found-by: scan-build 7.0.1-8
Change-Id: I4aac5e1f3bd0604b079e1fdd9b7f09f1f4fc2d7f
Signed-off-by: Elyes HAOUAS 
---
M pickit2_spi.c
1 file changed, 10 insertions(+), 4 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/34403/4

Change in flashrom[master]: pickit2_spi: Fix "dead" assignment

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/34403 )

Change subject: pickit2_spi: Fix "dead" assignment
......................................................................

Patch Set 4: Verified+1

Change in flashrom[master]: pickit2_spi: Fix "dead" assignment

[Nico Huber (Code Review)]

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/34403 )

Change subject: pickit2_spi: Fix "dead" assignment
......................................................................

pickit2_spi: Fix "dead" assignment

We never read the first 'ret'. Let's check the first 'ret'
and exit if it failed.

Also, print the version only when the command succeeded.

Found-by: scan-build 7.0.1-8
Change-Id: I4aac5e1f3bd0604b079e1fdd9b7f09f1f4fc2d7f
Signed-off-by: Elyes HAOUAS 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/34403
Tested-by: Nico Huber 
Reviewed-by: Nico Huber 
---
M pickit2_spi.c
1 file changed, 10 insertions(+), 4 deletions(-)

Approvals:
  Nico Huber: Verified; Looks good to me, approved

diff --git a/pickit2_spi.c b/pickit2_spi.c
index 6d9b28f..52021d9 100644
--- a/pickit2_spi.c
+++ b/pickit2_spi.c
@@ -93,17 +93,23 @@
 {
    int ret;
    uint8_t command[CMD_LENGTH] = {CMD_GET_VERSION, CMD_END_OF_BUFFER};
-
    int transferred;
-	ret = libusb_interrupt_transfer(pickit2_handle, ENDPOINT_OUT, command, CMD_LENGTH, &transferred, DFLT_TIMEOUT);
-	ret = libusb_interrupt_transfer(pickit2_handle, ENDPOINT_IN, command, CMD_LENGTH, &transferred, DFLT_TIMEOUT);

-	msg_pdbg("PICkit2 Firmware Version: %d.%d\n", (int)command[0], (int)command[1]);
+	ret = libusb_interrupt_transfer(pickit2_handle, ENDPOINT_OUT, command, CMD_LENGTH, &transferred, DFLT_TIMEOUT);
+
    if (ret != 0) {
    	msg_perr("Command Get Firmware Version failed!\n");
    	return 1;
    }

+	ret = libusb_interrupt_transfer(pickit2_handle, ENDPOINT_IN, command, CMD_LENGTH, &transferred, DFLT_TIMEOUT);
+
+	if (ret != 0) {
+		msg_perr("Command Get Firmware Version failed!\n");
+		return 1;
+	}
+
+	msg_pdbg("PICkit2 Firmware Version: %d.%d\n", (int)command[0], (int)command[1]);
    return 0;
 }