June 2023 - flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: flashchips: add support for MX25V16066/KH25V16066
by Anastasia Klimchuk (Code Review) June 27, 2023

June 27, 2023

Attention is currently required from: Joseph Goh. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68278?usp=email ) Change subject: flashchips: add support for MX25V16066/KH25V16066 ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: Joseph, last thing to do for you is to click Rebase (or rebase locally on the top and push). If I rebase myself I become an uploader, my approval might not count. -- To view, visit https://review.coreboot.org/c/flashrom/+/68278?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5f0548f023fcd09a970148586497e00414ad1ae Gerrit-Change-Number: 68278 Gerrit-PatchSet: 3 Gerrit-Owner: Joseph Goh <josephgoh7(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Joseph Goh <josephgoh7(a)gmail.com> Gerrit-Comment-Date: Tue, 27 Jun 2023 10:56:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom: only unlock for write/erase operations
by Edward O'Callaghan (Code Review) June 27, 2023

June 27, 2023

Attention is currently required from: Brian Norris, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75991?usp=email ) Change subject: flashrom: only unlock for write/erase operations ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > I checked the unlock functions, it turns out one of them actually does unlock for reads (last bullet […] Tricky, what about doing the `R|W` specialisation within the `unlock_flash_wp()` preamble? If the chip has `SPI_DISABLE_BLOCKPROTECT_SST26_GLOBAL_UNPROTECT` set `return 1` to let the fallback to the old `bp_func` mechanism for these special chips? -- To view, visit https://review.coreboot.org/c/flashrom/+/75991?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5dc66474a0b7969b51b86ac9f5daa2c95ae968f1 Gerrit-Change-Number: 75991 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 27 Jun 2023 00:02:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips: add support for MX25V16066/KH25V16066
by Joseph Goh (Code Review) June 26, 2023

June 26, 2023

Attention is currently required from: Anastasia Klimchuk. Joseph Goh has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68278?usp=email ) Change subject: flashchips: add support for MX25V16066/KH25V16066 ...................................................................... Patch Set 3: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/68278/comment/db0dd93c_82d2faf8 : PS2, Line 8772: MACRONIX_MX25L1605 > Thanks for explaining! I agree. […] Done! I've also added the erase commands in brackets although I'm not completely sure why the erase commands were included in brackets specifically for these chips. -- To view, visit https://review.coreboot.org/c/flashrom/+/68278?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5f0548f023fcd09a970148586497e00414ad1ae Gerrit-Change-Number: 68278 Gerrit-PatchSet: 3 Gerrit-Owner: Joseph Goh <josephgoh7(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 26 Jun 2023 13:42:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Joseph Goh <josephgoh7(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips: add support for MX25V16066/KH25V16066
by Joseph Goh (Code Review) June 26, 2023

June 26, 2023

Attention is currently required from: Anastasia Klimchuk, Joseph Goh. Hello Anastasia Klimchuk, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68278?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+1 by Anastasia Klimchuk, Verified+1 by build bot (Jenkins) Change subject: flashchips: add support for MX25V16066/KH25V16066 ...................................................................... flashchips: add support for MX25V16066/KH25V16066 Change-Id: Ic5f0548f023fcd09a970148586497e00414ad1ae Signed-off-by: Joseph Goh <josephgoh7(a)gmail.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M flashchips.c M include/flashchips.h 2 files changed, 39 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/68278/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/68278?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5f0548f023fcd09a970148586497e00414ad1ae Gerrit-Change-Number: 68278 Gerrit-PatchSet: 3 Gerrit-Owner: Joseph Goh <josephgoh7(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Joseph Goh <josephgoh7(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashchips: add support for MX77L25650F chip
by Sergii Dmytruk (Code Review) June 26, 2023

June 26, 2023

Attention is currently required from: Anastasia Klimchuk, Angel Pons, Artur Kowalski, ServError. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68557?usp=email ) Change subject: flashchips: add support for MX77L25650F chip ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > The datasheet specifies the set in the ML patch […] @ServError, since your patch is more complete, do you want to update this one? You would do that by using "DOWNLOAD" button to pull it locally, rebasing/amending the patch and then pushing it back via `git push {URL} HEAD:refs/for/master`. If not, I can combine patches myself and will put `Signed-off-by:` to give credit to you. -- To view, visit https://review.coreboot.org/c/flashrom/+/68557?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iaea5485f8b59b8538dc47beada2c308376ea027c Gerrit-Change-Number: 68557 Gerrit-PatchSet: 4 Gerrit-Owner: Artur Kowalski <artur.kowalski(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Artur Kowalski <arturkow2000(a)gmail.com> Gerrit-CC: ServError <admin(a)serverror.com> Gerrit-Attention: Artur Kowalski <artur.kowalski(a)3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: ServError <admin(a)serverror.com> Gerrit-Comment-Date: Mon, 26 Jun 2023 13:26:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: ServError <admin(a)serverror.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips: add support for MX77L25650F chip
by Anastasia Klimchuk (Code Review) June 26, 2023

June 26, 2023

Attention is currently required from: Angel Pons, Artur Kowalski, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68557?usp=email ) Change subject: flashchips: add support for MX77L25650F chip ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > I asked about it on Friday, waiting for a response after the work week begins. […] Thank you for help! Will wait for your response then. With the patches, all patches need to go through Gerrit. Even if patch is sent by ML, someone needs to push it to Gerrit. This one already created, so someone needs to rebase and push a new patchset to finish it. If that new patchset will be done by another contributor, they add a second Signed-off-by line. -- To view, visit https://review.coreboot.org/c/flashrom/+/68557?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iaea5485f8b59b8538dc47beada2c308376ea027c Gerrit-Change-Number: 68557 Gerrit-PatchSet: 4 Gerrit-Owner: Artur Kowalski <artur.kowalski(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Artur Kowalski <arturkow2000(a)gmail.com> Gerrit-CC: ServError <admin(a)serverror.com> Gerrit-Attention: Artur Kowalski <artur.kowalski(a)3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 26 Jun 2023 12:48:03 +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> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips: add support for MX77L25650F chip
by Sergii Dmytruk (Code Review) June 26, 2023

