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
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
Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81664?usp=email )
Change subject: [WIP] tests: Add tests for erasing chip with protected region
......................................................................
Patch Set 1:
(1 comment)
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/81664/comment/869bfab8_44cff11b :
PS1, Line 130: TEST_ERASE_INJECTOR
> I've prototyped this in CB:82251 but can't tell which (if any) of the new test cases should be faili […]
Correction: "without CB:81385".
--
To view, visit https://review.coreboot.org/c/flashrom/+/81664?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: I1e7cfd027d37437c917c054f30f3be59b76b0069
Gerrit-Change-Number: 81664
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Fri, 10 May 2024 07:12:24 +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>
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81664?usp=email )
Change subject: [WIP] tests: Add tests for erasing chip with protected region
......................................................................
Patch Set 1:
(1 comment)
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/81664/comment/bc040c5a_db9123c7 :
PS1, Line 130: TEST_ERASE_INJECTOR
> One thing I haven't figured yet how to do in this test: […]
I've prototyped this in CB:82251 but can't tell which (if any) of the new test cases should be failing without CB:82251. I want to create tests that show the current failure, then rebase the fix onto the tests and get them all to pass.
--
To view, visit https://review.coreboot.org/c/flashrom/+/81664?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: I1e7cfd027d37437c917c054f30f3be59b76b0069
Gerrit-Change-Number: 81664
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Fri, 10 May 2024 07:11:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Subrata Banik.
Anastasia Klimchuk 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:
(4 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/81357/comment/e7205458_afc84bff :
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!
Patchset:
PS12:
Subrata, just wanted to check, do you have interest to review the latest version of the patch? I know you reviewed, but maybe you want to have the last look with all the changes made.
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/81357/comment/af31708c_ee193798 :
PS10, Line 1853: mmio_readw
> You could see the document of Intel Core Ultra: […]
Thanks as usual for giving all details! appreciate it.
I was staring on the code for some time, but it looks like it should work.
I was mostly looking at conversion to smaller size
*region_read_access = (uint16_t)ICH_BRRA(tmp);
*region_write_access = (uint16_t)ICH_BRWA(tmp);
while previously `ICH_BRRA(tmp)` and `ICH_BRWA(tmp)` were used as is, without conversion to smaller size. But, as you pointed in docs, only 2 bytes are needed...
https://review.coreboot.org/c/flashrom/+/81357/comment/998aaaf2_094d074e :
PS10, Line 1876: inline
> Done […]
I am thinking from the other side: are there any reasons we should explicitly say it needs to be inline, and if there are no reasons, better leave this to compiler.
With just one callsite, I think the function will be inlined anyway.
--
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: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Comment-Date: Fri, 10 May 2024 06:04:03 +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
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82181?usp=email )
Change subject: doc: Add user doc with links to ChromeOS documents
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> Sounds good!
So this is now submitted, you add for yourself an entry in MAINTAINERS file for this file `doc/user_docs/chromebooks.rst` . Thanks!
--
To view, visit https://review.coreboot.org/c/flashrom/+/82181?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: If7b06c077b34f73bc6c33f617332dfc32b982c12
Gerrit-Change-Number: 82181
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 10 May 2024 03:35:34 +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
Attention is currently required from: Nicholas Chin.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82162?usp=email )
Change subject: flashrom_udev.rules: Add rule for CH347
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/82162?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: Ia83fa08f6d7c2f449b1a5c0c387c6d4368b99e3a
Gerrit-Change-Number: 82162
Gerrit-PatchSet: 1
Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Fri, 10 May 2024 03:30:13 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82238?usp=email )
Change subject: MAINTAINERS: add Peter Marheine for build system
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/82238/comment/de200ad4_6e6553d0 :
PS1, Line 7: pmarheine
> Peter Marheine […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/82238?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: Ibae0c006b293dad85a9571ec8e7081a6396bc7ce
Gerrit-Change-Number: 82238
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Thu, 09 May 2024 23:54:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Peter Marheine.
Hello Anastasia Klimchuk, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/82238?usp=email
to look at the new patch set (#2).
Change subject: MAINTAINERS: add Peter Marheine for build system
......................................................................
MAINTAINERS: add Peter Marheine for build system
Change-Id: Ibae0c006b293dad85a9571ec8e7081a6396bc7ce
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
---
M MAINTAINERS
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/82238/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/82238?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: Ibae0c006b293dad85a9571ec8e7081a6396bc7ce
Gerrit-Change-Number: 82238
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newpatchset