Attention is currently required from: Anastasia Klimchuk.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81266?usp=email )
Change subject: cli_common: Add link to the documentation how to mark chip tested
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/81266?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: I36105725058f2fecb82408c369b70b3324502ece
Gerrit-Change-Number: 81266
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 19 Mar 2024 22:47:59 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: 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 2:
(2 comments)
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/81357/comment/7402d0f1_974aeb6e :
PS1, Line 1848: case CHIPSET_METEOR_LAKE:
> this might increase the maintenance burden going forward ? for example: we need to add PTL, WCL etc. […]
It was your previous CL that put MTL in that position (https://review.coreboot.org/c/flashrom/+/62783) Could you share if (1) We could move it to the very end (2) Will there be any other chipset name which will be moved to make the order makes sense?
https://review.coreboot.org/c/flashrom/+/81357/comment/358a4f26_5b8731d3 :
PS1, Line 2236: ich9_handle_frap
> isn't `ich9_handle_frap` function takes only takes 3 arguments ?
In line#1889, I changed it to
```
static enum ich_access_protection ich9_handle_frap(struct fd_region *fd_regions,
uint32_t region_read_access,
uint32_t region_write_access, unsigned int i)
```
Since if we use `BIOS_BM`, we will derive two seperated bitmask, and I don't think we need to merge them.
Maybe `ich9_handle_frap` should be renamed to something like `ich9_handle_region_access` ?
--
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: 2
Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Comment-Date: Tue, 19 Mar 2024 22:38:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Hsuan-ting Chen.
Hello Hsuan Ting Chen, Subrata Banik, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/81357?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: ichspi.c: Add support for region 9 and beyond in Meteor Lake
......................................................................
ichspi.c: Add support for region 9 and beyond in Meteor Lake
Since Meteor Lake, configuring region access for FREG9 and higher is
necessary. This configuration is determined using BIOS_BM registers:
BIOS_BM_RAP (Offset 0x118): BIOS Master Read Access Permissions.
Each bit [15:0] corresponds to a region [15:0].
A set bit grants BIOS master read access.
BIOS_BM_WAP (Offset 0x11c): BIOS Master Write Access Permissions.
Each bit [15:0] corresponds to a region [15:0].
A set bit grants BIOS master write/erase access.
BUG=b:319773700, b:304439294
BUG=b:319336080
TEST=On MTL, use flashrom -VV to see correct FREG9 access
TEST=On ADL, use flashrom -VV to see not break anything
Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9
Signed-off-by: roccochen(a)chromium.com <roccochen(a)chromium.org>
---
M ichspi.c
1 file changed, 67 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/81357/2
--
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: 2
Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org>
Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen.
Subrata Banik 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 1:
(2 comments)
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/81357/comment/5a064777_d8989c4c :
PS1, Line 1848: case CHIPSET_METEOR_LAKE:
this might increase the maintenance burden going forward ? for example: we need to add PTL, WCL etc. etc. platform going forward.
can we add `CHIPSET_METEOR_LAKE` as last entry for now inside `ich_chipset` and then update this logic something like this:
```
if (ich_generation >= CHIPSET_METEOR_LAKE) {
/*
* Starting from Meteor Lake, we need to fetch the region
* read/write access permissions from the BIOS_BM registers
* because we need to support FREG9 or above.
*/
*region_read_access = mmio_readw(ich_spibar + ICH_REG_BIOS_BM_RAP);
*region_write_access = mmio_readw(ich_spibar + ICH_REG_BIOS_BM_WAP);
} else {
/*
* FRAP - Flash Regions Access Permissions Register
* Bit Descriptions:
* 31:24 BIOS Master Write Access Grant (BMWAG)
* 23:16 BIOS Master Read Access Grant (BMRAG)
* 15:8 BIOS Region Write Access (BRWA)
* 7:0 BIOS Region Read Access (BRRA)
*/
tmp = mmio_readl(ich_spibar + ICH9_REG_FRAP);
msg_pdbg("0x50: 0x%08"PRIx32" (FRAP)\n", tmp);
msg_pdbg("BMWAG 0x%02"PRIx32", ", ICH_BMWAG(tmp));
msg_pdbg("BMRAG 0x%02"PRIx32", ", ICH_BMRAG(tmp));
msg_pdbg("BRWA 0x%02"PRIx32", ", ICH_BRWA(tmp));
msg_pdbg("BRRA 0x%02"PRIx32"\n", ICH_BRRA(tmp));
*region_read_access = ICH_BRRA(tmp);
*region_write_access = ICH_BRWA(tmp);
}
```
https://review.coreboot.org/c/flashrom/+/81357/comment/1cb978df_fa7bff41 :
PS1, Line 2236: ich9_handle_frap
isn't `ich9_handle_frap` function takes only takes 3 arguments ?
--
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: 1
Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com>
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-Attention: Hsuan Ting Chen <roccochen(a)chromium.org>
Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Comment-Date: Tue, 19 Mar 2024 09:25:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel.
Kevin Yu has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81327?usp=email )
Change subject: Add support for Idaville HCC platform and iRC regions
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Hi Pual
Can you help review this code change?
Thanks
Kevin Yu
--
To view, visit https://review.coreboot.org/c/flashrom/+/81327?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: I39e7fa56b1b1de429f413ce32e69c8d8c769b6a9
Gerrit-Change-Number: 81327
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Yu <ykevin(a)celestica.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Tue, 19 Mar 2024 07:08:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel.
Kevin Yu has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81327?usp=email )
Change subject: Add support for Idaville HCC platform and iRC regions
......................................................................
Patch Set 1:
This change is ready for review.
--
To view, visit https://review.coreboot.org/c/flashrom/+/81327?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: I39e7fa56b1b1de429f413ce32e69c8d8c769b6a9
Gerrit-Change-Number: 81327
Gerrit-PatchSet: 1
Gerrit-Owner: Kevin Yu <ykevin(a)celestica.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Mon, 18 Mar 2024 06:02:18 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
DZ has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/81096?usp=email )
Change subject: flashchips: Add Macronix Flash EPN support
......................................................................
Abandoned
need modify the source code
--
To view, visit https://review.coreboot.org/c/flashrom/+/81096?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: Ia07032c1a3168bd595cea6b24e4875843e20190c
Gerrit-Change-Number: 81096
Gerrit-PatchSet: 1
Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: abandon