June 26, 2023

Attention is currently required from: Anastasia Klimchuk, Angel Pons, Artur Kowalski. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68557?usp=email ) Change subject: flashchips: add support for MX77L25650F chip ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > Sergii, do you know if anyone on your side (maybe you) would finish the patch (fix remaining comment […] I asked about it on Friday, waiting for a response after the work week begins. The ML patch looks more complete to me so far. -- To view, visit https://review.coreboot.org/c/flashrom/+/68557?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iaea5485f8b59b8538dc47beada2c308376ea027c Gerrit-Change-Number: 68557 Gerrit-PatchSet: 4 Gerrit-Owner: Artur Kowalski <artur.kowalski(a)3mdeb.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Artur Kowalski <arturkow2000(a)gmail.com> Gerrit-CC: ServError <admin(a)serverror.com> Gerrit-Attention: Artur Kowalski <artur.kowalski(a)3mdeb.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 26 Jun 2023 12:19:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom: only unlock for write/erase operations
by Nikolai Artemiev (Code Review) June 26, 2023

June 26, 2023

Attention is currently required from: Brian Norris, Edward O'Callaghan. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75991?usp=email ) Change subject: flashrom: only unlock for write/erase operations ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > I couldn't figure out whether this got here for a good reason. […] I checked the unlock functions, it turns out one of them actually does unlock for reads (last bullet). Most unlock functions call `spi_disable_blockprotect_generic()`, which only tries to clear bits in SR1. I've never seen a chip with SR1-controlled read protection, chips that do have read protection usually control it via special registers or command sequences. So these are probably safe. There are four unlock functions that don't call `spi_disable_blockprotect_generic()`: - `UNPROTECT_28SF040` - used by SST28SF040A - I don't have datasheet, it looks like a very old parallel flash. I would guess it doesn't have read protection. - `UNLOCK_SST_FWHUB` - used by several chips - the SST49LF0008A datasheet doesn't indicate it has read protection. - `SPI_DISABLE_BLOCKPROTECT_AT45DB` - used by several chips - the AT45DB321E datasheet doesn't indicate it has read protection. - `SPI_DISABLE_BLOCKPROTECT_SST26_GLOBAL_UNPROTECT` - used by several chips - according to the SST26VF032B datasheet, there is sector based read and write protection that can be globally cleared with a ULBPR (98h) instruciton, which is what the unlock function does. Maybe we can work it around with something like a `FEATURE_UNLOCK_FOR_READ` chip flag? -- To view, visit https://review.coreboot.org/c/flashrom/+/75991?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5dc66474a0b7969b51b86ac9f5daa2c95ae968f1 Gerrit-Change-Number: 75991 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 26 Jun 2023 12:14:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips: add support for MX25V16066/KH25V16066
by Anastasia Klimchuk (Code Review) June 26, 2023

June 26, 2023

Attention is currently required from: Joseph Goh. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68278?usp=email ) Change subject: flashchips: add support for MX25V16066/KH25V16066 ...................................................................... Patch Set 2: Code-Review+1 (3 comments) Patchset: PS2: Thanks Joseph! You need to add a comment about model id (see my comment below) and that should be it. File flashchips.c: https://review.coreboot.org/c/flashrom/+/68278/comment/cf3534cd_d620473b : PS2, Line 8772: MACRONIX_MX25L1605 > Nope, it is 0x2015 since the RDID command is used (probe_spi_rdid, see page 20). Thanks for explaining! I agree. Why I was initially confused is that ID MACRONIX_MX25L1605 from another chip is used. You need to add a comment to include/flashchips.h that this id also serves MX25V16066 (there are lots comments like this as examples). https://review.coreboot.org/c/flashrom/+/68278/comment/637da7c1_d2333141 : PS2, Line 8802: 2700 > yep, 2.3V-2.7V works, but at a slower speed. […] I see, thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/68278?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5f0548f023fcd09a970148586497e00414ad1ae Gerrit-Change-Number: 68278 Gerrit-PatchSet: 2 Gerrit-Owner: Joseph Goh <josephgoh7(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Joseph Goh <josephgoh7(a)gmail.com> Gerrit-Comment-Date: Mon, 26 Jun 2023 11:35:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Joseph Goh <josephgoh7(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashchips.c: Adding support for ISSI IS25WP020/40/80
by Anastasia Klimchuk (Code Review) June 26, 2023

June 26, 2023

Attention is currently required from: Ao Zhong. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75830?usp=email ) Change subject: flashchips.c: Adding support for ISSI IS25WP020/40/80 ...................................................................... Patch Set 4: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/75830/comment/d97d7eb6_888cead1 : PS4, Line 9: Hello, : I added Thank you, it is very nice of you to say hello :) And now since everyone saw this already, probably better to re-word the commit message slightly, let's say: "This patch adds support for ... <the rest of message>" Patchset: PS4: Thank you Ao, I checked the patch, all good! I have just one comment about commit message and that would be it. -- To view, visit https://review.coreboot.org/c/flashrom/+/75830?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8a786de5cf9ffefb2d57f89bbab71e289b5c2b28 Gerrit-Change-Number: 75830 Gerrit-PatchSet: 4 Gerrit-Owner: Ao Zhong <hacc1225(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Ao Zhong <hacc1225(a)gmail.com> Gerrit-Comment-Date: Mon, 26 Jun 2023 11:12:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0