- flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[main]: doc: Fix index files in Supported HW section
by Sydney (Code Review) 10 May '24

10 May '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine, Thomas Heijligen. Sydney has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82196?usp=email ) Change subject: doc: Fix index files in Supported HW section ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/82196?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia15e9766cce6f19be1e69fbb1236a327ae3d57b3 Gerrit-Change-Number: 82196 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sydney <git(a)funkeleinhorn.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 10 May 2024 11:27:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: doc: Add doc for supported flash chips
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Alexander Goncharov, Peter Marheine, Thomas Heijligen, Victor Lim. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82197?usp=email ) Change subject: doc: Add doc for supported flash chips ...................................................................... Patch Set 2: (1 comment) File doc/supported_hw/supported_flashchips.rst: https://review.coreboot.org/c/flashrom/+/82197/comment/e204da35_4be95fc6 : PS1, Line 9: If you want to check whether a flash chip is supported in the given release, you can rebase your local > `flashrom -L` also prints out all supported chips, which is probably more accessible to users who al […] I completely forgot about it, but it's super useful. Not only in this patch, but in all the other patches in the chain. And I have discovered there is one more branch of code which prints everything! print_wiki definitely can retire with a peace of mind. -- To view, visit https://review.coreboot.org/c/flashrom/+/82197?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I05fb60a4caf2cfb30586fa482687b10638996395 Gerrit-Change-Number: 82197 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Victor Lim <victorswlim(a)yahoo.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Victor Lim <victorswlim(a)yahoo.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 10 May 2024 10:44:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: doc: Fix index files in Supported HW section
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Peter Marheine, Sydney, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82196?usp=email ) Change subject: doc: Fix index files in Supported HW section ...................................................................... Patch Set 2: (2 comments) Patchset: PS1: > Thanks for requesting my review on this. […] Thanks! I uploaded new version after Peter's comment, to achieve the same effect but in more correct way. There are three more patches in the chain, if you are interested, you are very welcome to have a look and review! Gerrit works in a way that you can add comments and vote on any patch, even if you are not explicitly added to it (Gerrit will add you). File doc/supported_hw/index.rst: https://review.coreboot.org/c/flashrom/+/82196/comment/cea7ab84_790e6bf8 : PS1, Line 6: :maxdepth: 3 > Couldn't we just change maxdepth to have fewer nested levels? It seems error-prone to set this `hidd […] Oh somehow I was reading documentation for toctree directive and misunderstood it. Yes you idea is great, and it works! I updated the rest of the chain as well. -- To view, visit https://review.coreboot.org/c/flashrom/+/82196?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia15e9766cce6f19be1e69fbb1236a327ae3d57b3 Gerrit-Change-Number: 82196 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sydney <git(a)funkeleinhorn.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Sydney <git(a)funkeleinhorn.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 10 May 2024 10:42:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sydney <git(a)funkeleinhorn.com> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashrom: Change chip unlock error to warning
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82175?usp=email ) Change subject: flashrom: Change chip unlock error to warning ...................................................................... flashrom: Change chip unlock error to warning Failing to disable WP before write/erase doesn't necessarily indicate an error and flashrom doesn't treat it as such. Print a warning instead on an error. BUG=b:336220545 BRANCH=none TEST=build Change-Id: I14c3b55e387443909ca1efab2fc1901f87dd66d6 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82175 Reviewed-by: Hsuan-ting Chen <roccochen(a)google.com> Reviewed-by: Brian Norris <briannorris(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashrom.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Nikolai Artemiev: Verified Brian Norris: Looks good to me, but someone else must approve Hsuan-ting Chen: Looks good to me, but someone else must approve build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index 6b15ee5..a365303 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2092,7 +2092,7 @@ warn_out: if (ret) - msg_cerr("Failed to unlock flash status reg with wp support.\n"); + msg_cwarn("Failed to unlock flash status reg with wp support.\n"); return ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/82175?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I14c3b55e387443909ca1efab2fc1901f87dd66d6 Gerrit-Change-Number: 82175 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[main]: flashrom: Don't throw around "delay 1 second" so lightly
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/80807?usp=email ) Change subject: flashrom: Don't throw around "delay 1 second" so lightly ...................................................................... flashrom: Don't throw around "delay 1 second" so lightly Waiting a full second is a very long time, especially when default_delay() chooses to busy-loop. This code has been around for a decade, with vague references to user reports: commit 8ab49e72af8465d4527de2ec37b22cd44f7a1169 Date: Wed Aug 19 13:55:34 2009 +0000 Disallow erase/write for known bad chips so people won't try without a clear understanding Still, this logic does not belong in the high-level library logic, used by all programmers and all chips. If there is a timing issue, it should either be encoded in the appropriate programmer or flashchip timing. However, we don't really know what chips were in use, as the above commit doesn't have any links to reports. So in a feeble attempt at avoiding breaking users here, we also surmise that... * SPI chips weren't all that common in 2009; * I'm mostly motivated by flashrom performance on Chromebooks, were SPI chips (and linux_mtd / BUS_PROG) are the rule; and * SPI chips have precise timing requirements and an appropriate BUSY status. So we guess that this "calm down" magic delay wouldn't be necessary there. Thus, we allow this magic delay only on non-SPI (and non-BUG_PROG, used by linux_mtd for one) buses as a compromise. Now, this change has some (hopefully [1] tiny) chance of regression, so we have the following considerations: 1. emergency_help_message() already provides documentation on how to contact support, in case we need to handle any user-reported regressions. 2. If there is any regression here, it's only in the --verify code; so we can always provide workarounds for testing this, to determine whether this change may have been at fault. For example, something like: flashrom --write /my/new/image.bin --noverify sleep 1 flashrom --read /tmp/bar.bin cmp /my/new/image.bin /tmp/bar.bin If such problems occur, we can collect system/programmer/chip info to try to encode a more targeted delay into the appropriate chip/programmer implementation, and avoid penalizing the entire project like this. 3. We already have (embedded in erase_write()) erase verification that performs no such delay. So depending on the type of timing error that this delay was attempting to cover, we may have some proof that this delay is no longer necessary (or at least, that whatever systems were needing this delay in the first place are no longer caring about flashrom). 4. We've retained the delay for buses that were likely common in 2009 (per the above "feeble attempt"). NB: I avoid using the BUS_NONSPI macro, because I want to exclude any future buses from this workaround, even in the event that the BUS_NONSPI category grows in the future. [1] Famous last words. BUG=b:325539928 TEST=`flashrom_tester --flashrom_binary=$(which flashrom) \ internal Erase_and_Write Fail_to_verify`, TEST=`vpd -i RW_VPD -s foo=bar; vpd -i RW_VPD -l; \ vpd -i RW_VPD -d foo; vpd -i RW_VPD -l` TEST=`elogtool list; elogtool add 0xa7; elogtool list` on (at least) 2 systems: #1: Kukui/Kakadu rev2 - MTD programmer / kernel 5.10.215-24542-g0515a679eb42 / CrOS ~ 15857.0.0 #2: Zork/Dirinboz rev2 - chip name: vendor="Winbond" name="W25Q128.JW.DTR" / BIOS: Google_Dirinboz.13434.688.0 / kernel 5.4.267-21940-g67f70e251a74 / CrOS ~ 15753.43.0 Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc Signed-off-by: Brian Norris <briannorris(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/80807 Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashrom.c 1 file changed, 25 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved Peter Marheine: Looks good to me, but someone else must approve diff --git a/flashrom.c b/flashrom.c index 630c69d..6b15ee5 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2330,8 +2330,31 @@ if (verify && !all_skipped) { msg_cinfo("Verifying flash... "); - /* Work around chips which need some time to calm down. */ - programmer_delay(flashctx, 1000*1000); + /* + * Work around chips which "need some time to calm down." + * + * Frankly, it's not 100% clear why this delay is here at all, + * except for a terse message from 2009 of "a few reports where + * verify directly after erase had unpleasant side effects like + * corrupting flash or at least getting incorrect verify + * results". Ideally, if there were a few known problematic + * chips or programmers, we could add quirks flags for those + * specific implementations without penalizing all other + * flashrom users. But alas, we don't know which systems + * experienced those issues. + * + * Out of an extreme abundance of caution, we retain this + * delay, but only for a few non-SPI bus types that were the + * likely prevalent targets at the time. This is a complete + * guess, which conveniently avoids wasting time on common + * BUS_SPI and BUS_PROG systems. + * + * Background thread: + * Subject: RFC: removing 1 second verification delay + * https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/SFV3… + */ + if (flashctx->chip->bustype & (BUS_PARALLEL | BUS_LPC | BUS_FWH)) + programmer_delay(flashctx, 1000*1000); if (verify_all) combine_image_by_layout(flashctx, newcontents, oldcontents); -- To view, visit https://review.coreboot.org/c/flashrom/+/80807?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie09651fede3f9f03425244c94a2da8bae00315fc Gerrit-Change-Number: 80807 Gerrit-PatchSet: 7 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[main]: doc: Add doc for supported boards and laptops
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Thomas Heijligen. Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/82271?usp=email ) Change subject: doc: Add doc for supported boards and laptops ...................................................................... doc: Add doc for supported boards and laptops Change-Id: Iaae05ccd138fd8f7760823f867f3c7799018dc2e Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/supported_hw/index.rst A doc/supported_hw/supported_boards.rst 2 files changed, 20 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/71/82271/1 diff --git a/doc/supported_hw/index.rst b/doc/supported_hw/index.rst index ba77132..eebf394 100644 --- a/doc/supported_hw/index.rst +++ b/doc/supported_hw/index.rst @@ -8,3 +8,4 @@ supported_flashchips supported_prog/index supported_chipsets + supported_boards diff --git a/doc/supported_hw/supported_boards.rst b/doc/supported_hw/supported_boards.rst new file mode 100644 index 0000000..91b2899 --- /dev/null +++ b/doc/supported_hw/supported_boards.rst @@ -0,0 +1,19 @@ +======================== +Supported boards/laptops +======================== + +To see the list of all supported boards or laptops, check either ``struct board_info boards_known[]`` or ``struct board_info laptops_known[]`` +in the ``known_boards.c`` file in the source tree. + +If you have a flashrom repo cloned locally, you can look at the file in your repo, alternatively inspect the file +`on the web UI of our GitHub mirror <https://github.com/flashrom/flashrom/blob/main/known_boards.c#L29>`_. + +If you can run flashrom locally, the command ``flashrom -L`` prints the list of all supported boards and laptops +(see :doc:`/classic_cli_manpage` for more details on command line options). The output of this command is long, so you might +want to save it to file or grep. + +Each board entry is described by the ``struct board_info`` in ``include/programmer.h`` which you can inspect in the same way, either in the local source tree or +`in the GitHub web UI <https://github.com/flashrom/flashrom/blob/main/include/programmer.h#L207>`_. + +Note the ``enum test_state status`` of the board. ``OK`` means board is tested, ``NT`` means not tested, to see all possible +test states check the ``enum test_state`` definition in ``include/flash.h``. -- To view, visit https://review.coreboot.org/c/flashrom/+/82271?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Iaae05ccd138fd8f7760823f867f3c7799018dc2e Gerrit-Change-Number: 82271 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[main]: doc: Add doc for supported chipsets
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Thomas Heijligen. Hello Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/82198?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: doc: Add doc for supported chipsets ...................................................................... doc: Add doc for supported chipsets Change-Id: I9c9edc7deeeb7a783e2ba2fc6b372edb9c61609e Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/supported_hw/index.rst A doc/supported_hw/supported_chipsets.rst 2 files changed, 22 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/82198/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82198?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9c9edc7deeeb7a783e2ba2fc6b372edb9c61609e Gerrit-Change-Number: 82198 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[main]: doc: Add doc for supported flash chips
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Alexander Goncharov, Anastasia Klimchuk, Thomas Heijligen, Victor Lim. Hello Alexander Goncharov, Peter Marheine, Thomas Heijligen, Victor Lim, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/82197?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: doc: Add doc for supported flash chips ...................................................................... doc: Add doc for supported flash chips Change-Id: I05fb60a4caf2cfb30586fa482687b10638996395 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/supported_hw/index.rst A doc/supported_hw/supported_flashchips.rst 2 files changed, 26 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/82197/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82197?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I05fb60a4caf2cfb30586fa482687b10638996395 Gerrit-Change-Number: 82197 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Victor Lim <victorswlim(a)yahoo.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Victor Lim <victorswlim(a)yahoo.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[main]: doc: Fix index files in Supported HW section
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Anastasia Klimchuk, Sydney, Thomas Heijligen. Hello Peter Marheine, Sydney, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/82196?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by Sydney, Verified+1 by build bot (Jenkins) Change subject: doc: Fix index files in Supported HW section ...................................................................... doc: Fix index files in Supported HW section By default toctree in the index file displays full tree of docs with all the nested levels, and it's too much detail. Besides, left side menu displays the tree anyway, so duplication is not needed. Supported hardware section has the deepest nesting out of all other docs. This patch changes high-level index files to only display flat list of next level subtree. On deeper level, full index is displayed. Change-Id: Ia15e9766cce6f19be1e69fbb1236a327ae3d57b3 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/supported_hw/index.rst M doc/supported_hw/supported_prog/index.rst 2 files changed, 11 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/82196/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82196?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia15e9766cce6f19be1e69fbb1236a327ae3d57b3 Gerrit-Change-Number: 82196 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sydney <git(a)funkeleinhorn.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sydney <git(a)funkeleinhorn.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[main]: ichspi.c: Add support for region 9 and beyond in Meteor Lake
by Hsuan-ting Chen (Code Review) 10 May '24

