coreboot-gerrit December 2021

coreboot-gerrit@coreboot.org

1 participants
5525 discussions

Change in coreboot[master]: ec/lenovo/h8/acpi: Replace LAnd() with ACPI 2.0 syntax
by Felix Singer (Code Review) 29 Dec '21

29 Dec '21

Attention is currently required from: Alexander Couzens. Hello build bot (Jenkins), Alexander Couzens, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/60462 to look at the new patch set (#2). Change subject: ec/lenovo/h8/acpi: Replace LAnd() with ACPI 2.0 syntax ...................................................................... ec/lenovo/h8/acpi: Replace LAnd() with ACPI 2.0 syntax Replace `LAnd (a, b)` with `a && b`. Change-Id: Ic578506dd2a1ab4341361f1a3b435372f2dac260 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/ec/lenovo/h8/acpi/thermal.asl 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/60462/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/60462 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic578506dd2a1ab4341361f1a3b435372f2dac260 Gerrit-Change-Number: 60462 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: ec/hp/kbc1126/acpi: Use Printf() for debug prints
by 9elements QA (Code Review) 29 Dec '21

29 Dec '21

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60383 ) Change subject: ec/hp/kbc1126/acpi: Use Printf() for debug prints ...................................................................... Patch Set 5: Automatic boot test returned (PASS/FAIL/TOTAL): 7 / 1 / 8 PASS: x86_32 "ThinkPad T500" , build config LENOVO_T500 and payload SeaBIOS : https://lava.9esec.io/r/86135 PASS: x86_32 "HP Z220 SFF Workstation" , build config HP_Z220_SFF_WORKSTATION and payload LinuxBoot_BB_kexec : https://lava.9esec.io/r/86134 PASS: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload SeaBIOS : https://lava.9esec.io/r/86133 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload SeaBIOS : https://lava.9esec.io/r/86132 PASS: x86_64 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_X86_64 and payload SeaBIOS : https://lava.9esec.io/r/86131 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ASAN and payload SeaBIOS : https://lava.9esec.io/r/86130 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ and payload SeaBIOS : https://lava.9esec.io/r/86129 FAIL: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX and payload SeaBIOS : https://lava.9esec.io/r/86128 Please note: This test is under development and might not be accurate at all! -- To view, visit https://review.coreboot.org/c/coreboot/+/60383 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id343bde61706d4d1de2102c3567141e427d8126d Gerrit-Change-Number: 60383 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Iru Cai <mytbk920423(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Wed, 29 Dec 2021 14:58:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/apollolake/acpi: Replace LAnd() operations with ACPI 2.0 sy...
by Felix Singer (Code Review) 29 Dec '21

29 Dec '21

