Antonio Vázquez Blanco has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/84985?usp=email )
Change subject: Extract cli_output declarations to a separate header.
......................................................................
Abandoned
https://review.coreboot.org/c/flashrom/+/85134
--
To view, visit https://review.coreboot.org/c/flashrom/+/84985?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: I45d62bd219bdbed919788ae17a64aeb119a8aac4
Gerrit-Change-Number: 84985
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>
Attention is currently required from: Anastasia Klimchuk.
Antonio Vázquez Blanco has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/84985?usp=email )
Change subject: Extract cli_output declarations to a separate header.
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> The same comment as on the CB:84983 , but also, one more thing. […]
Got it, made the requested changes but unable to update this ticket and accidentally created a new one at https://review.coreboot.org/c/flashrom/+/85134.
--
To view, visit https://review.coreboot.org/c/flashrom/+/84985?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: I45d62bd219bdbed919788ae17a64aeb119a8aac4
Gerrit-Change-Number: 84985
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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 14 Nov 2024 08:37:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/85092?usp=email )
Change subject: sfdp: Update the message shown when SFDP-capable chip is detected
......................................................................
sfdp: Update the message shown when SFDP-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>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/85092
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Peter Marheine <pmarheine(a)chromium.org>
Reviewed-by: Matti Finder <matti.finder(a)gmail.com>
---
M flashrom.c
1 file changed, 7 insertions(+), 14 deletions(-)
Approvals:
build bot (Jenkins): Verified
Peter Marheine: Looks good to me, approved
Matti Finder: Looks good to me, approved
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: merged
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: If1a480ae78f158cc4626e345149ea9025443f8a7
Gerrit-Change-Number: 85092
Gerrit-PatchSet: 4
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: 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>
Anastasia Klimchuk has submitted this change. ( 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
SFDP 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>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/85075
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Reviewed-by: Matti Finder <matti.finder(a)gmail.com>
---
M flashchips.c
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Matti Finder: Looks good to me, approved
Sergii Dmytruk: Looks good to me, approved
diff --git a/flashchips.c b/flashchips.c
index 73b4036..0e83200 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 = { .probe = OK, .read = OK, .erase = OK, .write = OK, .wp = NA },
.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: merged
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I7e945389895a8042df3aaae72bccf73405be8651
Gerrit-Change-Number: 85075
Gerrit-PatchSet: 5
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: 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>
Michał Żygowski has posted comments on this change by Michał Żygowski. ( https://review.coreboot.org/c/flashrom/+/84987?usp=email )
Change subject: chipset_enable.c: Add TGL chipset detection based on SPI PCI ID
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> I didn't find a datasheet, but at least I found IDs 0xa0a4 and 0x43a4 on the internet as of being in […]
0x43a4 can be found here: https://cdrdv2-public.intel.com/635218/635218-008.pdf
0xa0a4 can be found here: https://www.intel.com/content/www/us/en/content-details/631119/intel-500-se…
--
To view, visit https://review.coreboot.org/c/flashrom/+/84987?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: Ie6859d81157760ca03fe34ba5ac311eba5a7c46b
Gerrit-Change-Number: 84987
Gerrit-PatchSet: 2
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Wed, 13 Nov 2024 11:35:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Daniel Campello, Edward O'Callaghan, Nico Huber, Peter Marheine, 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 6:
(1 comment)
File tests/chip.c:
https://review.coreboot.org/c/flashrom/+/64663/comment/e5b42e6a_606b99ea?us… :
PS6, Line 86: struct progress_user_data *progress_user_data = progress_state->user_data;
> I don't think we can change the API without either doing a whole ABI break (increment the soname or […]
Thanks heaps for all details! Very useful.
I thought of one more reason, apart of the question of mutability.
In the current code, we provide a parameter into progress callback, but then this value is not actually usable by external clients without one more API call? We know that callback needs progress state and user data, we can (should) give that as parameters straight away.
I guess this detail was missed when progress callback was introduced initially (at least I missed this), because our code, specifically tests and cli, can look into flash context. First paragraph of commit message explains the situation well.
So yes, I am going to try with progress callback v1 (this signature `void(flashrom_progress_callback)(enum flashrom_progress stage, size_t, size_t, void*)`)
and then `flashrom_get_progress_state` won't be needed anymore.
--
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: 6
Gerrit-Owner: Richard Hughes <richard(a)hughsie.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
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: Peter Marheine <pmarheine(a)chromium.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: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Wed, 13 Nov 2024 01:03:47 +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>