10 May '24

Attention is currently required from: Anastasia Klimchuk, Hsuan Ting Chen, Subrata Banik. Hsuan-ting Chen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81357?usp=email ) Change subject: ichspi.c: Add support for region 9 and beyond in Meteor Lake ...................................................................... Patch Set 12: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/81357/comment/c6175aab_81cead12 : PS12, Line 26: TEST=On MTL, use flashrom -VV to see correct FREG9 access : TEST=On ADL, use flashrom -VV to see not break anything : TEST=On APL, use flashrom -VV to see not break anything > Would you mind running the test scenarious again on the latest version of code? Thank you! Yes, I checked it on MTL and APL again when I submitted the code. File ichspi.c: https://review.coreboot.org/c/flashrom/+/81357/comment/bb41b5a9_7be97da4 : PS10, Line 1853: mmio_readw > Thanks as usual for giving all details! appreciate it. […] ICH_BRRA and ICH_BRWA are designed to be at most 8 bits, also we have a mask: ``` #define ICH_BRWA(x) ((x >> 8) & 0xff) #define ICH_BRRA(x) ((x >> 0) & 0xff) ``` So they should always fit in 8 bits. (uint8_t) https://review.coreboot.org/c/flashrom/+/81357/comment/10ad523a_d8ae2cdc : PS10, Line 1876: inline > I am thinking from the other side: are there any reasons we should explicitly say it needs to be inl […] Yes, and I believe that this short static would possibly inline it by default. -- To view, visit https://review.coreboot.org/c/flashrom/+/81357?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Gerrit-Change-Number: 81357 Gerrit-PatchSet: 12 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-CC: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 10 May 2024 07:42:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit