Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/19615?usp=email )
Change subject: Add probe_dummy for chips that are not practical to probe
......................................................................
Abandoned
The patch is on the staging branch (which we don't use anymore), and has not been touched for >5 years. If anyone is interested to resurrect and finish this work, the patch can be restored.
--
To view, visit https://review.coreboot.org/c/flashrom/+/19615?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: abandon
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Change-Id: I327400e337d6ce600c4f0f165f328715f5b341e2
Gerrit-Change-Number: 19615
Gerrit-PatchSet: 1
Gerrit-Owner: Urja Rannikko <urjaman(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Philippe Mathieu-Daudé <f4bug(a)amsat.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/19614?usp=email )
Change subject: cli_common: Dont request reports from known-bad chip operations
......................................................................
Abandoned
The patch is on the staging branch (which we don't use anymore), and has not been touched for >5 years. If anyone is interested to resurrect and finish this work, the patch can be restored.
--
To view, visit https://review.coreboot.org/c/flashrom/+/19614?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: abandon
Gerrit-Project: flashrom
Gerrit-Branch: staging
Gerrit-Change-Id: Ic4dc0d85534f2fcfe4dcb9733d4994c8853b48a7
Gerrit-Change-Number: 19614
Gerrit-PatchSet: 1
Gerrit-Owner: Urja Rannikko <urjaman(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Philippe Mathieu-Daudé <f4bug(a)amsat.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84960?usp=email )
Change subject: doc: Change link from gitiles to github
......................................................................
doc: Change link from gitiles to github
gitiles are not always available, so the link was not always working,
which could make readers confused.
Change-Id: I9103e5199bdf1b65fa3136aa01259a85e788a251
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M doc/about_flashrom/team.rst
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/60/84960/1
diff --git a/doc/about_flashrom/team.rst b/doc/about_flashrom/team.rst
index 215d930..94f922e 100644
--- a/doc/about_flashrom/team.rst
+++ b/doc/about_flashrom/team.rst
@@ -18,7 +18,7 @@
can do full approval of patches (i.e. vote +2).
In general, members of the group have some area of responsibility in the
-`MAINTAINERS <https://review.coreboot.org/plugins/gitiles/flashrom/+/refs/heads/main/MAIN…>`_ file,
+`MAINTAINERS <https://github.com/flashrom/flashrom/blob/main/MAINTAINERS>`_ file,
and are automatically added as reviewers to patches when the patch touches this area.
The responsibilities are the following.
--
To view, visit https://review.coreboot.org/c/flashrom/+/84960?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I9103e5199bdf1b65fa3136aa01259a85e788a251
Gerrit-Change-Number: 84960
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
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_client: Add rpmc command support
......................................................................
Patch Set 7:
(2 comments)
File doc/classic_cli_manpage.rst:
https://review.coreboot.org/c/flashrom/+/84840/comment/4a97349b_05a516ed?us… :
PS7, Line 329:
: RPMC commands
: ^^^^^^^^^^^^^
Perhaps you can add a little intro which mentions that commands are available only if chip model supports the feature.
File include/flash.h:
https://review.coreboot.org/c/flashrom/+/84840/comment/88a8dda6_808f048d?us… :
PS4, Line 581: rpmc_ctx
> This is sadly not as easily possible, since the chip reports itself with the same id as the W25Q128. […]
Ah, I think I understand now (also after reading CB:84934 with implementation).
So, the only way to use the feature in flashrom, would be to have the chip recognised as "SFDP-capable chip", is this correct?
Which means, models that are not yet defined in flashchips.c will "get" the feature by default (provided model supports rpmc).
However the models already defined in flashchips.c won't be able to use the feature. Unless the chip is somehow enforced to be handled as "SFDP-capable chip" instead of its own definition.
It might be even possible already, we have a `-c <chipname>` parameter, maybe it can be used as `-c "SFDP-capable chip"` . Looks awkward, but I want to fully understand the current situation.
There are a bunch of models already defined in flashchips.c which support rpmc. But since rpmc has not been implemented, the definitions had no indication of that.
For the long term, I am wondering how can we arrive to the solution so that all models which are capable, can use the feature. But this won't happen all in one commit of course.
I am marking as unresolved, just so that you won't miss and respond :) there is no action item right now.
--
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: 7
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: Sat, 02 Nov 2024 11:50:04 +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: Matti Finder, Peter Marheine.
Anastasia Klimchuk has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84934?usp=email )
Change subject: rpmc: add rpmc commands feature
......................................................................
Patch Set 3:
(7 comments)
Patchset:
PS3:
Given that you agreed in the other comment, you can add yourself in MAINTAINERS file and include in this patch. Add your entry in the CORE section, and in alphabetical order. I think better have full name of the feature (REPLAY PROTECTED MONOTONIC COUNTER). The file paths are rpmc.c and include/rpmc.h
What you are subscribing for, is described here: https://flashrom.org/about_flashrom/team.html ("flashrom reviewers" section).
Another doc to read (and maybe you read it already) is https://flashrom.org/dev_guide/development_guide.html
And once this patch is ready and approved, it will be submitted together with your addition to maintainers file! :)
File rpmc.c:
https://review.coreboot.org/c/flashrom/+/84934/comment/eaaa89b9_e2c2ba59?us… :
PS3, Line 131: Unsupported
Maybe `Unknown` instead of `Unsupported`, because it's not even in our enum.
https://review.coreboot.org/c/flashrom/+/84934/comment/9659cdaf_b4bca503?us… :
PS3, Line 223: {
You don't need { } when the body of if conditional is only one line.
(same in few more places in the code)
https://review.coreboot.org/c/flashrom/+/84934/comment/3e040fdc_38f3da54?us… :
PS3, Line 224: return -1;
I am adding comment here, as this is an example, but it applies to other code in this file.
Some functions in this file return -1 on error, and some return 1. Is there a reason for this inconsistency within the same file?
I haven't read the JESD260 document (but actually want to), but I understand the standard may require specific return values for operations. That's fine.
But, for functions which are `static int` in here, these are yours, is this right? So you can decide how to represent the error code.
Maybe if standard requires -1 on error, you can adopt this approach for all rpmc functions?
Also if -1 is required by standard, maybe create a macro for it RPMC_ERROR_CODE or something like that, and then use it everywhere.
File sfdp.c:
https://review.coreboot.org/c/flashrom/+/84934/comment/3758d1ed_fc51bb6c?us… :
PS3, Line 446: hdrs[i].id
Can these header IDs ever repeat from one header to the other? (probably not?)
For example, is id 0 always on header 0?
Is there any relation between `i` and `hdrs[i].id` ?
https://review.coreboot.org/c/flashrom/+/84934/comment/301e4aef_df73f51e?us… :
PS3, Line 449: msg_cdbg("The chip contains an unknown "
: "version of the JEDEC flash "
: "parameters table, skipping it.\n");
I know it was like this before, but while you are here... maybe you can add `hdrs[i].id` and `hdrs[i].v_major` to this debug message? Otherwise it's just saying "something went wrong" without any actual debug info.
(and same below)
https://review.coreboot.org/c/flashrom/+/84934/comment/559b34f2_5ca915fb?us… :
PS3, Line 461: 1
It's in hex few lines above (0x01 on line 448) and here is decimal. Same number, but let's use the same base.
--
To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38
Gerrit-Change-Number: 84934
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: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Sat, 02 Nov 2024 11:49:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/84856?usp=email )
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>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/84856
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Maximilian Brune <maximilian.brune(a)9elements.com>
Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M linux_mtd.c
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Anastasia Klimchuk: Looks good to me, approved
Maximilian Brune: Looks good to me, but someone else must approve
diff --git a/linux_mtd.c b/linux_mtd.c
index d5fc509..b1d0255 100644
--- a/linux_mtd.c
+++ b/linux_mtd.c
@@ -49,7 +49,7 @@
int i;
size_t bytes_read;
FILE *fp;
- char path[strlen(LINUX_MTD_SYSFS_ROOT) + 32];
+ char path[sizeof(LINUX_MTD_SYSFS_ROOT) + 31];
snprintf(path, sizeof(path), "%s/%s", sysfs_path, filename);
--
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: merged
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: If470a65702e9ae08e4303123a0014e53a1fee56e
Gerrit-Change-Number: 84856
Gerrit-PatchSet: 4
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>
Attention is currently required from: Anastasia Klimchuk, Peter Marheine.
Matti Finder has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84934?usp=email )
Change subject: rpmc: add rpmc commands feature
......................................................................
Patch Set 3:
(1 comment)
File include/rpmc.h:
https://review.coreboot.org/c/flashrom/+/84934/comment/cac341da_e675fc7e?us… :
PS1, Line 37: uint8_t status;
> I don't mind returning 0x80 on success, but the documentation comments should be clear on how to int […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38
Gerrit-Change-Number: 84934
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: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 15:25:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
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, Matti Finder.
Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/84934?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: rpmc: add rpmc commands feature
......................................................................
rpmc: add rpmc commands feature
Added optional support for all the commands specified in JESD260.
Added a new optional dependency to openssls libcrypto.
Added parsing for the rpmc parameter sfdp table.
Added necessary rpmc parameter information to flashchips struct and the
flash hardening feature to enable rpmc commands.
Enables future use of these commands in the cli_client and also
libflashrom.
Change-Id: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38
Signed-off-by: Matti Finder <matti.finder(a)gmail.com>
---
M include/flash.h
A include/rpmc.h
M meson.build
M meson_options.txt
A rpmc.c
M sfdp.c
6 files changed, 700 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/84934/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38
Gerrit-Change-Number: 84934
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>
Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84954?usp=email )
Change subject: doc: Add few sections to recent development doc
......................................................................
doc: Add few sections to recent development doc
Change-Id: Iedaca4a704c57c1db399c7888f743ad2961cbf9d
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M doc/release_notes/devel.rst
1 file changed, 54 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/84954/1
diff --git a/doc/release_notes/devel.rst b/doc/release_notes/devel.rst
index e0c42a5..1414dc8 100644
--- a/doc/release_notes/devel.rst
+++ b/doc/release_notes/devel.rst
@@ -67,6 +67,51 @@
The tradeoff is the speed of programming operation VS the longevity of
the chip. Default is longevity.
+Programmers updates
+===================
+
+* ch347_spi: Add spi clock frequency selection
+* dummyflasher: Enable higher frequency emulation, add docs and tests
+* ichspi: Change the opcode position for reprogramming on the fly 2->4
+* ichspi: Merge spi_master implementations for Intel ich
+
+Bugs fixes
+==========
+
+* Ensure verify operation completed in full if chip memory modified
+
+ Post-cleanup after processing unaligned region for the case when end
+ region needs to be extended to align with erase block. Writing was
+ done correctly, but post-processing of newcontents could cause
+ one-off offset at the end of the region, which would make
+ verification appear false-negative
+
+* erasure_layout: Fix init_eraseblock segmentation fault
+
+ Fix a segmentation fault that is caused by accessing an invalid
+ "subedata" pointer on the last iteration of the init_eraseblock loop.
+ Instead, short circuit the loop condition to check the sub block index
+ first, and do not access the invalid pointer if it is the last sub
+ block.
+
+ Issue was encountered in OpenBSD.
+
+* Fix FEATURE_NO_ERASE chips and add test for them
+
+ https://ticket.coreboot.org/issues/553
+
+* build: never install cmocka
+
+ https://ticket.coreboot.org/issues/561
+
+* ichspi: Probe opcode in POSSIBLE_OPCODES[] as well
+
+ https://ticket.coreboot.org/issues/556
+
+* erasure_layout: Erase larger block only when all sub-block need erase
+
+* erase/write: Deselect all smaller blocks when large block is selected
+
Chipset support
===============
@@ -114,3 +159,12 @@
* XM25QU256D
* XM25QU512C
* XM25QU512D
+
+Misc
+=========
+
+* reduce DELAY_MINIMUM_SLEEP_US to 100 by default
+* tests: Add assert for eraseblocks order of invocations for write op
+* VERSION: Change name pattern to match tag name from now on
+* writeprotect: Fix inaccurate return code
+* erasure_layout: Fix unreachable error message
--
To view, visit https://review.coreboot.org/c/flashrom/+/84954?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Iedaca4a704c57c1db399c7888f743ad2961cbf9d
Gerrit-Change-Number: 84954
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Angel Pons, Daniel Campello, Edward O'Callaghan, Nico Huber, Richard Hughes.
Anastasia Klimchuk has posted comments on this change by Richard Hughes. ( https://review.coreboot.org/c/flashrom/+/64663?usp=email )
Change subject: libflashrom: Allow getting the progress_state from the flashctx
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
I have good news: with CB:84102 already merged, and CB:84439 will be soon, we now can finish the work to make progress available via libflashrom!
It was a while ago, I forgot what it was in this patch, but I will review it again. Also cli_output.c has changed, so this patch will need to be rebased.
--
To view, visit https://review.coreboot.org/c/flashrom/+/64663?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: I322bf56ff92f7b4d0ffc92768e9f0cdf7cb82010
Gerrit-Change-Number: 64663
Gerrit-PatchSet: 3
Gerrit-Owner: Richard Hughes <richard(a)hughsie.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
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-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Comment-Date: Fri, 01 Nov 2024 09:50:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No