Attention is currently required from: Andrey Petrov, Patrick Rudolph. Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60466 ) Change subject: soc/intel/apollolake/acpi: Replace LAnd() operations with ACPI 2.0 syntax ...................................................................... soc/intel/apollolake/acpi: Replace LAnd() operations with ACPI 2.0 syntax Replace `LAnd (a, b)` with `a && b`. Change-Id: I4bbbc4888fc134b3862bb956b2ee17a72f282584 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/soc/intel/apollolake/acpi/pch_hda.asl M src/soc/intel/apollolake/acpi/pcie_port.asl M src/soc/intel/apollolake/acpi/scs.asl 3 files changed, 5 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/60466/1 diff --git a/src/soc/intel/apollolake/acpi/pch_hda.asl b/src/soc/intel/apollolake/acpi/pch_hda.asl index c3f862e..6c9a14a 100644 --- a/src/soc/intel/apollolake/acpi/pch_hda.asl +++ b/src/soc/intel/apollolake/acpi/pch_hda.asl @@ -39,9 +39,7 @@ * NHLT Query only supported for revision 1 and * if NHLT address and length are set in NVS. */ - If (LAnd (LEqual (Arg1, One), - LAnd (LNotEqual (NHLA, Zero), - LNotEqual (NHLL, Zero)))) { + If (LEqual (Arg1, One) && LNotEqual (NHLA, Zero) && LNotEqual (NHLL, Zero)) { Return (Buffer (One) { 0x03 }) } Else { diff --git a/src/soc/intel/apollolake/acpi/pcie_port.asl b/src/soc/intel/apollolake/acpi/pcie_port.asl index f0f49f3..710051b 100644 --- a/src/soc/intel/apollolake/acpi/pcie_port.asl +++ b/src/soc/intel/apollolake/acpi/pcie_port.asl @@ -49,7 +49,7 @@ Method (_ON, 0, Serialized) { - If (LAnd (LEqual (PDST, 1), LNotEqual (\PRT0, 0))) { + If (LEqual (PDST, 1) && LNotEqual (\PRT0, 0)) { /* Enter this condition if device * is connected */ @@ -83,7 +83,7 @@ Method (_OFF, 0, Serialized) { /* Set L23_Rdy Entry Request (L23ER) */ - If (LAnd (LEqual (PDST, 1), LNotEqual (\PRT0, 0))) { + If (LEqual (PDST, 1) && LNotEqual (\PRT0, 0)) { /* enter this condition if device * is connected */ diff --git a/src/soc/intel/apollolake/acpi/scs.asl b/src/soc/intel/apollolake/acpi/scs.asl index 1ee70ad..a5a1f06 100644 --- a/src/soc/intel/apollolake/acpi/scs.asl +++ b/src/soc/intel/apollolake/acpi/scs.asl @@ -118,7 +118,7 @@ Method (_INI, 0) { /* Check SDCard CD port is valid */ - If (LAnd (LNotEqual (\SCDP, 0), LNotEqual (\SCDO, 0) )) + If (LNotEqual (\SCDP, 0) && LNotEqual (\SCDO, 0)) { /* Store DW0 address of SD_CD */ Store (GDW0 (\SCDP, \SCDO), SCD0) @@ -133,7 +133,7 @@ Method (_PS0, 0, NotSerialized) { /* Check SDCard CD port is valid */ - If (LAnd (LNotEqual (\SCDP, 0), LNotEqual (\SCDO, 0) )) + If (LNotEqual (\SCDP, 0) && LNotEqual (\SCDO, 0)) { /* Store DW0 into local0 to get rxstate of GPIO */ Store (\_SB.GPC0 (SCD0), Local0) -- To view, visit https://review.coreboot.org/c/coreboot/+/60466 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4bbbc4888fc134b3862bb956b2ee17a72f282584 Gerrit-Change-Number: 60466 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: ec/google/chromeec/acpi: Replace LAnd() operations with ACPI 2.0 syntax
by Felix Singer (Code Review) 29 Dec '21

29 Dec '21

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60465 ) Change subject: ec/google/chromeec/acpi: Replace LAnd() operations with ACPI 2.0 syntax ...................................................................... ec/google/chromeec/acpi: Replace LAnd() operations with ACPI 2.0 syntax Replace `LAnd (a, b)` with `a && b`. Change-Id: I7d74e6a2ce4ee98c1c0f5b412e20661c5196735e Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/ec/google/chromeec/acpi/battery.asl 1 file changed, 2 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/60465/1 diff --git a/src/ec/google/chromeec/acpi/battery.asl b/src/ec/google/chromeec/acpi/battery.asl index fc9edc9..073905e 100644 --- a/src/ec/google/chromeec/acpi/battery.asl +++ b/src/ec/google/chromeec/acpi/battery.asl @@ -201,7 +201,7 @@ // 2: BATTERY REMAINING CAPACITY // Store (BTRA, Local1) - If (LAnd (Arg3, LAnd (ACEX, LNot (LAnd (BFDC, BFCG))))) { + If (Arg3 && ACEX && LNot (BFDC && BFCG)) { // On AC power and battery is neither charging // nor discharging. Linux expects a full battery // to report same capacity as last full charge. @@ -210,8 +210,7 @@ // See if within ~6% of full ShiftRight (Local2, 4, Local3) - If (LAnd (LGreater (Local1, Subtract (Local2, Local3)), - LLess (Local1, Add (Local2, Local3)))) + If (LGreater (Local1, Subtract (Local2, Local3)) && LLess (Local1, Add (Local2, Local3)))) { Store (Local2, Local1) } -- To view, visit https://review.coreboot.org/c/coreboot/+/60465 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7d74e6a2ce4ee98c1c0f5b412e20661c5196735e Gerrit-Change-Number: 60465 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: ec/quanta/it8518/acpi: Replace LAnd() operations with ACPI 2.0 syntax
by Felix Singer (Code Review) 29 Dec '21

29 Dec '21

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60464 ) Change subject: ec/quanta/it8518/acpi: Replace LAnd() operations with ACPI 2.0 syntax ...................................................................... ec/quanta/it8518/acpi: Replace LAnd() operations with ACPI 2.0 syntax Replace `LAnd (a, b)` with `a && b`. Change-Id: I72604a0efa2d8fcdf39cf5a8b70082aeb32dddab Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/ec/quanta/it8518/acpi/battery.asl 1 file changed, 2 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/60464/1 diff --git a/src/ec/quanta/it8518/acpi/battery.asl b/src/ec/quanta/it8518/acpi/battery.asl index adbde93..de51206 100644 --- a/src/ec/quanta/it8518/acpi/battery.asl +++ b/src/ec/quanta/it8518/acpi/battery.asl @@ -306,7 +306,7 @@ Store (ECRC, Local1) } - If (LAnd (BFWK, LAnd (ACPW, LNot (Local0)))) + If (BFWK && ACPW && LNot (Local0)) { // On AC power and battery is neither charging // nor discharging. Linux expects a full battery @@ -317,8 +317,7 @@ // See if within ~3% of full ShiftRight (Local2, 5, Local3) - If (LAnd (LGreater (Local1, Subtract (Local2, Local3)), - LLess (Local1, Add (Local2, Local3)))) + If (LGreater (Local1, Subtract (Local2, Local3)) && LLess (Local1, Add (Local2, Local3))) { Store (Local2, Local1) } -- To view, visit https://review.coreboot.org/c/coreboot/+/60464 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I72604a0efa2d8fcdf39cf5a8b70082aeb32dddab Gerrit-Change-Number: 60464 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: ec/smsc/mec1308/acpi: Replace LAnd() operations with ACPI 2.0 syntax
by Felix Singer (Code Review) 29 Dec '21

29 Dec '21

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60463 ) Change subject: ec/smsc/mec1308/acpi: Replace LAnd() operations with ACPI 2.0 syntax ...................................................................... ec/smsc/mec1308/acpi: Replace LAnd() operations with ACPI 2.0 syntax Replace `LAnd (a, b)` with `a && b`. Change-Id: I8c3323e1d367872e9f2a134eaf4ebd9e35f74313 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/ec/smsc/mec1308/acpi/battery.asl 1 file changed, 5 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/60463/1 diff --git a/src/ec/smsc/mec1308/acpi/battery.asl b/src/ec/smsc/mec1308/acpi/battery.asl index a072c93..bbbcfe2 100644 --- a/src/ec/smsc/mec1308/acpi/battery.asl +++ b/src/ec/smsc/mec1308/acpi/battery.asl @@ -133,8 +133,7 @@ // Store (SWAB (BTPR), Local1) - If (LAnd (LNotEqual (Local1, 0xFFFFFFFF), - LGreaterEqual (Local1, 0x8000))) { + If (LNotEqual (Local1, 0xFFFFFFFF) && LGreaterEqual (Local1, 0x8000)) { Xor (Local1, 0xFFFF, Local1) Increment (Local1) } @@ -144,13 +143,12 @@ // 2: BATTERY REMAINING CAPACITY // Store (SWAB (BTRA), Local1) - If (LAnd (LNotEqual (Local1, 0xFFFFFFFF), - LGreaterEqual (Local1, 0x8000))) { + If (LNotEqual (Local1, 0xFFFFFFFF) && LGreaterEqual (Local1, 0x8000)) { Xor (Local1, 0xFFFF, Local1) Increment (Local1) } - If (LAnd (BFWK, LAnd (ACEX, LNot (Local0)))) { + If (BFWK && ACEX && LNot (Local0)) { // On AC power and battery is neither charging // nor discharging. Linux expects a full battery // to report same capacity as last full charge. @@ -159,8 +157,8 @@ // See if within ~3% of full ShiftRight (Local2, 5, Local3) - If (LAnd (LGreater (Local1, Subtract (Local2, Local3)), - LLess (Local1, Add (Local2, Local3)))) + If (LGreater (Local1, Subtract (Local2, Local3)) && + LLess (Local1, Add (Local2, Local3))) { Store (Local2, Local1) } -- To view, visit https://review.coreboot.org/c/coreboot/+/60463 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8c3323e1d367872e9f2a134eaf4ebd9e35f74313 Gerrit-Change-Number: 60463 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: ec/lenovo/h8/acpi: Replace LAnd() operations with ACPI 2.0 syntax
by Felix Singer (Code Review) 29 Dec '21

29 Dec '21

Felix Singer has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60462 ) Change subject: ec/lenovo/h8/acpi: Replace LAnd() operations with ACPI 2.0 syntax ...................................................................... ec/lenovo/h8/acpi: Replace LAnd() operations with ACPI 2.0 syntax Replace `LAnd (a, b)` with `a && b`. Change-Id: Ic578506dd2a1ab4341361f1a3b435372f2dac260 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> --- M src/ec/lenovo/h8/acpi/thermal.asl 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/60462/1 diff --git a/src/ec/lenovo/h8/acpi/thermal.asl b/src/ec/lenovo/h8/acpi/thermal.asl index fe2d287..b21dafc 100644 --- a/src/ec/lenovo/h8/acpi/thermal.asl +++ b/src/ec/lenovo/h8/acpi/thermal.asl @@ -73,7 +73,7 @@ Method(_TMP) { #if defined(EC_LENOVO_H8_ME_WORKAROUND) /* Avoid tripping alarm if ME isn't booted at all yet */ - If (LAnd (LNot (MEB1), LEqual (\_SB.PCI0.LPCB.EC.TMP0, 128))) { + If (LNot (MEB1) && LEqual (\_SB.PCI0.LPCB.EC.TMP0, 128)) { Return (C2K(40)) } Store (1, MEB1) @@ -160,7 +160,7 @@ Method(_TMP) { #if defined(EC_LENOVO_H8_ME_WORKAROUND) /* Avoid tripping alarm if ME isn't booted at all yet */ - If (LAnd (LNot (MEB2), LEqual (\_SB.PCI0.LPCB.EC.TMP1, 128))) { + If (LNot (MEB2) && LEqual (\_SB.PCI0.LPCB.EC.TMP1, 128)) { Return (C2K(40)) } Store (1, MEB2) -- To view, visit https://review.coreboot.org/c/coreboot/+/60462 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic578506dd2a1ab4341361f1a3b435372f2dac260 Gerrit-Change-Number: 60462 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/intel/alderlake: Add option to make MRC log silent
by Nico Huber (Code Review) 29 Dec '21

29 Dec '21

Attention is currently required from: Felix Singer, Subrata Banik, Tim Wawrzynczak, Angel Pons, Arthur Heymans, Nick Vaccaro, Andrey Petrov, Patrick Rudolph. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60441 ) Change subject: soc/intel/alderlake: Add option to make MRC log silent ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > This is what my understanding is: > > 1. coreboot and FSP MRC log level are different for example: typical log level for coreboot is 8 where else for MRC the most verbose level is 5. > > Hence, the UPD assignment like below will set the wrong value to UPD. Isn't it? > m_cfg->SerialDebugMrcLevel = CONFIG_DEFAULT_CONSOLE_LOGLEVEL; > Yes, that's why I said "map". > 2. Most of the time, we want to see FSP debug logs but not that verbose MRC debug log which takes several minutes to boot the platform hence, we might still prefer to have different log levels between coreboot and FSP. And for that matter inside FSP even there are 2 different log level (there is one more UPD PcdSerialDebugLevel which controls overall FSP entire debug log level). But the intention is to just make FSP-M silent without additional MRC training prints. Why disable error messages then? Anyway, for verbose training output there is config DEBUG_RAM_SETUP. > > May be we can drop `SOC_INTEL_ALDERLAKE_MRC_DEBUG_CONSENT` config and assign the UPD like this ? > > if (CONFIG(SILENT_FSP_M_DEBUG_MESSAGE)) > m_cfg->SerialDebugMrcLevel = CONFIG_DEFAULT_CONSOLE_LOGLEVEL_0; > else > m_cfg->SerialDebugMrcLevel = CONFIG_DEFAULT_CONSOLE_LOGLEVEL_3; This would only set it to 1 in odd cases? I don't follow. Did you mean constant 0 / 3? Also, the console log level is a runtime setting. It's not easy to query right now, but the API could be changed. Maybe something like this (with to-be-written get_max_console_log_level()): int fspm_debug_level; switch (get_max_console_log_level(CONSOLE_LOG_ALL)) { case BIOS_EMERG: case BIOS_ALERT: case BIOS_CRIT: case BIOS_ERR: fspm_debug_level = 1; break; case BIOS_WARNING: fspm_debug_level = 2; break; case BIOS_NOTICE: fspm_debug_level = 3; break; case BIOS_INFO: fspm_debug_level = 4; break; case BIOS_DEBUG: case BIOS_SPEW: fspm_debug_level = 5; break; default: fspm_debug_level = 0; break; } if (!CONFIG(DEBUG_RAM_SETUP)) fspm_debug_level = MIN(fspm_debug_level, 2); m_cfg->SerialDebugMrcLevel = fspm_debug_level; > > in that way, we can reduce to only 1 new Kconfig rather SoC wise Kconfig. WDYT? I think this isn't FSP specific. It doesn't matter if one chose to use a blob instead of writing native code. coreboot already provides all the needed options. -- To view, visit https://review.coreboot.org/c/coreboot/+/60441 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iea3b32feca0893a83fdf700798b0883d26ccc718 Gerrit-Change-Number: 60441 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Subrata Banik <subratabanik(a)google.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: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 29 Dec 2021 14:35:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Add option to make MRC log silent
by Subrata Banik (Code Review) 29 Dec '21

29 Dec '21

Attention is currently required from: Felix Singer, Nico Huber, Tim Wawrzynczak, Angel Pons, Arthur Heymans, Nick Vaccaro, Andrey Petrov, Patrick Rudolph. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60441 ) Change subject: soc/intel/alderlake: Add option to make MRC log silent ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Why not map the console loglevel to this UPD? That would avoid adding more Kconfigs. This is what my understanding is: 1. coreboot and FSP MRC log level are different for example: typical log level for coreboot is 8 where else for MRC the most verbose level is 5. Hence, the UPD assignment like below will set the wrong value to UPD. Isn't it? m_cfg->SerialDebugMrcLevel = CONFIG_DEFAULT_CONSOLE_LOGLEVEL; 2. Most of the time, we want to see FSP debug logs but not that verbose MRC debug log which takes several minutes to boot the platform hence, we might still prefer to have different log levels between coreboot and FSP. And for that matter inside FSP even there are 2 different log level (there is one more UPD PcdSerialDebugLevel which controls overall FSP entire debug log level). But the intention is to just make FSP-M silent without additional MRC training prints. May be we can drop `SOC_INTEL_ALDERLAKE_MRC_DEBUG_CONSENT` config and assign the UPD like this ? if (CONFIG(SILENT_FSP_M_DEBUG_MESSAGE)) m_cfg->SerialDebugMrcLevel = CONFIG_DEFAULT_CONSOLE_LOGLEVEL_0; else m_cfg->SerialDebugMrcLevel = CONFIG_DEFAULT_CONSOLE_LOGLEVEL_3; in that way, we can reduce to only 1 new Kconfig rather SoC wise Kconfig. WDYT? -- To view, visit https://review.coreboot.org/c/coreboot/+/60441 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iea3b32feca0893a83fdf700798b0883d26ccc718 Gerrit-Change-Number: 60441 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> 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: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 29 Dec 2021 13:55:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/dedede/var/magolor: Set core display clock to 172.8 MHz
by Nico Huber (Code Review) 29 Dec '21

29 Dec '21

Attention is currently required from: Henry Sun, Paul Menzel, Ren Kuo, Simon Yang, Angel Pons, Kane Chen, Karthik Ramasubramanian. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/60301 ) Change subject: mb/google/dedede/var/magolor: Set core display clock to 172.8 MHz ...................................................................... Patch Set 9: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/60301/comment/1d53a0cd_c720cb06 PS9, Line 12: per Intel recommendation > This is a short term WA and will be fixed in newer version of FSP in long term. Interesting. I guess if any commit message had started with this, it would have saved us some hours of review. Also, why add a devicetree setting for a temporary workaround? Could have used mainboard_silicon_init_params(). -- To view, visit https://review.coreboot.org/c/coreboot/+/60301 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5a0ad2bed79b37775184f0bd0a0ef024900cbe34 Gerrit-Change-Number: 60301 Gerrit-PatchSet: 9 Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Simon Yang <simon1.yang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Henry Sun <henrysun(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Simon Yang <simon1.yang(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 29 Dec 2021 13:48:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Simon Yang <simon1.yang(a)intel.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2021