- flashrom-gerrit - mail.coreboot.org

[XS] Change in flashrom[main]: erasure_layout: Fix unreachable error message
by Anastasia Klimchuk (Code Review) 18 Oct '24

18 Oct '24

Attention is currently required from: Aarya, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84725?usp=email ) Change subject: erasure_layout: Fix unreachable error message ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: Note: test fail because of the previous patch in the chain. Not the fault of this one. -- To view, visit https://review.coreboot.org/c/flashrom/+/84725?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I11d2d1359f74475cb20a1c91bddb380b4952a704 Gerrit-Change-Number: 84725 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Fri, 18 Oct 2024 12:54:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in flashrom[main]: Complete and fix progress feature implementation for all operations
by Anastasia Klimchuk (Code Review) 18 Oct '24

18 Oct '24

Attention is currently required from: Aarya, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/flashrom/+/84102?usp=email ) Change subject: Complete and fix progress feature implementation for all operations ...................................................................... Patch Set 10: (1 comment) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/84102/comment/6352e976_f5d29d5b?us… : PS7, Line 84: } > Yeah this does not assert anything, just prints for a human to read in the logs. […] I got some time, and I updated the test to assert that the progress does not go backwards. The last unresolved comment (so far) is resolved :) The test fails on the next patch (because of "feature no erase" needs some more work on progress), but this is what I want to do next. -- To view, visit https://review.coreboot.org/c/flashrom/+/84102?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: If1e40fc97f443c4f0c0501cef11cff1f3f84c051 Gerrit-Change-Number: 84102 Gerrit-PatchSet: 10 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Fri, 18 Oct 2024 12:53:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>

1 0

[L] Change in flashrom[main]: Complete and fix progress feature implementation for all operations
by Anastasia Klimchuk (Code Review) 18 Oct '24

18 Oct '24

