Attention is currently required from: Wonkyu Kim, Tim Wawrzynczak, Angel Pons, Arthur Heymans, Kyösti Mälkki.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61776 )
Change subject: Revert "cpu/x86/lapic: Unconditionally use CPUID leaf 0xb if available"
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
i'm not sure if reverting back to the is_x2apic_mode check or adding a check if the x2apic feature is available in the cpuid feature bits would be the better option, but we do need to check if x2apic is available
--
To view, visit https://review.coreboot.org/c/coreboot/+/61776
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If1e3c55ce2d048b14c08e06bb79810179a87993d
Gerrit-Change-Number: 61776
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Comment-Date: Wed, 09 Feb 2022 22:43:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58386 )
Change subject: cpu/x86/lapic: Unconditionally use CPUID leaf 0xb if available
......................................................................
Patch Set 7:
(1 comment)
Patchset:
PS7:
> ok, this is a bug and we'll need to revert. […]
CB:61776
--
To view, visit https://review.coreboot.org/c/coreboot/+/58386
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0f1f46fe5091ebeab6dfb4c7e151150cf495d0cb
Gerrit-Change-Number: 58386
Gerrit-PatchSet: 7
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Wed, 09 Feb 2022 22:38:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/61776 )
Change subject: Revert "cpu/x86/lapic: Unconditionally use CPUID leaf 0xb if available"
......................................................................
Revert "cpu/x86/lapic: Unconditionally use CPUID leaf 0xb if available"
This reverts commit ceaf959678905f44a54a116f37bd15acab5d4608.
The AMD Picasso SoC doesn't support x2APIC and neither advertises the
presence of its support via bit 21 in EAX of CPUID leaf 1 nor has the
bit 10 in the APIC base address MSR 0x1b set, but it does have 0xd CPUID
leaves, so just checking for the presence of that CPUID leaf isn't
sufficient to be sure that EDX of the CPUID leaf 0xb will contain a
valid APIC ID.
In the case of Picasso EDX of the CPUID leaf 0xb returns 0 for all cores
which causes coreboot to get stuck somewhere at the end of MP init.
I'm not 100% sure if we should additionally check bit 21 in EAX of CPUID
function 1 is set instead of adding back the is_x2apic_mode check.
TEST=Mandolin with a Picasso SoC boots again.
Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de>
Change-Id: If1e3c55ce2d048b14c08e06bb79810179a87993d
---
M src/cpu/x86/smm/smm_stub.S
M src/include/cpu/x86/lapic.h
2 files changed, 13 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/61776/1
diff --git a/src/cpu/x86/smm/smm_stub.S b/src/cpu/x86/smm/smm_stub.S
index 25a35aa..c83839c 100644
--- a/src/cpu/x86/smm/smm_stub.S
+++ b/src/cpu/x86/smm/smm_stub.S
@@ -101,25 +101,30 @@
* the OS can manipulate the APIC id use the non-changing cpuid result
* for APIC id (eax). A table is used to handle a discontiguous
* APIC id space. */
+apic_id:
+ mov $LAPIC_BASE_MSR, %ecx
+ rdmsr
+ and $LAPIC_BASE_X2APIC_ENABLED, %eax
+ cmp $LAPIC_BASE_X2APIC_ENABLED, %eax
+ jne xapic
- mov $0, %eax
- cpuid
- cmp $0xb, %eax
- jc 1f
+x2apic:
mov $0xb, %eax
mov $0, %ecx
cpuid
mov %edx, %eax
- jmp 2f
-1:
+ jmp apicid_end
+
+xapic:
mov $1, %eax
cpuid
mov %ebx, %eax
shr $24, %eax
-2:
+apicid_end:
mov $(apic_to_cpu_num), %ebx
xor %ecx, %ecx
+
1:
cmp (%ebx, %ecx, 2), %ax
je 1f
diff --git a/src/include/cpu/x86/lapic.h b/src/include/cpu/x86/lapic.h
index 7006dbc..c509e61 100644
--- a/src/include/cpu/x86/lapic.h
+++ b/src/include/cpu/x86/lapic.h
@@ -126,7 +126,7 @@
static __always_inline unsigned int initial_lapicid(void)
{
uint32_t lapicid;
- if (cpuid_get_max_func() >= 0xb)
+ if (is_x2apic_mode() && cpuid_get_max_func() >= 0xb)
lapicid = cpuid_ext(0xb, 0).edx;
else
lapicid = cpuid_ebx(1) >> 24;
--
To view, visit https://review.coreboot.org/c/coreboot/+/61776
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If1e3c55ce2d048b14c08e06bb79810179a87993d
Gerrit-Change-Number: 61776
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newchange
Attention is currently required from: Jason Glenesk, Raul Rangel, ritul guru, Marshall Dawson, Name of user not set #1004133, Al Hirani, Rob Barnes, Fred Reitberger, Felix Held.
Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61462 )
Change subject: soc/amd/common/block/psp: add PSP command
......................................................................
Patch Set 12:
(1 comment)
File src/soc/amd/common/block/psp/psp_def.h:
https://review.coreboot.org/c/coreboot/+/61462/comment/c213e6b2_e7406d07
PS5, Line 29: CORE_2_PSP_MSG_38_OFFSET
> I left it as MSG_38 because i wanted to segregate the SPL functionality from the MSFT HSTI feature, […]
What i mean to say is that this register contains bits related to may security features, as well as bits 0-3 for HSTI
--
To view, visit https://review.coreboot.org/c/coreboot/+/61462
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0575356a7c6172e2e0f2eaf9d1a6706468fe92d
Gerrit-Change-Number: 61462
Gerrit-PatchSet: 12
Gerrit-Owner: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Al Hirani <al.hirani13(a)gmail.com>
Gerrit-Reviewer: Avinash Alevoor
Gerrit-Reviewer: Fred Reitberger <fred.reitberger(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Mohan Viswanathan
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Name of user not set #1004133
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Name of user not set #1004133
Gerrit-Attention: Al Hirani <al.hirani13(a)gmail.com>
Gerrit-Attention: Rob Barnes <robbarnes(a)google.com>
Gerrit-Attention: Fred Reitberger <fred.reitberger(a)amd.corp-partner.google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 09 Feb 2022 22:26:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: ritul guru <ritul.bits(a)gmail.com>
Comment-In-Reply-To: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com>
Gerrit-MessageType: comment
Attention is currently required from: Kangheui Won, Rizwan Qureshi, Tim Wawrzynczak, Krishna P Bhat D.
Reka Norman has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61764 )
Change subject: mb/google/nissa: Set half_populated true
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
File src/mainboard/google/brya/variants/baseboard/nissa/memory.c:
https://review.coreboot.org/c/coreboot/+/61764/comment/d71a1779_fc5b67c4
PS3, Line 100: ADL-N only has a single memory channel.
Could you please update this comment with a brief explanation?
--
To view, visit https://review.coreboot.org/c/coreboot/+/61764
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I414e5dc82caf47b6b96c474b3ef6e01c2ce0226e
Gerrit-Change-Number: 61764
Gerrit-PatchSet: 3
Gerrit-Owner: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Kangheui Won <khwon(a)chromium.org>
Gerrit-Attention: Rizwan Qureshi <rizwan.qureshi(a)intel.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com>
Gerrit-Comment-Date: Wed, 09 Feb 2022 22:23:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, ritul guru, Marshall Dawson, Name of user not set #1004133, Al Hirani, Rob Barnes, Fred Reitberger, Felix Held.
Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61462 )
Change subject: soc/amd/common/block/psp: add PSP command
......................................................................
Patch Set 12:
(1 comment)
File src/soc/amd/common/block/psp/psp_gen2.c:
https://review.coreboot.org/c/coreboot/+/61462/comment/d35b8ca9_36cd63df
PS12, Line 154: CORE_2_PSP_MSG_38_FUSE_SPL
> I'm not sure if this is correct for all APUs/CPUs that use the second generation of the PSP mailbox […]
Hi Felix, Mohan was able to confirm that this bit is not used for anything else in other purposes in other programs and the comment we saw will need to be corrected.
--
To view, visit https://review.coreboot.org/c/coreboot/+/61462
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0575356a7c6172e2e0f2eaf9d1a6706468fe92d
Gerrit-Change-Number: 61462
Gerrit-PatchSet: 12
Gerrit-Owner: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Al Hirani <al.hirani13(a)gmail.com>
Gerrit-Reviewer: Avinash Alevoor
Gerrit-Reviewer: Fred Reitberger <fred.reitberger(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Mohan Viswanathan
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Name of user not set #1004133
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Name of user not set #1004133
Gerrit-Attention: Al Hirani <al.hirani13(a)gmail.com>
Gerrit-Attention: Rob Barnes <robbarnes(a)google.com>
Gerrit-Attention: Fred Reitberger <fred.reitberger(a)amd.corp-partner.google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 09 Feb 2022 22:22:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58386 )
Change subject: cpu/x86/lapic: Unconditionally use CPUID leaf 0xb if available
......................................................................
Patch Set 7:
(1 comment)
Patchset:
PS7:
> this might totally be a bug in picasso; i don't know yet. […]
ok, this is a bug and we'll need to revert.
From the Picasso PPR:
CPUID_Fn00000001_ECX [Feature Identifiers] (Core::X86::Cpuid::FeatureIdEcx)
Bit 21: X2APIC. Read-only. Reset: Fixed,0. x2APIC capability.
CPUID_Fn00000000_EAX [Processor Vendor and Largest Standard Function Number]
(Core::X86::Cpuid::LargFuncNum)
Bits 31:0: LFuncStd: largest standard function. Read-only. Reset: Fixed,0000_000Dh. The largest CPUID standard function input value supported by the processor implementation.
MSR0000_001B [APIC Base Address] (Core::X86::Msr::APIC_BAR)
Bit 10: Reserved. Read-only,Error-on-write-1. Reset: Fixed,0.
with that bit 10 being LAPIC_BASE_MSR_X2APIC_MODE
so Picasso has the CPUID leaf, but no x2apic support and cpuid_ext(0xb, 0).edx doesn't return valid data.
I'll push a revert
--
To view, visit https://review.coreboot.org/c/coreboot/+/58386
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0f1f46fe5091ebeab6dfb4c7e151150cf495d0cb
Gerrit-Change-Number: 58386
Gerrit-PatchSet: 7
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Wed, 09 Feb 2022 22:20:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, ritul guru, Marshall Dawson, Name of user not set #1004133, Al Hirani, Rob Barnes, Fred Reitberger.
Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61462 )
Change subject: soc/amd/common/block/psp: add PSP command
......................................................................
Patch Set 12:
(1 comment)
File src/soc/amd/common/block/psp/psp_def.h:
https://review.coreboot.org/c/coreboot/+/61462/comment/1515da0c_dabe77ad
PS5, Line 29: CORE_2_PSP_MSG_38_OFFSET
> can we have more descriptive name PSB_HSTI_STATUS_OFFSET?
I left it as MSG_38 because i wanted to segregate the SPL functionality from the MSFT HSTI feature, which might not be so relevant here in coreboot.
--
To view, visit https://review.coreboot.org/c/coreboot/+/61462
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: If0575356a7c6172e2e0f2eaf9d1a6706468fe92d
Gerrit-Change-Number: 61462
Gerrit-PatchSet: 12
Gerrit-Owner: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Al Hirani <al.hirani13(a)gmail.com>
Gerrit-Reviewer: Avinash Alevoor
Gerrit-Reviewer: Fred Reitberger <fred.reitberger(a)amd.corp-partner.google.com>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Mohan Viswanathan
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-CC: Name of user not set #1004133
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Attention: ritul guru <ritul.bits(a)gmail.com>
Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Attention: Name of user not set #1004133
Gerrit-Attention: Al Hirani <al.hirani13(a)gmail.com>
Gerrit-Attention: Rob Barnes <robbarnes(a)google.com>
Gerrit-Attention: Fred Reitberger <fred.reitberger(a)amd.corp-partner.google.com>
Gerrit-Comment-Date: Wed, 09 Feb 2022 22:20:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: ritul guru <ritul.bits(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Nick Vaccaro, Karthik Ramasubramanian.
Reka Norman has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61730 )
Change subject: util/spd_tools/spd_gen/lp5: Encode Optional SDRAM features
......................................................................
Patch Set 1:
(2 comments)
File util/spd_tools/src/spd_gen/lp5.go:
https://review.coreboot.org/c/coreboot/+/61730/comment/16190286_766a1b10
PS1, Line 201: 5
nit: 5:4
https://review.coreboot.org/c/coreboot/+/61730/comment/478f8b81_e3491b8a
PS1, Line 211: 0x40
Shouldn't this be 0x20?
--
To view, visit https://review.coreboot.org/c/coreboot/+/61730
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Icac8ae148458162768a919d9690d7bf96734e6c0
Gerrit-Change-Number: 61730
Gerrit-PatchSet: 1
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Wed, 09 Feb 2022 22:18:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Rob Barnes, Karthik Ramasubramanian, Felix Held.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61259 )
Change subject: soc/amd/cezanne: Turn off gpp clock request for disabled devices
......................................................................
Patch Set 19:
(5 comments)
File src/soc/amd/cezanne/fch.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-140458):
https://review.coreboot.org/c/coreboot/+/61259/comment/20fbb07d_85f7ec53
PS19, Line 161: /*
trailing whitespace
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-140458):
https://review.coreboot.org/c/coreboot/+/61259/comment/2418a63f_6af4ce6c
PS19, Line 167: printk(BIOS_WARNING,
trailing whitespace
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-140458):
https://review.coreboot.org/c/coreboot/+/61259/comment/4a46a1a5_f764430b
PS19, Line 168: "CLK_ENABLE is an invalid clk_req value for PCIe device %d.%d, DXIO descriptior %d\n",
'descriptior' may be misspelled - perhaps 'descriptor'?
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-140458):
https://review.coreboot.org/c/coreboot/+/61259/comment/b4422d53_b1f29752
PS19, Line 181: dxio_desc->device_number, dxio_desc->function_number, i);
line over 96 characters
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-140458):
https://review.coreboot.org/c/coreboot/+/61259/comment/658f300f_80e1d139
PS19, Line 221: } else if (pci_device->enabled
trailing whitespace
--
To view, visit https://review.coreboot.org/c/coreboot/+/61259
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I77389372c60bdec572622a3b49484d4789fd4e4c
Gerrit-Change-Number: 61259
Gerrit-PatchSet: 19
Gerrit-Owner: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Eric Peers <epeers(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Rob Barnes <robbarnes(a)google.com>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 09 Feb 2022 22:17:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment