Attention is currently required from: Furquan Shaikh, Paul Menzel, Sumeet R Pawnikar, Patrick Rudolph, Karthik Ramasubramanian.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57096 )
Change subject: drivers/intel/dptf: Introduce new thermal control mechanism for pch device
......................................................................
Patch Set 6: Code-Review+1
(1 comment)
File src/drivers/intel/dptf/dptf.c:
https://review.coreboot.org/c/coreboot/+/57096/comment/14f1afb1_07e09574
PS5, Line 211: acpigen_emit_byte(RETURN_OP);
: acpigen_write_package(0);
: acpigen_write_package_end();
> We need this return value to resolve any kind of EVAL failure in DPTF ESIF shell. […]
Can you add a comment then that the reason for returning a value here is a W/A for the ESIF shell?
--
To view, visit https://review.coreboot.org/c/coreboot/+/57096
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I035d2844b9ba6a9532ae006fc1c43e34cb94328a
Gerrit-Change-Number: 57096
Gerrit-PatchSet: 6
Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Fri, 03 Sep 2021 16:16:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56787 )
Change subject: soc/amd/common/block/gpio_banks: factor out gpio_mux_ptr
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56787
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4054656c5cc23ea942e8dd370fbbffca304755d6
Gerrit-Change-Number: 56787
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 03 Sep 2021 16:16:13 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Michał Żygowski, Marshall Dawson, Felix Held, Piotr Król.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56786 )
Change subject: soc/amd/common/include/acpimmio: reduce visibility of GPIO MMIO access
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56786
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I18872dfa40d53ba8b0d7802eec52ede5e2ae617a
Gerrit-Change-Number: 56786
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com>
Gerrit-Comment-Date: Fri, 03 Sep 2021 16:15:02 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Sumeet Pawnikar, Furquan Shaikh, Paul Menzel, Sumeet R Pawnikar, Karthik Ramasubramanian.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57290 )
Change subject: mb/google/brya/variants: fix override values for power limits
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57290/comment/5e224316_5748a69d
PS4, Line 9: There are two different types of 682 SKU available with TDP of 28W and 45W.
: This patch fix override values for power limits for these 682 SKU.
: This patch also sets power limit values dynamically based on machine ID and
: CPU TDP of SKU.
reflow for 72 characters wide please
--
To view, visit https://review.coreboot.org/c/coreboot/+/57290
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I796e56321ae9c8312530a4b8986cd73a2245f5fa
Gerrit-Change-Number: 57290
Gerrit-PatchSet: 4
Gerrit-Owner: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Sumeet Pawnikar <sumeet.r.pawnikar(a)intel.corp-partner.google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sumeet Pawnikar <sumeet.r.pawnikar(a)intel.corp-partner.google.com>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Fri, 03 Sep 2021 16:14:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Furquan Shaikh, Subrata Banik, Ronak Kanabar, Andrey Petrov, Patrick Rudolph.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/57343 )
Change subject: drivers/intel/fsp2_0: Allow `mp_startup_all_cpus()` to run serially
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/57343/comment/bd0453bb_7c371d91
PS1, Line 17: But another MP service API `StartupAllCPUs` doesn't specifies any such
: requirement. Running the `func` simultaneously on APs results into
: coherency issue due to lack of acquire spin lock while accessing common
: data structure in multiprocessor environment.
> > It's hard to figure out if this is called out explicitly anywhere... […]
But right here, in the MpServices2 interface (https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/CpuMpPei/CpuMp2Pei…), it says (in reference to EdkiiPeiStartupAllCPUs, which is the last callback in the MpServices2 PPI):
```
This service executes a caller provided function on all enabled CPUs. CPUs can
run either simultaneously or one at a time in sequence. This service may only
be called from the BSP.
```
If the FSP needs the programming to happen in sequence, isn't it using the wrong MP Services 2 PPI callback? Shouldn't it use `StartupAllAPs` and set `SingleThread` to TRUE ?
--
To view, visit https://review.coreboot.org/c/coreboot/+/57343
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ia95d11408f663212fd40daa9fd9b0881a07f1ce7
Gerrit-Change-Number: 57343
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Fri, 03 Sep 2021 16:13:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held.
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56700 )
Change subject: soc/amd/common/block/gpio_banks/gpio: use gpio_t for GPIO numbers
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/56700
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7cf9cbd2a287dcfe3a47a8a6b164c2b3d8ae95d6
Gerrit-Change-Number: 56700
Gerrit-PatchSet: 5
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Fri, 03 Sep 2021 16:11:42 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment