Change in flashrom[master]: flashchips: Fix W25Q256.W
Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/44810 to review the following change. Change subject: flashchips: Fix W25Q256.W ...................................................................... flashchips: Fix W25Q256.W Used the datasheet from here: https://www.winbond.com/resource-files/w25q256jw%20spi%20revb%2012082017.pdf The chips supports all native 4BA instructions. Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> --- M flashchips.c 1 file changed, 8 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/44810/1 diff --git a/flashchips.c b/flashchips.c index 8b5b5cc..2c16f89 100644 --- a/flashchips.c +++ b/flashchips.c @@ -16854,8 +16854,8 @@ .page_size = 256, /* supports SFDP */ /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER_WREN - | FEATURE_4BA_EXT_ADDR | FEATURE_4BA_READ | FEATURE_4BA_FAST_READ, + /* FOUR_BYTE_ADDR: supports 4-bytes addressing mode */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA, .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, @@ -16863,12 +16863,18 @@ { { .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = spi_block_erase_21, + }, { + .eraseblocks = { {4 * 1024, 8192} }, .block_erase = spi_block_erase_20, }, { .eraseblocks = { {32 * 1024, 1024} }, .block_erase = spi_block_erase_52, }, { .eraseblocks = { {64 * 1024, 512} }, + .block_erase = spi_block_erase_dc, + }, { + .eraseblocks = { {64 * 1024, 512} }, .block_erase = spi_block_erase_d8, }, { .eraseblocks = { {32 * 1024 * 1024, 1} }, -- To view, visit https://review.coreboot.org/c/flashrom/+/44810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Gerrit-Change-Number: 44810 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-MessageType: newchange
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44810 ) Change subject: flashchips: Fix W25Q256.W ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/44810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Gerrit-Change-Number: 44810 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Wed, 26 Aug 2020 10:50:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44810 ) Change subject: flashchips: Fix W25Q256.W ...................................................................... Patch Set 1: Code-Review+1 (1 comment) https://review.coreboot.org/c/flashrom/+/44810/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/flashrom/+/44810/1//COMMIT_MSG@12 PS1, Line 12: chips supports `chip supports` or `chips support` -- To view, visit https://review.coreboot.org/c/flashrom/+/44810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Gerrit-Change-Number: 44810 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> 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: Wed, 26 Aug 2020 12:09:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44810 ) Change subject: flashchips: Fix W25Q256.W ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/44810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Gerrit-Change-Number: 44810 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> 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:06:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44810 ) Change subject: flashchips: Fix W25Q256.W ...................................................................... Patch Set 2: Code-Review-1 (1 comment) Patchset: PS2: Sorry, I could not figure out from the history why there is a wildcard `.` in the name. It seems the original patch was tested with a JW. But there was a thread on the mailing list "Do you have support for W25Q128FW and W25Q256.W?" where a Winbond employee suggested that an FW once existed. It seems possible that it did not support the full command set, just like the FV version (if that is true at all and not an error in the datasheet). Without further details, we'd have to split the flashchips entry into FW and JW. Otherwise, we'd risk a regression for the former. Only getting hands on a 256FW sample could clear this up :-/ -- To view, visit https://review.coreboot.org/c/flashrom/+/44810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Gerrit-Change-Number: 44810 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> 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-Attention: Patrick Rudolph <siro@das-labor.org> Gerrit-Comment-Date: Tue, 11 May 2021 10:16:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Patrick Rudolph. Nico Huber has uploaded a new patch set (#3) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/flashrom/+/44810 ) Change subject: flashchips: Fix W25Q256.W ...................................................................... flashchips: Fix W25Q256.W The JW is the only known variant. A W25Q256FW may have existed with less 4BA instructions supported, but it never showed up and no data- sheet is available. Used the datasheet from here: https://www.winbond.com/resource-files/w25q256jw%20spi%20revb%2012082017.pdf Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Signed-off-by: Nico Huber <nico.h@gmx.de> --- M flashchips.c 1 file changed, 8 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/44810/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/44810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Gerrit-Change-Number: 44810 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Thomas Heijligen <src@posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Attention: Thomas Heijligen <src@posteo.de> Gerrit-Attention: Patrick Rudolph <siro@das-labor.org> Gerrit-MessageType: newpatchset
Attention is currently required from: Thomas Heijligen, Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44810 ) Change subject: flashchips: Fix W25Q256.W ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Patchset: PS2:
Sorry, I could not figure out from the history why there is […] I went ahead and updated the entry name to W25Q256JW and pushed a follow up to split W25Q256.V. If no FW sample shows up, it's probably better to do nothing about it.
-- To view, visit https://review.coreboot.org/c/flashrom/+/44810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Gerrit-Change-Number: 44810 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Thomas Heijligen <src@posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Attention: Thomas Heijligen <src@posteo.de> Gerrit-Attention: Patrick Rudolph <siro@das-labor.org> Gerrit-Comment-Date: Mon, 23 May 2022 13:24:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Angel Pons, Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44810 ) Change subject: flashchips: Fix W25Q256.W ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/44810/comment/2d940e80_01c2be06 PS1, Line 12: chips supports
`chip supports` or `chips support` Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/44810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Gerrit-Change-Number: 44810 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Thomas Heijligen <src@posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Attention: Thomas Heijligen <src@posteo.de> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Patrick Rudolph <siro@das-labor.org> Gerrit-Comment-Date: Mon, 23 May 2022 13:26:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons, Patrick Rudolph. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44810 ) Change subject: flashchips: Fix W25Q256.W ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/44810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Gerrit-Change-Number: 44810 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Thomas Heijligen <src@posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Thomas Heijligen <src@posteo.de> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Patrick Rudolph <siro@das-labor.org> Gerrit-Comment-Date: Mon, 23 May 2022 14:21:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons, Patrick Rudolph. Nico Huber has uploaded a new patch set (#4) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/flashrom/+/44810 ) Change subject: flashchips: Fix W25Q256.W ...................................................................... flashchips: Fix W25Q256.W The JW is the only known variant. A W25Q256FW may have existed with less 4BA instructions supported, but it never showed up and no data- sheet is available. Used the datasheet from here: https://www.winbond.com/resource-files/w25q256jw%20spi%20revb%2012082017.pdf Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Signed-off-by: Nico Huber <nico.h@gmx.de> Ticket: https://ticket.coreboot.org/issues/362 --- M flashchips.c 1 file changed, 8 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/44810/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/44810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Gerrit-Change-Number: 44810 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Thomas Heijligen <src@posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Attention: Nico Huber <nico.h@gmx.de> Gerrit-Attention: Thomas Heijligen <src@posteo.de> Gerrit-Attention: Angel Pons <th3fanbus@gmail.com> Gerrit-Attention: Patrick Rudolph <siro@das-labor.org> Gerrit-MessageType: newpatchset
Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/44810 ) Change subject: flashchips: Fix W25Q256.W ...................................................................... flashchips: Fix W25Q256.W The JW is the only known variant. A W25Q256FW may have existed with less 4BA instructions supported, but it never showed up and no data- sheet is available. Used the datasheet from here: https://www.winbond.com/resource-files/w25q256jw%20spi%20revb%2012082017.pdf Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> Signed-off-by: Nico Huber <nico.h@gmx.de> Ticket: https://ticket.coreboot.org/issues/362 Reviewed-on: https://review.coreboot.org/c/flashrom/+/44810 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> --- M flashchips.c 1 file changed, 8 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve Arthur Heymans: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index 82bb5b5..a9fd1bf 100644 --- a/flashchips.c +++ b/flashchips.c @@ -17597,7 +17597,7 @@ { .vendor = "Winbond", - .name = "W25Q256.W", + .name = "W25Q256JW", .bustype = BUS_SPI, .manufacture_id = WINBOND_NEX_ID, .model_id = WINBOND_NEX_W25Q256_W, @@ -17605,8 +17605,7 @@ .page_size = 256, /* supports SFDP */ /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER_WREN - | FEATURE_4BA_EXT_ADDR | FEATURE_4BA_READ | FEATURE_4BA_FAST_READ, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA, .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, @@ -17614,12 +17613,18 @@ { { .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = spi_block_erase_21, + }, { + .eraseblocks = { {4 * 1024, 8192} }, .block_erase = spi_block_erase_20, }, { .eraseblocks = { {32 * 1024, 1024} }, .block_erase = spi_block_erase_52, }, { .eraseblocks = { {64 * 1024, 512} }, + .block_erase = spi_block_erase_dc, + }, { + .eraseblocks = { {64 * 1024, 512} }, .block_erase = spi_block_erase_d8, }, { .eraseblocks = { {32 * 1024 * 1024, 1} }, 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/44810 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9a3995c66ad7b74823e17984bf1ffac50b5663e0 Gerrit-Change-Number: 44810 Gerrit-PatchSet: 6 Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks@gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger@posteo.net> Gerrit-Reviewer: Nico Huber <nico.h@gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph@9elements.com> Gerrit-Reviewer: Thomas Heijligen <src@posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec@chromium.org> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-MessageType: merged
participants (5)
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
David Hendricks (Code Review)
-
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)