Change in flashrom[master]: dediprog: Enable 4BA for all protocols
Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/44776 to review the following change. Change subject: dediprog: Enable 4BA for all protocols ...................................................................... dediprog: Enable 4BA for all protocols * Tested on SF600 with protocol version 3 with "W25Q256.W" (32768 kB) * Tested on SF100 with protocol version 2 with "W25Q256.W" (32768 kB) As it works with all protocol version, assume that the SF200 works fine as well. Change-Id: Iee0ba972245b9317ef86345432fec5fc32614888 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> --- M dediprog.c 1 file changed, 1 insertion(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/44776/1 diff --git a/dediprog.c b/dediprog.c index 827d5e4..13365cd 100644 --- a/dediprog.c +++ b/dediprog.c @@ -993,7 +993,7 @@ } static struct spi_master spi_master_dediprog = { - .features = SPI_MASTER_NO_4BA_MODES, + .features = SPI_MASTER_4BA, .max_data_read = 16, /* 18 seems to work fine as well, but 19 times out sometimes with FW 5.15. */ .max_data_write = 16, .command = dediprog_spi_send_command, @@ -1268,12 +1268,6 @@ if (dediprog_standalone_mode()) return 1; - if (dediprog_devicetype == DEV_SF100 && protocol() == PROTOCOL_V1) - spi_master_dediprog.features &= ~SPI_MASTER_NO_4BA_MODES; - - if (protocol() == PROTOCOL_V2) - spi_master_dediprog.features |= SPI_MASTER_4BA; - if (register_spi_master(&spi_master_dediprog) || dediprog_set_leds(LED_NONE)) return 1; -- To view, visit https://review.coreboot.org/c/flashrom/+/44776 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iee0ba972245b9317ef86345432fec5fc32614888 Gerrit-Change-Number: 44776 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-MessageType: newchange
Hello build bot (Jenkins), Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/44776 to look at the new patch set (#2). Change subject: dediprog: Disable SPI_MASTER_NO_4BA_MODES for additional devices ...................................................................... dediprog: Disable SPI_MASTER_NO_4BA_MODES for additional devices The SPI_MASTER_NO_4BA_MODES is for SPI master not keeping the flash powered between programming commands. Tests on the following devices showed that the power is stable accross commands: * SF100 protocol 2 V:6.5.03 * SF600 protocol 3 V:7.2.45 Change-Id: Iee0ba972245b9317ef86345432fec5fc32614888 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/76/44776/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/44776 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iee0ba972245b9317ef86345432fec5fc32614888 Gerrit-Change-Number: 44776 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44776 ) Change subject: dediprog: Disable SPI_MASTER_NO_4BA_MODES for additional devices ...................................................................... Patch Set 2: Code-Review+2 Perhaps it will be cleaner to enable 4BA in both native instruction mode and extended address mode by default, and then disallow it for older devices/FW revisions. In any case, this helps get users past an annoying error, so LGTM. -- To view, visit https://review.coreboot.org/c/flashrom/+/44776 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iee0ba972245b9317ef86345432fec5fc32614888 Gerrit-Change-Number: 44776 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 04 Oct 2020 23:15:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/44776 ) Change subject: dediprog: Disable SPI_MASTER_NO_4BA_MODES for additional devices ...................................................................... dediprog: Disable SPI_MASTER_NO_4BA_MODES for additional devices The SPI_MASTER_NO_4BA_MODES is for SPI master not keeping the flash powered between programming commands. Tests on the following devices showed that the power is stable accross commands: * SF100 protocol 2 V:6.5.03 * SF600 protocol 3 V:7.2.45 Change-Id: Iee0ba972245b9317ef86345432fec5fc32614888 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/44776 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: David Hendricks <david.hendricks@gmail.com> --- M dediprog.c 1 file changed, 2 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved diff --git a/dediprog.c b/dediprog.c index c50e374..ac7d462 100644 --- a/dediprog.c +++ b/dediprog.c @@ -1267,7 +1267,8 @@ if (dediprog_standalone_mode()) return 1; - if (dediprog_devicetype == DEV_SF100 && protocol() == PROTOCOL_V1) + if ((dediprog_devicetype == DEV_SF100) || + (dediprog_devicetype == DEV_SF600 && protocol() == PROTOCOL_V3)) spi_master_dediprog.features &= ~SPI_MASTER_NO_4BA_MODES; if (protocol() == PROTOCOL_V2) -- To view, visit https://review.coreboot.org/c/flashrom/+/44776 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iee0ba972245b9317ef86345432fec5fc32614888 Gerrit-Change-Number: 44776 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-MessageType: merged
participants (3)
-
David Hendricks (Code Review) -
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)