Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Felix Held, Hannah Williams, Jamie Ryu, Saurabh Mishra.
Subrata Banik has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84104?usp=email )
Change subject: soc/intel/common/block/pmc: Add GPE1 functions
......................................................................
Patch Set 18:
(1 comment)
File src/soc/intel/common/block/include/intelblocks/pmclib.h:
https://review.coreboot.org/c/coreboot/+/84104/comment/b8b143ed_1ecd4165?us… :
PS11, Line 241: gpe0_mask
> @subratabanik@google.com I do not see use case for pmc_enable_std_gpe and pmc_disable_std_gpe. Where is this going be used? I think we just need to be able to disable all GPE0 and GPE1 bits and also clear all of them from Coreboot boot sequence.
> Can we just have pmc_disable_all_gpe and remove pmc_enable_std_gpe and pmc_disable_std_gpe?
isn't `pmc_enable_std_gpe` also part of the pmc_disable_all_gpe? (ideally that should be the flow). I agree with you that we are not calling pmc_enable_std_gpe/pmc_disable_std_gpe today in BIOS flow. But those are useful APIs to be used while debugging the issue. As per my suggestion above, I have extended the GPE0 PMC_B0/Hot_Plug/PCI_exp status to GPE1 events.
--
To view, visit https://review.coreboot.org/c/coreboot/+/84104?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7ac1fbe6d45cbe0c86c3f72911900d92a186168d
Gerrit-Change-Number: 84104
Gerrit-PatchSet: 18
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 12 Sep 2024 06:24:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hannah Williams <hannah.williams(a)intel.com>
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Felix Held, Marshall Dawson, Nikolai Vyssotski.
Hello Felix Held, Marshall Dawson, Nikolai Vyssotski, Zheng Bao, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/75682?usp=email
to look at the new patch set (#8).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: amdfwtool: Add PSP backup directory for A/B recovery
......................................................................
amdfwtool: Add PSP backup directory for A/B recovery
Change-Id: Ia5a745c86595554c83cd13a35c312c17987b716b
Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com>
---
M util/amdfwtool/amdfwtool.c
M util/amdfwtool/amdfwtool.h
2 files changed, 24 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/75682/8
--
To view, visit https://review.coreboot.org/c/coreboot/+/75682?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ia5a745c86595554c83cd13a35c312c17987b716b
Gerrit-Change-Number: 75682
Gerrit-PatchSet: 8
Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Zheng Bao
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Attention is currently required from: Hung-Te Lin, Jarried Lin, Zhanzhan Ge.
Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/83928?usp=email )
Change subject: soc/mediatek/mt8196: Fix timer reset in BL31
......................................................................
Patch Set 23:
(1 comment)
File src/soc/mediatek/mt8196/timer_prepare.c:
https://review.coreboot.org/c/coreboot/+/83928/comment/b3f5fcd7_ef3ff119?us… :
PS23, Line 18: write32
> Dear yuping: […]
Yes, there are still many `setbits32` calls in coreboot, because `SET32_BITFIELDS` is the new API. See CB:35463 for the benefits of the new API. I admit I didn't require using the new API when reviewing all the coreboot patches. However, since hungte@ pointed that out in this patch in another comment, I think we should follow his suggestion.
About how to use the new API, see src/soc/mediatek/mt8188/msdc.c as an example. It will be just a few lines of changes in this file. If you are still not sure how to write the code, please let me know.
--
To view, visit https://review.coreboot.org/c/coreboot/+/83928?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I520986b81ca153ec3ce56558a80619448cfc0c59
Gerrit-Change-Number: 83928
Gerrit-PatchSet: 23
Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com>
Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com>
Gerrit-Reviewer: Zhanzhan Ge <zhanzhan.ge(a)mediatek.corp-partner.google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Zhanzhan Ge <zhanzhan.ge(a)mediatek.corp-partner.google.com>
Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com>
Gerrit-Comment-Date: Thu, 12 Sep 2024 05:58:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Zhanzhan Ge <zhanzhan.ge(a)mediatek.corp-partner.google.com>
Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>
Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Felix Held, Jamie Ryu, Subrata Banik.
Hannah Williams has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/84104?usp=email )
Change subject: soc/intel/common/block/pmc: Add GPE1 functions
......................................................................
Patch Set 17:
(1 comment)
File src/soc/intel/common/block/include/intelblocks/pmclib.h:
https://review.coreboot.org/c/coreboot/+/84104/comment/5b80adcf_a8447981?us… :
PS11, Line 241: gpe0_mask
> > Subrata, […]
@subratabanik@google.com I do not see use case for pmc_enable_std_gpe and pmc_disable_std_gpe. Where is this going be used? I think we just need to be able to disable all GPE0 and GPE1 bits and also clear all of them from Coreboot boot sequence.
Can we just have pmc_disable_all_gpe and remove pmc_enable_std_gpe and pmc_disable_std_gpe?
--
To view, visit https://review.coreboot.org/c/coreboot/+/84104?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7ac1fbe6d45cbe0c86c3f72911900d92a186168d
Gerrit-Change-Number: 84104
Gerrit-PatchSet: 17
Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com>
Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 12 Sep 2024 05:10:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com>
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>