Attention is currently required from: Alexandru Stan, 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)
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/84752/comment/ed44d148_c03d2437?us… :
PS2, Line 20239: 1024B total, 256B reserved
> Maybe the better comment is what's in the datasheet: […]
I have something! Another developer kindly explained to me about OTP (in another patch CB:84827), and I think we need to go by what datasheet says:
`OTP: 3X256B`
"Reserved" OTP is not mentioned in datasheet, so let's drop that part.
--
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-Comment-Date: Mon, 28 Oct 2024 10:06:08 +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>
Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim.
Anastasia Klimchuk has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/84827?usp=email )
Change subject: flashchips: add GD25F128F
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/84827/comment/2bf40960_c9e0d3a9?us… :
PS1, Line 8199: OTP: 1024B total, 256B reserved
> Thanks. I need to make a correction here. It is 3x2048 bytes. Each can be erased and be programed.
I see that you updated, so I mark it as resolved, right?
I don't have any other comments.
--
To view, visit https://review.coreboot.org/c/flashrom/+/84827?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: I14c0905f50e92492287d50f8377790b997c4acfe
Gerrit-Change-Number: 84827
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-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Mon, 28 Oct 2024 09:57:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Victor Lim <vlim(a)gigadevice.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Aarya, Bill XIE, Peter Marheine.
Anastasia Klimchuk has posted comments on this change by Bill XIE. ( https://review.coreboot.org/c/flashrom/+/84721?usp=email )
Change subject: erasure_layout: Add an option to sacrifice unchanged blocks for speed
......................................................................
Patch Set 10:
(1 comment)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/84721/comment/761482e9_716df6de?us… :
PS8, Line 124: " --sacrifice-ratio <ratio> Maximum %% memory that is allowed for redundant\n"
> And just right now, I realised manpage needs to be updated too! […]
So, I did all three things:
1) cli_classic.c , I combined the text from everyone! tell me what do you think
flashrom -h shows it, all good
2) doc/release_notes/devel.rst has the section, text copied from commit message
3) doc/classic_cli_manpage.rst also has the text, a bit longer than 1)
--
To view, visit https://review.coreboot.org/c/flashrom/+/84721?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: I154e8a713f626c37dbbe118db700055b96d24803
Gerrit-Change-Number: 84721
Gerrit-PatchSet: 10
Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.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-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Mon, 28 Oct 2024 08:28:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Attention is currently required from: Aarya, Bill XIE, Peter Marheine.
Anastasia Klimchuk has uploaded a new patch set (#10) to the change originally created by Bill XIE. ( https://review.coreboot.org/c/flashrom/+/84721?usp=email )
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: erasure_layout: Add an option to sacrifice unchanged blocks for speed
......................................................................
erasure_layout: Add an option to sacrifice unchanged blocks for speed
The patch adds command line option to handle the following situation:
There is a region which is requested to be erased (or written, because
the write operation uses erase too). Some of the areas inside this
region don't need to be erased, because the bytes already have expected
value. Such areas can be skipped.
The logic selects eraseblocks that can cover the areas which need to be
erased. Suppose there is a region which is partially covered by
eraseblocks of size S (partially because remaining areas don't need to
be erased). Now suppose we can cover the whole region with eraseblock
of larger size, S+1, and erase it all at once. This will run faster:
erase opcode will only be sent once instead of many smaller opcodes.
However, this will run erase over some areas of the chip memory that
didn't need to be erased. Which means, the chip, as a hardware, will
wear faster.
New command line option sets the maximum % memory that is allowed for
redundant erase. Default is 0, S+1 size block only selected if all the
area needs to be erased in full. 50 means that if more than a half of
the area needs to be erased, a S+1 size block can be selected to cover
all area with one block.
The tradeoff is the speed of programming operation VS the longevity of
the chip. Default is longevity.
Change-Id: I154e8a713f626c37dbbe118db700055b96d24803
Co-developed-by: persmule <persmule(a)hardenedlinux.org
Co-developed-by: Anastasia Klimchuk <aklm(a)flashrom.org>
Signed-off-by: persmule <persmule(a)hardenedlinux.org>
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M cli_classic.c
M doc/classic_cli_manpage.rst
M doc/release_notes/devel.rst
M erasure_layout.c
M include/flash.h
M tests/erase_func_algo.c
6 files changed, 152 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/84721/10
--
To view, visit https://review.coreboot.org/c/flashrom/+/84721?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: I154e8a713f626c37dbbe118db700055b96d24803
Gerrit-Change-Number: 84721
Gerrit-PatchSet: 10
Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.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-Attention: Peter Marheine <pmarheine(a)chromium.org>
Attention is currently required from: Aarya, Bill XIE, Peter Marheine.
Anastasia Klimchuk has posted comments on this change by Bill XIE. ( https://review.coreboot.org/c/flashrom/+/84721?usp=email )
Change subject: erasure_layout: Add an option to sacrifice unchanged blocks for speed
......................................................................
Patch Set 9:
(3 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/84721/comment/1f7af8a1_255511d3?us… :
PS8, Line 124: " --sacrifice-ratio <ratio> Maximum %% memory that is allowed for redundant\n"
> This is understandable, but reads somewhat awkwardly to me. Perhaps: […]
And just right now, I realised manpage needs to be updated too!
Also would be good to add info into release_notes/devel.html. I actually think to copy the long commit message that I wrote :) It can go as is (only without "this patch adds")
File erasure_layout.c:
https://review.coreboot.org/c/flashrom/+/84721/comment/514f3879_91d95a7f?us… :
PS8, Line 253: needless
> needing?
That was actually the blocks that not needed to be erased, which was "needless".
I did a modification to that comment text, to say directly that's `not needed to change`
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/84721/comment/4ee3e2e4_2e22d703?us… :
PS8, Line 1235: }
: },
: sizeof(*all_cases)
: );
: memcpy(
: &all_cases[num_parameterized * 2 + 1],
: (const struct CMUnitTest[]){
: (const struct CMUnitTest) {
: .name = "write with sacrifice ratio 50",
: .test_func = test_write_with_sacrifice_ratio50,
: }
: },
: sizeof(*all_cases)
: );
> The previous `memcpy` is already set up to copy an array of structs, so you can inline the new struc […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/84721?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: I154e8a713f626c37dbbe118db700055b96d24803
Gerrit-Change-Number: 84721
Gerrit-PatchSet: 9
Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.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-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Mon, 28 Oct 2024 05:47:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Attention is currently required from: Aarya, Anastasia Klimchuk, Bill XIE.
Anastasia Klimchuk has uploaded a new patch set (#9) to the change originally created by Bill XIE. ( https://review.coreboot.org/c/flashrom/+/84721?usp=email )
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: erasure_layout: Add an option to sacrifice unchanged blocks for speed
......................................................................
erasure_layout: Add an option to sacrifice unchanged blocks for speed
The patch adds command line option to handle the following situation:
There is a region which is requested to be erased (or written, because
the write operation uses erase too). Some of the areas inside this
region don't need to be erased, because the bytes already have expected
value. Such areas can be skipped.
The logic selects eraseblocks that can cover the areas which need to be
erased. Suppose there is a region which is partially covered by
eraseblocks of size S (partially because remaining areas don't need to
be erased). Now suppose we can cover the whole region with eraseblock
of larger size, S+1, and erase it all at once. This will run faster:
erase opcode will only be sent once instead of many smaller opcodes.
However, this will run erase over some areas of the chip memory that
didn't need to be erased. Which means, the chip, as a hardware, will
wear faster.
New command line option sets the maximum % memory that is allowed for
redundant erase. Default is 0, S+1 size block only selected if all the
area needs to be erased in full. 50 means that if more than a half of
the area needs to be erased, a S+1 size block can be selected to cover
all area with one block.
The tradeoff is the speed of programming operation VS the longevity of
the chip. Default is longevity.
Change-Id: I154e8a713f626c37dbbe118db700055b96d24803
Co-developed-by: persmule <persmule(a)hardenedlinux.org
Co-developed-by: Anastasia Klimchuk <aklm(a)flashrom.org>
Signed-off-by: persmule <persmule(a)hardenedlinux.org>
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M cli_classic.c
M erasure_layout.c
M include/flash.h
M tests/erase_func_algo.c
4 files changed, 113 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/84721/9
--
To view, visit https://review.coreboot.org/c/flashrom/+/84721?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: I154e8a713f626c37dbbe118db700055b96d24803
Gerrit-Change-Number: 84721
Gerrit-PatchSet: 9
Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Arnaud Ferraris.
Nikolai Artemiev has posted comments on this change by Arnaud Ferraris. ( https://review.coreboot.org/c/flashrom/+/84856?usp=email )
Change subject: linux_mtd: fix build with clang >= 19
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/84856?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: If470a65702e9ae08e4303123a0014e53a1fee56e
Gerrit-Change-Number: 84856
Gerrit-PatchSet: 1
Gerrit-Owner: Arnaud Ferraris <arnaud.ferraris(a)collabora.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Arnaud Ferraris <arnaud.ferraris(a)collabora.com>
Gerrit-Comment-Date: Sun, 27 Oct 2024 23:59:32 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Aarya, Anastasia Klimchuk, Bill XIE.
Peter Marheine has posted comments on this change by Bill XIE. ( https://review.coreboot.org/c/flashrom/+/84721?usp=email )
Change subject: erasure_layout: Add an option to sacrifice unchanged blocks for speed
......................................................................
Patch Set 8:
(3 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/84721/comment/59675f49_9e423cad?us… :
PS8, Line 124: " --sacrifice-ratio <ratio> Maximum %% memory that is allowed for redundant\n"
This is understandable, but reads somewhat awkwardly to me. Perhaps:
> Fraction (as a percentage, 0-50) of an erase block that may be erased even if unmodified. Larger values may complete programming faster, but may also hurt chip longevity by erasing cells unnecessarily. Default 0.
File erasure_layout.c:
https://review.coreboot.org/c/flashrom/+/84721/comment/c2b42544_bc111060?us… :
PS8, Line 253: needless
needing?
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/84721/comment/73021652_bf21c555?us… :
PS8, Line 1235: }
: },
: sizeof(*all_cases)
: );
: memcpy(
: &all_cases[num_parameterized * 2 + 1],
: (const struct CMUnitTest[]){
: (const struct CMUnitTest) {
: .name = "write with sacrifice ratio 50",
: .test_func = test_write_with_sacrifice_ratio50,
: }
: },
: sizeof(*all_cases)
: );
The previous `memcpy` is already set up to copy an array of structs, so you can inline the new struct there.
```suggestion
},
(const struct CMUnitTest) {
.name = "write with sacrifice ratio 50",
.test_func = test_write_with_sacrifice_ratio50,
},
},
sizeof(*all_cases) * num_unparameterized
);
```
--
To view, visit https://review.coreboot.org/c/flashrom/+/84721?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: I154e8a713f626c37dbbe118db700055b96d24803
Gerrit-Change-Number: 84721
Gerrit-PatchSet: 8
Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Sun, 27 Oct 2024 23:27:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Anastasia Klimchuk, Matti Finder.
Peter Marheine has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84840?usp=email )
Change subject: cli_classic: Add rpmc command support
......................................................................
Patch Set 3:
(2 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/84840/comment/b754461a_59e9f84e?us… :
PS2, Line 159: printf(".\n\nYou can specify one of -h, -R, -L, "
> Do you think the way I extended it is ok. Otherwise I think the message would get too long. […]
Yeah, that seems good. It's clear from how we put those in their own section what options the RPMC commands are.
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/84840/comment/4843cd38_e29f3eb8?us… :
PS3, Line 165: ", the RPMC commands"
"a RPMC command" since specifying more than one in a single invocation is an error.
--
To view, visit https://review.coreboot.org/c/flashrom/+/84840?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: I36c823bbee65f256eb6edabe6f058321c9a0cfa1
Gerrit-Change-Number: 84840
Gerrit-PatchSet: 3
Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Comment-Date: Sun, 27 Oct 2024 22:46:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Matti Finder <matti.finder(a)gmail.com>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Attention is currently required from: Anastasia Klimchuk.
Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84870?usp=email )
Change subject: doc: Add chip models support into recent development
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/84870?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: I9b96902a02b83d35f0a0f221bd1678b7edf99dad
Gerrit-Change-Number: 84870
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Sun, 27 Oct 2024 22:42:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes