Attention is currently required from: Arnaud Ferraris, Nikolai Artemiev.
Anastasia Klimchuk 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 3: 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: 3
Gerrit-Owner: Arnaud Ferraris <arnaud.ferraris(a)collabora.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
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-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Tue, 29 Oct 2024 23:00:56 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Matti Finder, Peter Marheine.
Anastasia Klimchuk 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 4:
(1 comment)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/84840/comment/2ae7e1e9_564bccff?us… :
PS4, Line 1323: if (options.rpmc_read_data)
: ret = rpmc_read_data(fill_flash);
> Making it API-friendly sounds good, but I would hold off on adding this support to the public API si […]
I think having code in a separate file rpmc.c is better, you don't need to move it. Keep rpmc.c and include/rpmc.h as separate files.
When it comes to libflashrom, you will call functions from rpmc.c in the libflashrom.c.
As existing example, write-protect functionality designed like this. You can look at the code, in libflashrom.c it has prefix `flashrom_wp`
--
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: 4
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: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Tue, 29 Oct 2024 23:00:31 +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>
Comment-In-Reply-To: Matti Finder <matti.finder(a)gmail.com>
Attention is currently required from: Anastasia Klimchuk.
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 4:
(1 comment)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/84840/comment/fdf5ab80_6c77bc8b?us… :
PS4, Line 1323: if (options.rpmc_read_data)
: ret = rpmc_read_data(fill_flash);
> I agree I think the stuff from rpmc.c should rather be in libflashrom. […]
Making it API-friendly sounds good, but I would hold off on adding this support to the public API since it would be difficult to make backward-compatible changes if we find issues.
--
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: 4
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-Comment-Date: Tue, 29 Oct 2024 22:39:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Matti Finder <matti.finder(a)gmail.com>
Attention is currently required from: Anastasia Klimchuk.
Matti Finder 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 4:
(1 comment)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/84840/comment/8ba2cb37_837095b6?us… :
PS4, Line 1323: if (options.rpmc_read_data)
: ret = rpmc_read_data(fill_flash);
> I am thinking, and maybe this is more the question to Peter, what do you think, […]
I agree I think the stuff from rpmc.c should rather be in libflashrom. I'm gonna move my code over there, and make it more api friendly. And then seperate the commits.
--
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: 4
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-Comment-Date: Tue, 29 Oct 2024 17:05:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Anastasia Klimchuk, Arnaud Ferraris, Nikolai Artemiev.
Maximilian Brune 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 3: Code-Review+1
--
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: 3
Gerrit-Owner: Arnaud Ferraris <arnaud.ferraris(a)collabora.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
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-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 29 Oct 2024 15:21:18 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Anastasia Klimchuk, Maximilian Brune, Nikolai Artemiev.
Arnaud Ferraris 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 3:
(1 comment)
File linux_mtd.c:
https://review.coreboot.org/c/flashrom/+/84856/comment/51516784_3a557db1?us… :
PS2, Line 52: char path[sizeof(LINUX_MTD_SYSFS_ROOT) + 32];
> Oh I got worried that I missed something. […]
That's correct, `\0` is also counted by `sizeof`.
Amended to keep the same size as previously.
--
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: 3
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-CC: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 29 Oct 2024 08:10:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Anastasia Klimchuk, Arnaud Ferraris, Maximilian Brune, Nikolai Artemiev.
Hello Anastasia Klimchuk, Nikolai Artemiev, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/84856?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Code-Review+2 by Anastasia Klimchuk, Code-Review+2 by Nikolai Artemiev, Verified+1 by build bot (Jenkins)
The change is no longer submittable: Code-Review and Verified are unsatisfied now.
Change subject: linux_mtd: fix build with clang >= 19
......................................................................
linux_mtd: fix build with clang >= 19
Starting with version 19, clang issues a warning when using `strlen()`
for initializing a static array's size. This causes the build to fail as
the project also sets `-Werror`.
This is fixed by using `sizeof()` instead, which is guaranteed to be
evaluated at compilation time and therefore not triggering the
problematic warning.
Change-Id: If470a65702e9ae08e4303123a0014e53a1fee56e
Signed-off-by: Arnaud Ferraris <arnaud.ferraris(a)collabora.com>
---
M linux_mtd.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/56/84856/3
--
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: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: If470a65702e9ae08e4303123a0014e53a1fee56e
Gerrit-Change-Number: 84856
Gerrit-PatchSet: 3
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-CC: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Arnaud Ferraris <arnaud.ferraris(a)collabora.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
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 11: Code-Review+2
--
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: 11
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: Tue, 29 Oct 2024 08:03:09 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
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 11:
(2 comments)
File doc/classic_cli_manpage.rst:
https://review.coreboot.org/c/flashrom/+/84721/comment/3b927795_124d3b60?us… :
PS10, Line 23: | [-V[V[V]]] [-o <logfile>] [--progress][--sacrifice-ratio <ratio>]
> Add a space between flags. […]
Done
File doc/release_notes/devel.rst:
https://review.coreboot.org/c/flashrom/+/84721/comment/9b4e0305_28deaaa5?us… :
PS10, Line 57: 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.
> Small rewording suggestions. […]
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: 11
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: Tue, 29 Oct 2024 05:59:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>