Attention is currently required from: Arthur Heymans, Felix Held, Maximilian Brune, Nico Huber.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64184?usp=email )
Change subject: haswell NRI: Initialise MPLL
......................................................................
Patch Set 5:
(1 comment)
File src/northbridge/intel/haswell/native_raminit/init_mpll.c:
https://review.coreboot.org/c/coreboot/+/64184/comment/23aeb34c_08abb675 :
PS5, Line 155: if (!ctrl->qclkps) {
> i wonder what this if statement is for, since qclkps will only be set after the while loop. […]
Ah, I remember: this is for the fast boot path. In CB:81897 this value is restored from MRC cache before calling this function. I think this could at least use a comment, though.
--
To view, visit https://review.coreboot.org/c/coreboot/+/64184?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I978c352de68f6d8cecc76f4ae3c12daaf4be9ed6
Gerrit-Change-Number: 64184
Gerrit-PatchSet: 5
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 18 Apr 2024 20:02:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Arthur Heymans, Maximilian Brune, Nico Huber.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64184?usp=email )
The change is no longer submittable: All-Comments-Resolved is unsatisfied now.
Change subject: haswell NRI: Initialise MPLL
......................................................................
Patch Set 5:
(1 comment)
File src/northbridge/intel/haswell/native_raminit/init_mpll.c:
https://review.coreboot.org/c/coreboot/+/64184/comment/5cb6abbb_953eabf3 :
PS5, Line 155: if (!ctrl->qclkps) {
i wonder what this if statement is for, since qclkps will only be set after the while loop. only had a brief look and didn't look at the patches after this one, so maybe i'm missing something here, but it at least seems to be unexpected
--
To view, visit https://review.coreboot.org/c/coreboot/+/64184?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I978c352de68f6d8cecc76f4ae3c12daaf4be9ed6
Gerrit-Change-Number: 64184
Gerrit-PatchSet: 5
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-CC: Martin L Roth <gaumless(a)gmail.com>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 18 Apr 2024 19:58:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Andrey Petrov, Paul Menzel, Ronak Kanabar, Subrata Banik.
Appukuttan V K has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81663?usp=email )
Change subject: drivers/intel/fsp2_0: Add dedicated caller function for ap procedure calls
......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81663/comment/419b7a20_800036ae :
PS1, Line 7: drivers/intel/fsp2_0:Dedicated caller function for ap procedure calls
> Please add a space after the colon, and make the summary a statement by adding a verb in imperative […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/81663?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I64e65b2941207375d5e27c84aa26061e7e72a7f6
Gerrit-Change-Number: 81663
Gerrit-PatchSet: 5
Gerrit-Owner: Appukuttan V K <appukuttan.vk(a)intel.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Comment-Date: Thu, 18 Apr 2024 19:37:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Andrey Petrov, Chen, Gang C, Jincheng Li, Jérémy Compostella, Martin L Roth, Ronak Kanabar, Shuo Liu, Subrata Banik.
Appukuttan V K has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80323?usp=email )
Change subject: drivers/intel/fsp2_0: Default to 64-bits for FSP 2.4
......................................................................
Patch Set 29:
(1 comment)
File src/drivers/intel/fsp2_0/Kconfig:
https://review.coreboot.org/c/coreboot/+/80323/comment/5763aad6_f326ebb4 :
PS27, Line 62: Starting with FSP specification 2.4 they can be 64-bits.
> help to rewrite this statement.
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/80323?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: If0397f5cc8d0f4f1872bd37a001fe42e0c37ec98
Gerrit-Change-Number: 80323
Gerrit-PatchSet: 29
Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com>
Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Appukuttan V K <appukuttan.vk(a)intel.com>
Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com>
Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>
Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com>
Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com>
Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com>
Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Comment-Date: Thu, 18 Apr 2024 19:36:59 +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: Angel Pons, Varshit Pandya.
Máté Kukri has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81529?usp=email )
Change subject: mb/dell/optiplex_9020: Implement late HWM initialization
......................................................................
Patch Set 6:
(4 comments)
File src/mainboard/dell/optiplex_9020/mainboard.c:
https://review.coreboot.org/c/coreboot/+/81529/comment/cfeeb8be_29bd981f :
PS5, Line 290: die("Unknown GPIO chassis type\n");
> I am fine with turning these into warning.
Turned this into a warning with fans at full speed
https://review.coreboot.org/c/coreboot/+/81529/comment/e39512cb_05694347 :
PS5, Line 368: die("Unknown chassis type\n");
> Can the fan still be set to run at full speed? Should work to prevent overheating.
See above.
https://review.coreboot.org/c/coreboot/+/81529/comment/1a908c0d_8605ce11 :
PS5, Line 371: if (CONFIG_MAX_CPUS > 2) {
> Mine is 4 core 4 thread, and that MSR returns 7 on mine, so I wonder what value would it be on the 2 […]
I am just going to make it always apply these values, I think that should match stock fw based on the MSR values I've read.
File src/mainboard/dell/optiplex_9020/sch5555_ec.h:
https://review.coreboot.org/c/coreboot/+/81529/comment/ec0bac30_9f7e9dc0 :
PS5, Line 3: #pragma once
> Indeed, I missed that as this file copy pasted from my user-space utility.
Fixed.
--
To view, visit https://review.coreboot.org/c/coreboot/+/81529?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ibdccd3fc7364e03e84ca606592928410624eed43
Gerrit-Change-Number: 81529
Gerrit-PatchSet: 6
Gerrit-Owner: Máté Kukri <kukri.mate(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 18 Apr 2024 19:33:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Máté Kukri <kukri.mate(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Varshit Pandya.
Hello Angel Pons, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/81529?usp=email
to look at the new patch set (#6).
The following approvals got outdated and were removed:
Code-Review+1 by Angel Pons, Verified+1 by build bot (Jenkins)
Change subject: mb/dell/optiplex_9020: Implement late HWM initialization
......................................................................
mb/dell/optiplex_9020: Implement late HWM initialization
There are 4 different chassis types specified by vendor firmware, each
with a slightly different HWM configuration.
The chassis type to use is determined at runtime by reading a set of
4 PCH GPIOs: 70, 38, 17, and 0.
Additionally vendor firmware also provides an option to run the fans at
full speed. This is substituted with a coreboot nvram option in this
implementation.
This was tested to make fan control work on my OptiPlex 7020 SFF.
NOTE: This is superficially similar to the OptiPlex 9010's SCH5545
however the OptiPlex 9020's SCH5555 does not use externally
programmed EC firmware.
Change-Id: Ibdccd3fc7364e03e84ca606592928410624eed43
Signed-off-by: Mate Kukri <kukri.mate(a)gmail.com>
---
M src/mainboard/dell/optiplex_9020/Makefile.mk
M src/mainboard/dell/optiplex_9020/bootblock.c
M src/mainboard/dell/optiplex_9020/cmos.default
M src/mainboard/dell/optiplex_9020/cmos.layout
M src/mainboard/dell/optiplex_9020/mainboard.c
A src/mainboard/dell/optiplex_9020/sch5555_ec.c
A src/mainboard/dell/optiplex_9020/sch5555_ec.h
7 files changed, 458 insertions(+), 21 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/81529/6
--
To view, visit https://review.coreboot.org/c/coreboot/+/81529?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ibdccd3fc7364e03e84ca606592928410624eed43
Gerrit-Change-Number: 81529
Gerrit-PatchSet: 6
Gerrit-Owner: Máté Kukri <kukri.mate(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset