Hello Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/flashrom/+/33195
to review the following change.
Change subject: dediprog: Allow 4BA on SF100 ......................................................................
dediprog: Allow 4BA on SF100
Tested on dediprog SF100.
Change-Id: I8822b79f46876feff0fd443f711c57dffb67b349 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M dediprog.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/33195/1
diff --git a/dediprog.c b/dediprog.c index 03675e5..3564f6f 100644 --- a/dediprog.c +++ b/dediprog.c @@ -1134,7 +1134,8 @@ if (dediprog_standalone_mode()) return 1;
- if (dediprog_devicetype == DEV_SF600 && protocol() == PROTOCOL_V2) + if ((dediprog_devicetype == DEV_SF600 || dediprog_devicetype == DEV_SF100) && + protocol() == PROTOCOL_V2) spi_master_dediprog.features |= SPI_MASTER_4BA;
if (register_spi_master(&spi_master_dediprog) || dediprog_set_leds(LED_NONE))
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/33195
to look at the new patch set (#2).
Change subject: dediprog: Allow 4BA on SF100 ......................................................................
dediprog: Allow 4BA on SF100
Tested on dediprog SF100 protocol V2.
Change-Id: I8822b79f46876feff0fd443f711c57dffb67b349 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M dediprog.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/33195/2
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33195 )
Change subject: dediprog: Allow 4BA on SF100 ......................................................................
Patch Set 2: Code-Review-1
(1 comment)
I think we can simplify that and just check protocol(). Does that sound right, Nico?
https://review.coreboot.org/#/c/33195/1/dediprog.c File dediprog.c:
https://review.coreboot.org/#/c/33195/1/dediprog.c@1137 PS1, Line 1137: (dediprog_devicetype == DEV_SF600 || dediprog_devicetype == DEV_SF100) && Checking the devicetype seems kind of pointless, I think. The protocol() check should be sufficient.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33195 )
Change subject: dediprog: Allow 4BA on SF100 ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/33195/1/dediprog.c File dediprog.c:
https://review.coreboot.org/#/c/33195/1/dediprog.c@1137 PS1, Line 1137: (dediprog_devicetype == DEV_SF600 || dediprog_devicetype == DEV_SF100) &&
Checking the devicetype seems kind of pointless, I think. The protocol() check should be sufficient.
There is the SF200 now too. So it boils down to the question if we trust it to always work with v2. I'm ok with either way.
Hello Patrick Rudolph, David Hendricks, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/33195
to look at the new patch set (#3).
Change subject: dediprog: Allow 4BA on all protocol V2 devices ......................................................................
dediprog: Allow 4BA on all protocol V2 devices
Tested on dediprog SF100 protocol V2. Assume it works fine on SF200 protocol V2, too.
Change-Id: I8822b79f46876feff0fd443f711c57dffb67b349 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M dediprog.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/33195/3
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33195 )
Change subject: dediprog: Allow 4BA on all protocol V2 devices ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/33195/1/dediprog.c File dediprog.c:
https://review.coreboot.org/#/c/33195/1/dediprog.c@1137 PS1, Line 1137: (dediprog_devicetype == DEV_SF600 || dediprog_devicetype == DEV_SF100) &&
There is the SF200 now too. So it boils down to the question if we trust it […]
Done. Only depend on protocol V2.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33195 )
Change subject: dediprog: Allow 4BA on all protocol V2 devices ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
Needs manual rebase, though.
https://review.coreboot.org/#/c/33195/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33195/3//COMMIT_MSG@9 PS3, Line 9: Tested on dediprog SF100 protocol V2. Please mention exact firmware and hardware revisions.
Hello Patrick Rudolph, David Hendricks, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/33195
to look at the new patch set (#4).
Change subject: dediprog: Allow 4BA on all protocol V2 devices ......................................................................
dediprog: Allow 4BA on all protocol V2 devices
Tested on dediprog SF100 protocol V2 (firmware V:6.5.03). Assume it works fine on SF200 protocol V2, too.
Change-Id: I8822b79f46876feff0fd443f711c57dffb67b349 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M dediprog.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/33195/4
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33195 )
Change subject: dediprog: Allow 4BA on all protocol V2 devices ......................................................................
Patch Set 4: Code-Review+2
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/33195 )
Change subject: dediprog: Allow 4BA on all protocol V2 devices ......................................................................
dediprog: Allow 4BA on all protocol V2 devices
Tested on dediprog SF100 protocol V2 (firmware V:6.5.03). Assume it works fine on SF200 protocol V2, too.
Change-Id: I8822b79f46876feff0fd443f711c57dffb67b349 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/33195 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M dediprog.c 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/dediprog.c b/dediprog.c index d48c541..8552a3c 100644 --- a/dediprog.c +++ b/dediprog.c @@ -1153,7 +1153,7 @@ if (dediprog_devicetype == DEV_SF100 && protocol() == PROTOCOL_V1) spi_master_dediprog.features &= ~SPI_MASTER_NO_4BA_MODES;
- if (dediprog_devicetype == DEV_SF600 && protocol() == PROTOCOL_V2) + if (protocol() == PROTOCOL_V2) spi_master_dediprog.features |= SPI_MASTER_4BA;
if (register_spi_master(&spi_master_dediprog) || dediprog_set_leds(LED_NONE))
-
David Hendricks (Code Review) -
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)