May 2025 - flashrom-gerrit - mail.coreboot.org

[XS] Change in flashrom[main]: Add more delay time for jlink power on feature to stabilize the LDO a...
by Anastasia Klimchuk (Code Review) May 25, 2025

May 25, 2025

Attention is currently required from: attila-v. Anastasia Klimchuk has posted comments on this change by attila-v. ( https://review.coreboot.org/c/flashrom/+/86085?usp=email ) Change subject: Add more delay time for jlink power on feature to stabilize the LDO and the decoupling capacitors ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/86085/comment/db3dda82_d241be66?us… : PS2, Line 7: Add more delay time for jlink power on feature to stabilize the LDO and the decoupling capacitors > For this, I can give a suggestion, you can use it or compose a different one (just make sure it wrap […] Attila, I wanted to check, are you planning to fix the commit message? If you are busy I can do that (if you agree with my wording) -- To view, visit https://review.coreboot.org/c/flashrom/+/86085?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: Ic2dd94e99ac6ffa17a009b8488ce027698ae2c28 Gerrit-Change-Number: 86085 Gerrit-PatchSet: 3 Gerrit-Owner: attila-v Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: attila-v Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: attila-v Gerrit-Comment-Date: Sun, 25 May 2025 11:48:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: fmap: Skip unreadable regions when attempting to locate FMAP
by Anastasia Klimchuk (Code Review) May 23, 2025

May 23, 2025

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/87748?usp=email ) Change subject: fmap: Skip unreadable regions when attempting to locate FMAP ...................................................................... fmap: Skip unreadable regions when attempting to locate FMAP When using the on-chip FMAP to restrict flashrom operations to one or more regions (--fmap), flashrom must first locate the FMAP. This requires flashrom to read from multiple addresses, some of which may be located in regions which are not readable (such as the Intel ME region). In order to avoid a substantial amount of output from read_flash() when trying to locate the FMAP in these regions, set `the skip_unreadable_regions` flag before performing any reads to locate the FMAP, and restore the original flag value when finished. This resolves https://ticket.coreboot.org/issues/587 TEST=build flashrom, use cli to update COREBOOT and EC FMAP regions on an Intel Alderlake-N platform board (starlabs/starlite_adl). Change-Id: Ie78b977b4e6a5db02a25e69603f697834043ea99 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/87748 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M fmap.c 1 file changed, 11 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/fmap.c b/fmap.c index 50cb969..7081598 100644 --- a/fmap.c +++ b/fmap.c @@ -190,6 +190,7 @@ struct fmap *fmap; const unsigned int chip_size = flashctx->chip->total_size * 1024; const int sig_len = strlen(FMAP_SIGNATURE); + bool skip_unreadable_orig = flashctx->flags.skip_unreadable_regions; if (rom_offset + len > flashctx->chip->total_size * 1024) return 1; @@ -207,6 +208,13 @@ } /* + * Ignore unreadable regions when trying to locate the FMAP, in order + * to avoid a large amount of false-error prints to stderr. Unreadable + * regions have already reported so errors here are redundant. + */ + flashctx->flags.skip_unreadable_regions = true; + + /* * For efficient operation, we start with the largest stride possible * and then decrease the stride on each iteration. Also, check for a * remainder when modding the offset with the previous stride. This @@ -264,6 +272,9 @@ break; } + /* Restore original flag, since we're done searching */ + flashctx->flags.skip_unreadable_regions = skip_unreadable_orig; + if (!fmap_found) goto _free_ret; -- To view, visit https://review.coreboot.org/c/flashrom/+/87748?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: Ie78b977b4e6a5db02a25e69603f697834043ea99 Gerrit-Change-Number: 87748 Gerrit-PatchSet: 2 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in flashrom[main]: fmap: Skip unreadable regions when attempting to locate FMAP
by Anastasia Klimchuk (Code Review) May 20, 2025

May 20, 2025