Attention is currently required from: Aarya, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has uploaded a new patch set (#10) to the change originally created by Sergii Dmytruk. ( https://review.coreboot.org/c/flashrom/+/84102?usp=email ) The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: Complete and fix progress feature implementation for all operations ...................................................................... Complete and fix progress feature implementation for all operations Original progress reporting implemented in CB:49643 and it has some issues, for example: size_t start_address = start; size_t end_address = len - start; End address is anything but length minus start address. update_progress(flash, FLASHROM_PROGRESS_READ, /*current*/ start - start_address + to_read, /*total*/ end_address); Total should just be length if that's how current value is computed. --- libflashrom needs to know total size ahead of time. That's init_progress() and changed update_progress(). It also needs to store the last current value to be able to update it. That's stage_progress in flashrom_flashctx. Measuring accurately amount of data which will be read/erased/written isn't easy because things can be skipped as optimizations. The next patch in the chain aims to address this, there are TODO/FIXME comments there. --- CLI shares terminal with the rest of the code and has to maintain more state to handle that reasonably well. Similar to CB:64668, an effort is made to keep the progress on a single line. Non-progress output is kept track of to know when moving to a new line cannot be avoided. --- A script to test the CLI: \#!/bin/bash t=${1:-rewW} shift if [[ $t =~ r ]]; then echo ">>> READ" ./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -r dump.rom --progress "$@" echo fi if [[ $t =~ e ]]; then echo ">>> ERASE" ./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -E --progress "$@" echo fi if [[ $t =~ w ]]; then echo ">>> WRITE (without erase)" dd if=/dev/zero of=zero.rom bs=1M count=16 2> /dev/null ./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -w zero.rom --progress "$@" echo fi if [[ $t =~ W ]]; then echo ">>> WRITE (with erase)" dd if=/dev/zero of=zero.rom bs=1M count=16 2> /dev/null dd if=/dev/random of=random.rom bs=1M count=16 2> /dev/null ./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz,image=random.rom -w zero.rom --progress "$@" echo fi Co-developed-by: Anastasia Klimchuk <aklm(a)flashrom.org> Co-developed-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Change-Id: If1e40fc97f443c4f0c0501cef11cff1f3f84c051 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M 82802ab.c M at45db.c M cli_classic.c M cli_output.c M dediprog.c M doc/release_notes/devel.rst M en29lv640b.c M erasure_layout.c M flashrom.c M include/flash.h M it87spi.c M jedec.c M libflashrom.c M linux_mtd.c M parade_lspcon.c M realtek_mst_i2c_spi.c M spi.c M spi25.c M sst28sf040.c M tests/chip.c M tests/spi25.c M tests/tests.c M tests/tests.h 23 files changed, 325 insertions(+), 50 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/02/84102/10 -- To view, visit https://review.coreboot.org/c/flashrom/+/84102?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: If1e40fc97f443c4f0c0501cef11cff1f3f84c051 Gerrit-Change-Number: 84102 Gerrit-PatchSet: 10 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>

1 0

[M] Change in flashrom[main]: Display progress for what is actually erased/written
by Anastasia Klimchuk (Code Review) 18 Oct '24

18 Oct '24

Attention is currently required from: Sergii Dmytruk. Hello Sergii Dmytruk, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84439?usp=email to look at the new patch set (#10). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: Display progress for what is actually erased/written ...................................................................... Display progress for what is actually erased/written Change-Id: I88ac4d40f1b6ccc1636b1efb690d8d68bdebec08 Co-developed-by: Anastasia Klimchuk <aklm(a)flashrom.org> Co-developed-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/classic_cli_manpage.rst M flashrom.c M tests/chip.c M tests/tests.c M tests/tests.h 5 files changed, 107 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/84439/10 -- To view, visit https://review.coreboot.org/c/flashrom/+/84439?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I88ac4d40f1b6ccc1636b1efb690d8d68bdebec08 Gerrit-Change-Number: 84439 Gerrit-PatchSet: 10 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>

1 0

[S] Change in flashrom[main]: flashchips: add Winbond W25R512NW / W74M51NW
by Anastasia Klimchuk (Code Review) 18 Oct '24

18 Oct '24

Attention is currently required from: Alexandru Stan, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Alexandru Stan. ( https://review.coreboot.org/c/flashrom/+/84752?usp=email ) Change subject: flashchips: add Winbond W25R512NW / W74M51NW ...................................................................... Patch Set 2: (3 comments) File flashchips.c: https://review.coreboot.org/c/flashrom/+/84752/comment/4ff7045d_42bc76c9?us… : PS1, Line 20271: 1650 > What the datasheet says? I would say, voltage you better just copy from datasheet ah, but you already updated the voltage according to datasheet. I am resolving this. File flashchips.c: https://review.coreboot.org/c/flashrom/+/84752/comment/d67c813b_4d2c36c1?us… : PS2, Line 20239: 1024B total, 256B reserved Maybe the better comment is what's in the datasheet: > 3X256-Bytes Security Registers with OTP locks and then the rest of the comment as it is, with opcodes. Or as I said you can remove the comment line. https://review.coreboot.org/c/flashrom/+/84752/comment/58908027_b4550506?us… : PS2, Line 20269: .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, : .unlock = SPI_DISABLE_BLOCKPROTECT, : > Since you haven't tested write-protection and not marking it as tested, you can put here […] Actually, from the datasheet this is fine, leave it. And yes, you are not marking it as tested anyway. -- To view, visit https://review.coreboot.org/c/flashrom/+/84752?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ibf670e4014a22e4636789768b759cb51f75cd046 Gerrit-Change-Number: 84752 Gerrit-PatchSet: 2 Gerrit-Owner: Alexandru Stan <ams(a)frame.work> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alexandru Stan <ams(a)frame.work> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 18 Oct 2024 11:55:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alexandru Stan <ams(a)frame.work> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: flashchips: add Winbond W25R512NW / W74M51NW
by Anastasia Klimchuk (Code Review) 18 Oct '24

18 Oct '24

Attention is currently required from: Alexandru Stan, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Alexandru Stan. ( https://review.coreboot.org/c/flashrom/+/84752?usp=email ) Change subject: flashchips: add Winbond W25R512NW / W74M51NW ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/84752/comment/38a6e96c_bb681731?us… : PS1, Line 8: : I don't actually have a datasheet for this (so feel free to reject this : CL) > > I just got a datasheet myself […] Actually, the latest news: the datasheet made its way to me! :) I will look today later. Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/84752?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ibf670e4014a22e4636789768b759cb51f75cd046 Gerrit-Change-Number: 84752 Gerrit-PatchSet: 2 Gerrit-Owner: Alexandru Stan <ams(a)frame.work> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alexandru Stan <ams(a)frame.work> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 18 Oct 2024 04:46:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alexandru Stan <ams(a)frame.work> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: flashchips: add Winbond W25R512NW / W74M51NW
by Anastasia Klimchuk (Code Review) 17 Oct '24

17 Oct '24

Attention is currently required from: Alexandru Stan, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Alexandru Stan. ( https://review.coreboot.org/c/flashrom/+/84752?usp=email ) Change subject: flashchips: add Winbond W25R512NW / W74M51NW ...................................................................... Patch Set 2: (5 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/84752/comment/07e291c5_cc09a398?us… : PS1, Line 8: : I don't actually have a datasheet for this (so feel free to reject this : CL) > I just got a datasheet myself This is perfect, thanks so much for your effort! I understand not everything is distributable and we don't have Winbond engineers collaborating with us yet, not yet. However if you have the datasheet and the chip itself, and you tested it, that's all what is needed. You already checked the chip definition to the best of your knowledge, and tested the chip. If you have any doubts about anything, please ask! But I think you asked your questions already. So there are few open comments left and it should be all good. Sadly, I won't have anything relevant on my gdrive anymore. Also sadly, my job situation has changed dramatically, my current job is completely irrelevant to flashrom. I am here as a hobby <3 File flashchips.c: https://review.coreboot.org/c/flashrom/+/84752/comment/69fcf0e8_4191ab55?us… : PS1, Line 20271: 1650 > Datasheet disagreed here. What the datasheet says? I would say, voltage you better just copy from datasheet File flashchips.c: https://review.coreboot.org/c/flashrom/+/84752/comment/0cc5e647_fcc3c82f?us… : PS2, Line 20230: W25R512NW The situation you described in the following two lines, we do the following: we mark with the name `.name = W25R512NW/W74M51NW` , and then you don't need the comment in the following two lines. https://review.coreboot.org/c/flashrom/+/84752/comment/e3f8ed38_de5f2bbf?us… : PS2, Line 20239: 1024B total, 256B reserved > Again, copied this comment from that Winbond chip. […] I am often confused by how OTP size is described in the datasheet :( although I shouldn't be. Let's leave it. OTP is not implemented yet, these comments are for future. I will try to find similar chip model with similar OTP size with the datasheet I have, to compare. You can leave it, or if you have any concerns, you can just remove this comment line. https://review.coreboot.org/c/flashrom/+/84752/comment/bdb960de_6c116a1d?us… : PS2, Line 20269: .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, : .unlock = SPI_DISABLE_BLOCKPROTECT, : > Given I haven't tested WP, I don't know if these are good. […] Since you haven't tested write-protection and not marking it as tested, you can put here .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_PLAIN, .unlock = SPI_DISABLE_BLOCKPROTECT, and then later when/if write-protection is added, these values might be updated. -- To view, visit https://review.coreboot.org/c/flashrom/+/84752?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ibf670e4014a22e4636789768b759cb51f75cd046 Gerrit-Change-Number: 84752 Gerrit-PatchSet: 2 Gerrit-Owner: Alexandru Stan <ams(a)frame.work> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alexandru Stan <ams(a)frame.work> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 17 Oct 2024 09:21:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alexandru Stan <ams(a)frame.work> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XS] Change in flashrom[main]: flashchips: Add Support for XMC XM25LU64C
by Anastasia Klimchuk (Code Review) 17 Oct '24

17 Oct '24

Attention is currently required from: Nikolai Artemiev, Ssunk, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Ssunk. ( https://review.coreboot.org/c/flashrom/+/84775?usp=email ) Change subject: flashchips: Add Support for XMC XM25LU64C ...................................................................... Patch Set 1: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/84775/comment/7afa072b_bf855298?us… : PS1, Line 22696: TEST_UNTESTED The existing entry for XM25QU64C was untested, and if two models share the same definition, they also share test status. So the new model would also be marked "untested" even though you actually tested it. Is it possible that you test XM25QU64C (the one that was here before)? Then you will be able to mark them both as tested. If you do that, you will then add in commit message the info that you tested both models. Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/84775?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8d1af7fbfb4c45db09ed5ac82c683d273c8151c7 Gerrit-Change-Number: 84775 Gerrit-PatchSet: 1 Gerrit-Owner: Ssunk <ssunkkan(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Ssunk <ssunkkan(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 17 Oct 2024 07:59:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in flashrom[main]: flashchips: add GD25F256F
by Anastasia Klimchuk (Code Review) 17 Oct '24

17 Oct '24

Attention is currently required from: Stefan Reinauer, Victor Lim. Anastasia Klimchuk has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/84090?usp=email ) Change subject: flashchips: add GD25F256F ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Victor, this is now in the state of "merge conflict" because another flashchip was submitted earlier. Would you be able to rebase on the top of main? Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/84090?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ibbbbb8a55adbcbc2ee1785782c4eb3771d50c167 Gerrit-Change-Number: 84090 Gerrit-PatchSet: 2 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Victor Lim <vlim(a)gigadevice.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Thu, 17 Oct 2024 07:29:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[main]: erase/write: Deselect all smaller blocks when large block is selected
by Anastasia Klimchuk (Code Review) 16 Oct '24

16 Oct '24

Attention is currently required from: Aarya, Bill XIE. Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84686?usp=email ) Change subject: erase/write: Deselect all smaller blocks when large block is selected ...................................................................... Patch Set 4: (2 comments) File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/84686/comment/d5592b3b_ca42e7f2?us… : PS1, Line 237: /* Deselect all smaller blocks covering the same region. */ > I still need to add a test for this in `tests/erase_func_algo. […] I made the test! CB:84783 I need to tell you more about it! :) I needed to upgrade the test case so that they assert the eraseblocks invocations for write operation (it was recorded and logged, but not asserted). Previously, it was asserted for erase op only. With that, cases #9, 13, 20 feel the difference of CB:84614. So this is useful. I updated them to pass on current head. Now, about this patch, the issue it is fixing can never happen after CB:84614, however it can start re-happening after CB:84721. So the case #20 is specifically creating the situation of "nested sub-blocks", to be able to test CB:84721 later. I used case #20 to test this patch in isolation. File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/84686/comment/9b455365_e21f93ee?us… : PS2, Line 254: We are selecting one large block instead, to send opcode once : instead of sending many smaller once. */ > Done thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/84686?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Icfc18d5c090b1dcb92ab157e2c139be71af59300 Gerrit-Change-Number: 84686 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Wed, 16 Oct 2024 12:08:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Bill XIE <persmule(a)hardenedlinux.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit