Attention is currently required from: Bora Guvendik, Paul Menzel, Subrata Banik, Angel Pons.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51444 )
Change subject: commonlib: Add new TS for IA pre-cpu reset entities
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/51444/comment/e4802ae8_bb6b1440
PS4, Line 7: IA
What is IA?
--
To view, visit https://review.coreboot.org/c/coreboot/+/51444
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0ba5939aa7a6356eff121a931198608401f1a3e6
Gerrit-Change-Number: 51444
Gerrit-PatchSet: 4
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 11 Oct 2021 15:29:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Paul Menzel, Tim Wawrzynczak, Arthur Heymans, Michael Niewöhner.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58221 )
Change subject: soc/intel/cannonlake: Enable Energy/Performance Bias control
......................................................................
Patch Set 1:
(2 comments)
Patchset:
PS1:
> Sorry, I was wrong. Even FSP sets it by default (EnergyEfficientPState is 1 in bsf). […]
Um, CB:34458 already took care of it.
File src/soc/intel/cannonlake/cpu.c:
https://review.coreboot.org/c/coreboot/+/58221/comment/4ea522b5_38d9fd93
PS1, Line 54: 1 << 18
> tbh I'd prefer if these bits were defined where MSR_POWER_CTL is, but since the others aren't too, l […]
I don't think it's worth defining a macro for something used only once, as a comment can already provide sufficient information without the hassle of having to find and open the file with the macro definition.
However, this isn't the case for platforms using soc/intel/common MSR definitions; if the bit definitions are the same for all supported CPUs, then they could be defined once in there. I still believe it's not worth the hassle in this case.
I don't think the same applies to macros for MSR numbers and, by extension, register offsets. Finding out which occurrences of a magic number correspond to accesses to the register one is looking for isn't trivial, having a defined name makes this significantly easier. And since bit definitions are associated to a register, one can check the places where a register is being accessed to find out if a given bit is written to, even if the bit doesn't have a defined name.
TL;DR: I like the current approach.
--
To view, visit https://review.coreboot.org/c/coreboot/+/58221
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibd1db77b5b63cb6e2b0ad9d2f79caa2f3b576ead
Gerrit-Change-Number: 58221
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Mon, 11 Oct 2021 15:25:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Julius Werner, Karthik Ramasubramanian.
Hello build bot (Jenkins), Julius Werner, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/58205
to look at the new patch set (#5).
Change subject: cpu/x86/cpu_info.S: Remove ebx save/restore
......................................................................
cpu/x86/cpu_info.S: Remove ebx save/restore
The push/pop of %ebx was only added because smm_stub saves the canary
value in it. Now that we no longer use cpu_info in smm, we no longer
need to save the register.
BUG=b:179699789
TEST=Boot guybrush to the OS
Signed-off-by: Raul E Rangel <rrangel(a)chromium.org>
Change-Id: I554dbe016db8b1c61246c8ffc7fa252b2542ba92
---
M src/cpu/x86/cpu_info.S.inc
1 file changed, 1 insertion(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/58205/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/58205
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I554dbe016db8b1c61246c8ffc7fa252b2542ba92
Gerrit-Change-Number: 58205
Gerrit-PatchSet: 5
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Paul Menzel, Julius Werner, Karthik Ramasubramanian.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58204 )
Change subject: cpu/x86/smm/smm_stub: Remove cpu_info
......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/58204/comment/3813a7d5_e0aa0255
PS3, Line 10: setup
> set up
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/58204
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id64f32cc63082880a92dab6deb473431b2238cd0
Gerrit-Change-Number: 58204
Gerrit-PatchSet: 5
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Mon, 11 Oct 2021 15:22:27 +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: Paul Menzel, Julius Werner, Karthik Ramasubramanian.
Hello build bot (Jenkins), Paul Menzel, Julius Werner, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/58204
to look at the new patch set (#5).
Change subject: cpu/x86/smm/smm_stub: Remove cpu_info
......................................................................
cpu/x86/smm/smm_stub: Remove cpu_info
Now that cpu_info() is no longer used by COOP_MULTITASKING, we no
longer need to set up cpu_info in SMM. When using CPU_INFO_V2, if
something does manage to call cpu_info() while executing in SMM mode,
the %gs segment is disabled, so it will generate an exception.
BUG=b:179699789
TEST=Boot guybrush to OS with threads enabled
Signed-off-by: Raul E Rangel <rrangel(a)chromium.org>
Change-Id: Id64f32cc63082880a92dab6deb473431b2238cd0
---
M src/cpu/x86/smm/smm_stub.S
1 file changed, 1 insertion(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/58204/5
--
To view, visit https://review.coreboot.org/c/coreboot/+/58204
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id64f32cc63082880a92dab6deb473431b2238cd0
Gerrit-Change-Number: 58204
Gerrit-PatchSet: 5
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Julius Werner <jwerner(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-MessageType: newpatchset
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58070 )
Change subject: mb/google/guybrush/bootblock: add comment to PM_ACPI_CONF write
......................................................................
mb/google/guybrush/bootblock: add comment to PM_ACPI_CONF write
Document what setting the PM_ACPI_S5_LPC_PIN_MODE and
PM_ACPI_S5_LPC_PIN_MODE_SEL bits causes. The corresponding code will
eventually be factored out and moved to the Cezanne SoC code.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: I10e3eee5cfc1c5ba2c88b8b7e83e96e481f787e1
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58070
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Raul Rangel <rrangel(a)chromium.org>
---
M src/mainboard/google/guybrush/bootblock.c
1 file changed, 1 insertion(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Raul Rangel: Looks good to me, approved
diff --git a/src/mainboard/google/guybrush/bootblock.c b/src/mainboard/google/guybrush/bootblock.c
index 3f7d5a1..9c9fce7 100644
--- a/src/mainboard/google/guybrush/bootblock.c
+++ b/src/mainboard/google/guybrush/bootblock.c
@@ -61,6 +61,7 @@
dword |= PM_ESPI_CS_USE_DATA2;
pm_write32(PM_SPI_PAD_PU_PD, dword);
+ /* Switch the pads that can be used as either LPC or secondary eSPI to 1.8V mode */
dword = pm_read32(PM_ACPI_CONF);
dword |= PM_ACPI_S5_LPC_PIN_MODE | PM_ACPI_S5_LPC_PIN_MODE_SEL;
pm_write32(PM_ACPI_CONF, dword);
--
To view, visit https://review.coreboot.org/c/coreboot/+/58070
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I10e3eee5cfc1c5ba2c88b8b7e83e96e481f787e1
Gerrit-Change-Number: 58070
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58069 )
Change subject: mb/google/guybrush: simplify LPC_MISC_CONTROL_BITS update
......................................................................
mb/google/guybrush: simplify LPC_MISC_CONTROL_BITS update
Since the LPC_LDRQ0_PD_EN gets set right after it got cleared, we can
remove the clearing of that bit. This is split off from the previous
patch to be able to use timeless build to verify that the previous patch
didn't change any behavior.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: Ieb300e7c7ce7e74c32ebdade0360ee4bd499b11a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58069
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Raul Rangel <rrangel(a)chromium.org>
---
M src/mainboard/google/guybrush/bootblock.c
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
build bot (Jenkins): Verified
Raul Rangel: Looks good to me, approved
diff --git a/src/mainboard/google/guybrush/bootblock.c b/src/mainboard/google/guybrush/bootblock.c
index 5857d85..3f7d5a1 100644
--- a/src/mainboard/google/guybrush/bootblock.c
+++ b/src/mainboard/google/guybrush/bootblock.c
@@ -35,7 +35,7 @@
const struct soc_amd_gpio *base_gpios, *override_gpios;
dword = pci_read_config32(SOC_LPC_DEV, LPC_MISC_CONTROL_BITS);
- dword &= ~(LPC_LDRQ0_PU_EN | LPC_LDRQ0_PD_EN | LPC_LDRQ1_EN | LPC_LDRQ0_EN);
+ dword &= ~(LPC_LDRQ0_PU_EN | LPC_LDRQ1_EN | LPC_LDRQ0_EN);
dword |= LPC_LDRQ0_PD_EN;
pci_write_config32(SOC_LPC_DEV, LPC_MISC_CONTROL_BITS, dword);
--
To view, visit https://review.coreboot.org/c/coreboot/+/58069
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ieb300e7c7ce7e74c32ebdade0360ee4bd499b11a
Gerrit-Change-Number: 58069
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.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-MessageType: merged