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

[M] Change in flashrom[master]: flashrom: only perform WP unlock for write/erase operations
by Nikolai Artemiev (Code Review) 04 Jul '23

04 Jul '23

Attention is currently required from: Brian Norris, Nikolai Artemiev. Hello Brian Norris, Edward O'Callaghan, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/75991?usp=email to look at the new patch set (#3). Change subject: flashrom: only perform WP unlock for write/erase operations ...................................................................... flashrom: only perform WP unlock for write/erase operations Don't unlock using WP for read/verify operations because WP will only disable write locks. Most chips don't have read locks anyway, but some do, so we still call the chip's unlock function for read/verify operations. Unconditionally unlocking using WP slows down flashrom significantly with some programmers, particularly linux_mtd due to inefficiency in the current kernel MTD interface. BUG=b:283779258 BRANCH=none TEST=`ninja test` TEST=`flashrom -{r,w,E,v}` on strongbad TEST=`flashrom --wp-enable; flashrom -{w,E}` on strongbad Change-Id: I5dc66474a0b7969b51b86ac9f5daa2c95ae968f1 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c 1 file changed, 42 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/91/75991/3 -- 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: 3 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-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom: only perform WP unlock for write/erase operations
by Nikolai Artemiev (Code Review) 04 Jul '23

04 Jul '23

Attention is currently required from: Brian Norris, Nikolai Artemiev. Hello Brian Norris, Edward O'Callaghan, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/75991?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by Brian Norris, Verified+1 by build bot (Jenkins) Change subject: flashrom: only perform WP unlock for write/erase operations ...................................................................... flashrom: only perform WP unlock for write/erase operations Don't unlock using WP for read/verify operations because WP will only disable write locks. Most chips don't have read locks anyway, but some do, so we still call the chip's unlock function for read/verify operations. Unconditionally unlocking using WP slows down flashrom significantly with some programmers, particularly linux_mtd due to inefficiency in the current kernel MTD interface. BUG=b:283779258 BRANCH=none TEST=`ninja test` TEST=`flashrom -{r,w,E,v}` on strongbad TEST=`flashrom --wp-enable; flashrom -{w,E}` on strongbad Change-Id: I5dc66474a0b7969b51b86ac9f5daa2c95ae968f1 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c 1 file changed, 38 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/91/75991/2 -- 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: 2 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-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: doc: Add Team page which describes Gerrit groups
by Alexander Goncharov (Code Review) 02 Jul '23

02 Jul '23

Attention is currently required from: Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/76075?usp=email ) Change subject: doc: Add Team page which describes Gerrit groups ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Great idea! I was just wondering why you added a new directory for it? At first glance, this page should be placed alongside with the Contribution guide / Contact page. Do you have any plans to add more pages here? -- To view, visit https://review.coreboot.org/c/flashrom/+/76075?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: I3118b2b036eab93e901814447543b02c760c6a80 Gerrit-Change-Number: 76075 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 02 Jul 2023 19:07:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: doc: Add documentation license
by Alexander Goncharov (Code Review) 02 Jul '23

02 Jul '23

Attention is currently required from: Anastasia Klimchuk, Patrick Georgi, Stefan Reinauer, Thomas Heijligen. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75727?usp=email ) Change subject: doc: Add documentation license ...................................................................... Patch Set 3: (2 comments) File doc/documentation_license.rst: https://review.coreboot.org/c/flashrom/+/75727/comment/45723609_59162025 : PS3, Line 2: flashrom documentation license How about just `Documentation license`? https://review.coreboot.org/c/flashrom/+/75727/comment/a703b5e2_c5586b63 : PS3, Line 5: doc/ We also have "Documentation" directory in our tree. As far as I know, we're going to get rid of it and move all the info to "docs". However, no idea when it happens, so it makes sense to add this directory as well. I suppose the same license is used there. -- To view, visit https://review.coreboot.org/c/flashrom/+/75727?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: Ied858b5f1e9c4a83a6eb21dcefb288c4474b08c0 Gerrit-Change-Number: 75727 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 02 Jul 2023 18:43:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips: add support for MX77L25650F chip
by Sergii Dmytruk (Code Review) 02 Jul '23

