Attention is currently required from: 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_client: Add rpmc command support
......................................................................
Patch Set 14: Code-Review+2
--
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: 14
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-Comment-Date: Sun, 10 Nov 2024 22:28:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Anastasia Klimchuk, Matti Finder.
Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/85092?usp=email )
Change subject: sfdp: Update the message shown when SDFP-capable chip is detected
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/85092?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: If1a480ae78f158cc4626e345149ea9025443f8a7
Gerrit-Change-Number: 85092
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Matti Finder <matti.finder(a)gmail.com>
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, 10 Nov 2024 22:25:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Attention is currently required from: Matti Finder, Nikolai Artemiev, Sergii Dmytruk, Stefan Reinauer.
Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/85075?usp=email )
Change subject: flashchips: Skip "WP untested" message for SFDP-capable chip
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
The comment just above the line with tested status indicates that the idea always was to mark everything tested. It's just that support for `B` in `TEST_OK_PREWB` was implemented much later (later than the comment), but should have been here.
Additional context, is that the message itself is upgraded in the next patch CB:85092
--
To view, visit https://review.coreboot.org/c/flashrom/+/85075?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: I7e945389895a8042df3aaae72bccf73405be8651
Gerrit-Change-Number: 85075
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>
Gerrit-Comment-Date: Sun, 10 Nov 2024 20:48:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/85092?usp=email
to look at the new patch set (#2).
Change subject: sfdp: Update the message shown when SDFP-capable chip is detected
......................................................................
sfdp: Update the message shown when SDFP-capable chip is detected
Testing:
flashrom -p dummy:emulate=MX25L6436 -c "SFDP-capable chip"
Change-Id: If1a480ae78f158cc4626e345149ea9025443f8a7
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M flashrom.c
1 file changed, 7 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/92/85092/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/85092?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: If1a480ae78f158cc4626e345149ea9025443f8a7
Gerrit-Change-Number: 85092
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.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/+/85092?usp=email )
Change subject: sfdp: Update the message shown when SDFP-capable chip is detected
......................................................................
sfdp: Update the message shown when SDFP-capable chip is detected
Change-Id: If1a480ae78f158cc4626e345149ea9025443f8a7
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M flashrom.c
1 file changed, 7 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/92/85092/1
diff --git a/flashrom.c b/flashrom.c
index 4ae9390..8daad74 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1159,25 +1159,18 @@
* been found on this interface.
*/
if (startchip == 0 && flash->chip->model_id == SFDP_DEVICE_ID) {
- msg_cinfo("===\n"
- "SFDP has autodetected a flash chip which is "
- "not natively supported by flashrom yet.\n");
+ msg_cinfo("===\nSFDP has autodetected a flash chip.\n");
if (count_usable_erasers(flash) == 0)
msg_cinfo("The standard operations read and "
- "verify should work, but to support "
- "erase, write and all other "
- "possible features");
+ "verify should work, but support for "
+ "erase and write needs to be added manually.\n");
else
msg_cinfo("All standard operations (read, "
- "verify, erase and write) should "
- "work, but to support all possible "
- "features");
+ "verify, erase and write) should work.\n");
- msg_cinfo(" we need to add them manually.\n"
- "You can help us by mailing us the output of the following command to "
- "flashrom@flashrom.org:\n"
- "'flashrom -VV [plus the -p/--programmer parameter]'\n"
- "Thanks for your help!\n"
+ msg_cinfo("Additionally, flashrom supports RPMC commands via SFDP autodetection.\n"
+ "We may add support for more features via SFDP in future.\n"
+ "If you are interested, join us on the mailing list https://flashrom.org/contact.html#mailing-list-1\n"
"===\n");
}
--
To view, visit https://review.coreboot.org/c/flashrom/+/85092?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: If1a480ae78f158cc4626e345149ea9025443f8a7
Gerrit-Change-Number: 85092
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Nikolai Artemiev, Stefan Reinauer.
Hello Nikolai Artemiev, Stefan Reinauer,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/85075?usp=email
to look at the new patch set (#2).
Change subject: flashchips: Skip "WP untested" message for SFDP-capable chip
......................................................................
flashchips: Skip "WP untested" message for SFDP-capable chip
This entry in the flashchips represent a "SFDP-capable chip" and
it doesn't make sense to show the message "WP operation has status
untested, please report this". The entry can cover any generic
SDFP chip and what would you report?
Secondly, the entry "SFDP-capable chip" does not currently support
WP operations anyway.
Going further, we will be working with SFDP way more, so this area
needs to be gradually upgraded.
Testing:
flashrom -p dummy:emulate=MX25L6436 -c "SFDP-capable chip" -r dump.rom
Change-Id: I7e945389895a8042df3aaae72bccf73405be8651
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M flashchips.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/85075/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/85075?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: I7e945389895a8042df3aaae72bccf73405be8651
Gerrit-Change-Number: 85075
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/85075?usp=email )
Change subject: flashchips: Skip "WP untested" message for SFDP-capable chip
......................................................................
flashchips: Skip "WP untested" message for SFDP-capable chip
This entry in the flashchips represent a "SFDP-capable chip" and
it doesn't make sense to show the message "WP operation has status
untested, please report this". The entry can cover any generic
SDFP chip and what would you report?
Secondly, the entry "SFDP-capable chip" does not currently support
WP operations anyway.
Going further, we will be working with SFDP way more, so this area
needs to be gradually upgraded.
Change-Id: I7e945389895a8042df3aaae72bccf73405be8651
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M flashchips.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/85075/1
diff --git a/flashchips.c b/flashchips.c
index 742e017..17b5fe8 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -23522,7 +23522,7 @@
/* We present our own "report this" text hence we do not */
/* want the default "This flash part has status UNTESTED..." */
/* text to be printed. */
- .tested = TEST_OK_PREW,
+ .tested = TEST_OK_PREWB,
.probe = PROBE_SPI_SFDP,
.block_erasers = {}, /* set by probing function */
.unlock = SPI_DISABLE_BLOCKPROTECT, /* is this safe? */
--
To view, visit https://review.coreboot.org/c/flashrom/+/85075?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: I7e945389895a8042df3aaae72bccf73405be8651
Gerrit-Change-Number: 85075
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Anastasia Klimchuk has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/84983?usp=email )
Change subject: Rename cli_classic.h to a more adequate cli_getop.h
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> That fully makes sense. Thanks for the feedback. […]
No problem! I did review for your new patch.
Gerrit tracks changes by `Change-Id:` which you can see from commit message. When you push a patch, before running `git push` you can check what is the Change-Id in commit message, is it the same as your existing patch or a new one.
Since this is a part of commit message, if you do `git commit --amend` it will stay, if you create a new commit then a new Change-Id will be generated.
--
To view, visit https://review.coreboot.org/c/flashrom/+/84983?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: I5986828f3bdec583af6f7b02cbe1a9c45ed2000f
Gerrit-Change-Number: 84983
Gerrit-PatchSet: 2
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Sat, 09 Nov 2024 11:50:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Antonio Vázquez Blanco.
Anastasia Klimchuk has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/85072?usp=email )
Change subject: Rename cli_classic.h to a more adequate cli_getop.h
......................................................................
Patch Set 1:
(2 comments)
File cli_getopt.c:
https://review.coreboot.org/c/flashrom/+/85072/comment/7f31a2e3_e17b61c3?us… :
PS1, Line 28: #include "cli_getopt.h"
I noticed you put this line on the top, why? Usually the style we have is: first come includes in <> and then in ""
Given that cli_getopt.h is replacement for cli_classic.h, probably keep it on the same line.
https://review.coreboot.org/c/flashrom/+/85072/comment/c23f746e_137f0513?us… :
PS1, Line 35: // msg_gerr
I noticed you are adding this comments for includes of flash.h, and while the comment is true, but I don't think we need it. There are so many includes, and we won't be commenting on each one anyway.
--
To view, visit https://review.coreboot.org/c/flashrom/+/85072?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: Iaceeabedc26e93147d8122376d88e730aad1e355
Gerrit-Change-Number: 85072
Gerrit-PatchSet: 1
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Comment-Date: Sat, 09 Nov 2024 11:37:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Antonio Vázquez Blanco has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/84983?usp=email )
Change subject: Rename cli_classic.h to a more adequate cli_getop.h
......................................................................
Abandoned
New ticket at https://review.coreboot.org/c/flashrom/+/85072
--
To view, visit https://review.coreboot.org/c/flashrom/+/84983?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: main
Gerrit-Change-Id: I5986828f3bdec583af6f7b02cbe1a9c45ed2000f
Gerrit-Change-Number: 84983
Gerrit-PatchSet: 2
Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>