Attention is currently required from: Matt DeVillier. Anastasia Klimchuk has posted comments on this change by Matt DeVillier. ( https://review.coreboot.org/c/flashrom/+/87748?usp=email ) Change subject: fmap: Skip unreadable regions when attempting to locate FMAP ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/87748?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: Ie78b977b4e6a5db02a25e69603f697834043ea99 Gerrit-Change-Number: 87748 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Comment-Date: Mon, 19 May 2025 22:26:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: fmap: Skip unreadable regions when attempting to locate FMAP
by Matt DeVillier (Code Review) May 19, 2025

May 19, 2025

Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/87748?usp=email ) Change subject: fmap: Skip unreadable regions when attempting to locate FMAP ...................................................................... fmap: Skip unreadable regions when attempting to locate FMAP When using the on-chip FMAP to restrict flashrom operations to one or more regions (--fmap), flashrom must first locate the FMAP. This requires flashrom to read from multiple addresses, some of which may be located in regions which are not readable (such as the Intel ME region). In order to avoid a substantial amount of output from read_flash() when trying to locate the FMAP in these regions, set `the skip_unreadable_regions` flag before performing any reads to locate the FMAP, and restore the original flag value when finished. This resolves https://ticket.coreboot.org/issues/587 TEST=build flashrom, use cli to update COREBOOT and EC FMAP regions on an Intel Alderlake-N platform board (starlabs/starlite_adl). Change-Id: Ie78b977b4e6a5db02a25e69603f697834043ea99 Signed-off-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M fmap.c 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/87748/1 diff --git a/fmap.c b/fmap.c index 50cb969..7081598 100644 --- a/fmap.c +++ b/fmap.c @@ -190,6 +190,7 @@ struct fmap *fmap; const unsigned int chip_size = flashctx->chip->total_size * 1024; const int sig_len = strlen(FMAP_SIGNATURE); + bool skip_unreadable_orig = flashctx->flags.skip_unreadable_regions; if (rom_offset + len > flashctx->chip->total_size * 1024) return 1; @@ -207,6 +208,13 @@ } /* + * Ignore unreadable regions when trying to locate the FMAP, in order + * to avoid a large amount of false-error prints to stderr. Unreadable + * regions have already reported so errors here are redundant. + */ + flashctx->flags.skip_unreadable_regions = true; + + /* * For efficient operation, we start with the largest stride possible * and then decrease the stride on each iteration. Also, check for a * remainder when modding the offset with the previous stride. This @@ -264,6 +272,9 @@ break; } + /* Restore original flag, since we're done searching */ + flashctx->flags.skip_unreadable_regions = skip_unreadable_orig; + if (!fmap_found) goto _free_ret; -- To view, visit https://review.coreboot.org/c/flashrom/+/87748?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: Ie78b977b4e6a5db02a25e69603f697834043ea99 Gerrit-Change-Number: 87748 Gerrit-PatchSet: 1 Gerrit-Owner: Matt DeVillier <matt.devillier(a)gmail.com>

1 0

[S] Change in flashrom[main]: doc/devel: Add info about new libflashrom APIs
by Anastasia Klimchuk (Code Review) May 16, 2025

May 16, 2025

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/87644?usp=email ) Change subject: doc/devel: Add info about new libflashrom APIs ...................................................................... doc/devel: Add info about new libflashrom APIs Change-Id: I98524353b8bedff3364568636ed42a4bd6ce8b3d Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/87644 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M doc/release_notes/devel.rst 1 file changed, 39 insertions(+), 4 deletions(-) Approvals: Peter Marheine: Looks good to me, approved build bot (Jenkins): Verified diff --git a/doc/release_notes/devel.rst b/doc/release_notes/devel.rst index 2a2870d..ef8364d 100644 --- a/doc/release_notes/devel.rst +++ b/doc/release_notes/devel.rst @@ -52,7 +52,15 @@ to ``enabled`` in which case the build will fail if the requirements are not satisfied. -New libflashrom API for progress reporting +Programmer updates +------------------ + +* spidriver: Add support for the Excamera Labs SPIDriver + +libflashrom API updates +======================= + +New API for progress reporting ------------------------------------------ The old ``flashrom_set_progress_callback`` function for requesting progress updates @@ -65,7 +73,34 @@ define their own global state to track progress, and it was impossible to fix that issue while maintaining binary compatibility without adding a new API. -Programmer updates ------------------- +New API for logging and setting log level +----------------------------------------- -* spidriver: Add support for the Excamera Labs SPIDriver +New API ``flashrom_set_log_callback_v2`` is added which allows the users to provide +a pointer to user data, the latter is managed by the caller of the API. New API also +introduces a new signature for log callback function, ``flashrom_log_callback_v2``. + +New API supports setting the log level for messages from libflashrom, ``flashrom_set_log_level``. +The log callback is invoked only for the messages with log level less or equal than the one +requested (i.e. only for the messages at least as urgent as the level requested). + +Old API ``flashrom_set_log_callback`` without user data continues to work as before. + +New API for probing flashchips +------------------------------ + +New API for probing is added ``flashrom_flash_probe_v2`` which can (if requested) +go through all known flashchips and find all matches. v2 returns the number of matches +found (or 0 if none found) and the list of names of all matched entries. + +``flashrom_flash_probe_v2`` continues to support an optional parameter ``chip_name`` +if the caller want to probe for only one specific chip with given name. + +Old API ``flashrom_flash_probe`` which stops probing if more than one chip entry matches +continues to work as before. + +New API to get list of supported programmers +-------------------------------------------- + +New API ``flashrom_supported_programmers`` returns the list of all programmers that are +supported on a current run environment. -- To view, visit https://review.coreboot.org/c/flashrom/+/87644?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: I98524353b8bedff3364568636ed42a4bd6ce8b3d Gerrit-Change-Number: 87644 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in flashrom[main]: doc/devel: Add info about new libflashrom APIs
by Peter Marheine (Code Review) May 13, 2025