02 Jul '23

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 6: (5 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/68557/comment/dd4505cd_e2a5c70c : PS4, Line 11: can't disable it > can't enable it […] I think he meant that if WP is enabled, `flashrom` won't be able to disable it to perform write/erase. Patchset: PS4: > @ServError, since your patch is more complete, do you want to update this one? You would do that by […] I updated the patch. File flashchips.c: https://review.coreboot.org/c/flashrom/+/68557/comment/36baea18_0006743f : PS4, Line 10428: .vendor = "Macronix", > The values need to be tab-aligned, see the other entries. Done https://review.coreboot.org/c/flashrom/+/68557/comment/6243f628_42124f37 : PS4, Line 10449: // TODO: add support for flash protection > I don't think we need this TODO is the code. […] Quite a lot of other chips have TODOs. https://review.coreboot.org/c/flashrom/+/68557/comment/79900c22_20e57da7 : PS4, Line 10452: }, > The voltage is missing? Done -- 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: 6 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: ServError <admin(a)serverror.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Artur Kowalski <arturkow2000(a)gmail.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: Sun, 02 Jul 2023 15:54:38 +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> 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) 02 Jul '23

02 Jul '23

Attention is currently required from: Anastasia Klimchuk, Angel Pons, Artur Kowalski, ServError. Sergii Dmytruk has uploaded a new patch set (#5) to the change originally created by Artur Kowalski. ( https://review.coreboot.org/c/flashrom/+/68557?usp=email ) The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: flashchips: add support for MX77L25650F chip ...................................................................... flashchips: add support for MX77L25650F chip Add initial support for Macronix MX77L25650F. Can read, write and erase the chip. Change-Id: Iaea5485f8b59b8538dc47beada2c308376ea027c Signed-off-by: Artur Kowalski <artur.kowalski(a)3mdeb.com> Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Signed-off-by: ServError <admin(a)serverror.com> --- M flashchips.c M include/flashchips.h 2 files changed, 41 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/68557/5 -- 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: 5 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: ServError <admin(a)serverror.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Artur Kowalski <arturkow2000(a)gmail.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-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashchips: Add XTX XT25F64B
by Anastasia Klimchuk (Code Review) 02 Jul '23

02 Jul '23

Attention is currently required from: Angel Pons, Thomas Heijligen, WereCatf. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58173?usp=email ) Change subject: flashchips: Add XTX XT25F64B ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Hello WereCatf, I am sorry that your patches got lost :( I mean this one and CB:58134. Are you still here? (I hope so). Would you be able to rebase the manually on the top? If you make this one and CB:58134 as a chain, that would be even better. What happened since last time you pushed the patches: all function pointers in chip definitions were replaced with enums. So if you rebase on the latest head, it won't compile, you need to use enums in chip definitions. Would you do that? I will also look later in the datasheets. Sorry again this was hanging for so long. -- To view, visit https://review.coreboot.org/c/flashrom/+/58173?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: I369db9ccfd5319d28424d10f77aab49ec73a8836 Gerrit-Change-Number: 58173 Gerrit-PatchSet: 2 Gerrit-Owner: WereCatf <werecatf(a)outlook.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: WereCatf <werecatf(a)outlook.com> Gerrit-Comment-Date: Sun, 02 Jul 2023 08:53:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips: Add Macronix MX25R1635F
by Anastasia Klimchuk (Code Review) 02 Jul '23

02 Jul '23

Attention is currently required from: ra1nb0w. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69678?usp=email ) Change subject: flashchips: Add Macronix MX25R1635F ...................................................................... Patch Set 1: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/69678/comment/c891ae65_3672a846 : PS1, Line 13: Tested Can you mention which operation you tested, did you run verify at the end? (for example, from the chip definition it seems you ran probe/read/erase/write?) Patchset: PS1: Davide, I am so sorry that your patch got missed :\ I hope you are still here! Do you have a link to datasheets? File flashchips.c: https://review.coreboot.org/c/flashrom/+/69678/comment/55704117_1f7f9fe6 : PS1, Line 9306: .printlock = spi_prettyprint_status_register_bp3_srwd, /* bit 6 is quad enable */ : .unlock = spi_disable_blockprotect_bp3_srwd, These two are now enums are well (not function pointers anymore). If you rebase on the latest head you will see it does not compile anymore, because you need enum value here. -- To view, visit https://review.coreboot.org/c/flashrom/+/69678?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: Idce301ed90d6742b56e928068d201e5c3a2e5aee Gerrit-Change-Number: 69678 Gerrit-PatchSet: 1 Gerrit-Owner: ra1nb0w <rainbow(a)irh.it> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: ra1nb0w <rainbow(a)irh.it> Gerrit-Comment-Date: Sun, 02 Jul 2023 08:38:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashchips.c: Adding support for ISSI IS25WP020/40/80
by Anastasia Klimchuk (Code Review) 02 Jul '23

02 Jul '23

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: (1 comment) Patchset: PS4: Also when you will be changing the commit message, please also rebase the patch on the top, to make it fully ready (there was another change merged into flashchips last week, so this patch needs to be rebased). Thank you! -- 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: Sun, 02 Jul 2023 08:09:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips: Add support for GigaDevice GD251R512ME
by Anastasia Klimchuk (Code Review) 02 Jul '23

02 Jul '23

Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/69928?usp=email ) Change subject: flashchips: Add support for GigaDevice GD251R512ME ...................................................................... Abandoned There is another patch from the same owner, which covers this chip CB:58025. Let's finish CB:58025 -- To view, visit https://review.coreboot.org/c/flashrom/+/69928?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: I15f76ce089cb2a2baf8bf8525510a7a7fe44b352 Gerrit-Change-Number: 69928 Gerrit-PatchSet: 1 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Harsha B R <harsha.b.r(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: abandon

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit July 2023