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
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
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