May 13, 2025

Attention is currently required from: Anastasia Klimchuk, Dmitry Zhadinets. Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87644?usp=email ) Change subject: doc/devel: Add info about new libflashrom APIs ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/87644?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: I98524353b8bedff3364568636ed42a4bd6ce8b3d Gerrit-Change-Number: 87644 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 12 May 2025 23:50:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: cli: Use flashrom_supported_programmers API in cli
by Anastasia Klimchuk (Code Review) May 12, 2025

May 12, 2025

Attention is currently required from: Dmitry Zhadinets. Anastasia Klimchuk has posted comments on this change by Dmitry Zhadinets. ( https://review.coreboot.org/c/flashrom/+/87195?usp=email ) Change subject: cli: Use flashrom_supported_programmers API in cli ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > I just thought about something, it's for the separate commit. […] I made a patch and added all updated for libflashrom API in one patch CB:87644 (added you as reviewer) -- To view, visit https://review.coreboot.org/c/flashrom/+/87195?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: I86ac83ee0ef4e850e69aa4e7b607e28ebab9d3d5 Gerrit-Change-Number: 87195 Gerrit-PatchSet: 2 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(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: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Comment-Date: Mon, 12 May 2025 12:17:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XS] Change in flashrom[main]: Add more delay time for jlink power on feature to stabilize the LDO a...
by Anastasia Klimchuk (Code Review) May 12, 2025

May 12, 2025

Attention is currently required from: attila-v. Anastasia Klimchuk has posted comments on this change by attila-v. ( https://review.coreboot.org/c/flashrom/+/86085?usp=email ) Change subject: Add more delay time for jlink power on feature to stabilize the LDO and the decoupling capacitors ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: I rebased to make Jenkins re-run, it's successful now. -- To view, visit https://review.coreboot.org/c/flashrom/+/86085?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: Ic2dd94e99ac6ffa17a009b8488ce027698ae2c28 Gerrit-Change-Number: 86085 Gerrit-PatchSet: 3 Gerrit-Owner: attila-v Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: attila-v Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: attila-v Gerrit-Comment-Date: Mon, 12 May 2025 12:15:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in flashrom[main]: Add more delay time for jlink power on feature to stabilize the LDO a...
by Anastasia Klimchuk (Code Review) May 12, 2025

May 12, 2025

Attention is currently required from: attila-v. Anastasia Klimchuk has posted comments on this change by attila-v. ( https://review.coreboot.org/c/flashrom/+/86085?usp=email ) Change subject: Add more delay time for jlink power on feature to stabilize the LDO and the decoupling capacitors ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/86085/comment/bf8a2ea6_131cc4d1?us… : PS2, Line 7: Add more delay time for jlink power on feature to stabilize the LDO and the decoupling capacitors For this, I can give a suggestion, you can use it or compose a different one (just make sure it wraps at 72 chars) title (first line) > jlink_spi: Increase delay on power on feature from 10 to 100 millisec and then a longer detailed description like this for example > More delay time is needed to to stabilize the LDO and the decoupling capacitors when power on feature is enabled. Specifically 100 ms was tested on HT7333 and it's sufficient. > I used the power supply from the J-Link device pin 19. this power is 5V, I put a 3.3V LDO (HT7333) after it, and this LDO (and decoupling capacitors) need more time to fix output voltage. File jlink_spi.c: https://review.coreboot.org/c/flashrom/+/86085/comment/e3b85a07_5c19de94?us… : PS2, Line 469: internal_sleep(100000); > I think it will be enoughwthis small delay patch for now, nobody will sense this little 0.1 seconds delay which I think is definitely enough. That's good! Do I understand correctly, you think we can just go ahead with this change? It's easier if parameter is not needed, then all the code you done already. For me, yes it seems 0.1s is small enough, we can have it. One thing to do is to update commit message: write a good commit title and message which explains the reason. I did my suggestion (in the other comment) but you can make corrections or add details of you want. -- To view, visit https://review.coreboot.org/c/flashrom/+/86085?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: Ic2dd94e99ac6ffa17a009b8488ce027698ae2c28 Gerrit-Change-Number: 86085 Gerrit-PatchSet: 2 Gerrit-Owner: attila-v Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: attila-v Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: attila-v Gerrit-Comment-Date: Mon, 12 May 2025 12:06:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: attila-v

1 0

[S] Change in flashrom[main]: doc/devel: Add info about new libflashrom APIs
by Anastasia Klimchuk (Code Review) May 12, 2025

May 12, 2025

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/87644?usp=email ) Change subject: doc/devel: Add info about new libflashrom APIs ...................................................................... doc/devel: Add info about new libflashrom APIs Change-Id: I98524353b8bedff3364568636ed42a4bd6ce8b3d Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/release_notes/devel.rst 1 file changed, 39 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/87644/1 diff --git a/doc/release_notes/devel.rst b/doc/release_notes/devel.rst index 2a2870d..ef8364d 100644 --- a/doc/release_notes/devel.rst +++ b/doc/release_notes/devel.rst @@ -52,7 +52,15 @@ to ``enabled`` in which case the build will fail if the requirements are not satisfied. -New libflashrom API for progress reporting +Programmer updates +------------------ + +* spidriver: Add support for the Excamera Labs SPIDriver + +libflashrom API updates +======================= + +New API for progress reporting ------------------------------------------ The old ``flashrom_set_progress_callback`` function for requesting progress updates @@ -65,7 +73,34 @@ define their own global state to track progress, and it was impossible to fix that issue while maintaining binary compatibility without adding a new API. -Programmer updates ------------------- +New API for logging and setting log level +----------------------------------------- -* spidriver: Add support for the Excamera Labs SPIDriver +New API ``flashrom_set_log_callback_v2`` is added which allows the users to provide +a pointer to user data, the latter is managed by the caller of the API. New API also +introduces a new signature for log callback function, ``flashrom_log_callback_v2``. + +New API supports setting the log level for messages from libflashrom, ``flashrom_set_log_level``. +The log callback is invoked only for the messages with log level less or equal than the one +requested (i.e. only for the messages at least as urgent as the level requested). + +Old API ``flashrom_set_log_callback`` without user data continues to work as before. + +New API for probing flashchips +------------------------------ + +New API for probing is added ``flashrom_flash_probe_v2`` which can (if requested) +go through all known flashchips and find all matches. v2 returns the number of matches +found (or 0 if none found) and the list of names of all matched entries. + +``flashrom_flash_probe_v2`` continues to support an optional parameter ``chip_name`` +if the caller want to probe for only one specific chip with given name. + +Old API ``flashrom_flash_probe`` which stops probing if more than one chip entry matches +continues to work as before. + +New API to get list of supported programmers +-------------------------------------------- + +New API ``flashrom_supported_programmers`` returns the list of all programmers that are +supported on a current run environment. -- To view, visit https://review.coreboot.org/c/flashrom/+/87644?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: I98524353b8bedff3364568636ed42a4bd6ce8b3d Gerrit-Change-Number: 87644 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>

1 0