coreboot-gerrit September 2020

coreboot-gerrit@coreboot.org

1 participants
2629 discussions

Change in coreboot[master]: cpu/qemu-x86: Drop select SMP
by Kyösti Mälkki (Code Review) 27 Sep '20

27 Sep '20

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43865 ) Change subject: cpu/qemu-x86: Drop select SMP ...................................................................... cpu/qemu-x86: Drop select SMP With MAX_CPUS==1, this has the effect of removing spinlock implementation. But since is_smp_boot() evaluates false and SMM uses separate smi_semaphore, there is no concurrency to protect against with a spinlock. Change-Id: I7c2ac221af78055879e7359bd03907f2416a9919 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M src/cpu/qemu-x86/Kconfig 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/43865/1 diff --git a/src/cpu/qemu-x86/Kconfig b/src/cpu/qemu-x86/Kconfig index 21ada02..3136162 100644 --- a/src/cpu/qemu-x86/Kconfig +++ b/src/cpu/qemu-x86/Kconfig @@ -6,7 +6,6 @@ select ARCH_VERSTAGE_X86_32 select ARCH_ROMSTAGE_X86_32 select ARCH_RAMSTAGE_X86_32 - select SMP select UDELAY_TSC select TSC_MONOTONIC_TIMER select UNKNOWN_TSC_RATE -- To view, visit https://review.coreboot.org/c/coreboot/+/43865 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7c2ac221af78055879e7359bd03907f2416a9919 Gerrit-Change-Number: 43865 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newchange

4 8

Change in coreboot[master]: [WIP] mb/emulation/qemu-i440fx: Remove TRACE=y from test build
by Kyösti Mälkki (Code Review) 27 Sep '20

27 Sep '20

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43817 ) Change subject: [WIP] mb/emulation/qemu-i440fx: Remove TRACE=y from test build ...................................................................... [WIP] mb/emulation/qemu-i440fx: Remove TRACE=y from test build Looks like the option is generally not compatible with garbage collections. Change-Id: I1b76dc34b5f39d8988368f71a0a2f43d1bc4177e Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M configs/config.emulation_qemu_x86_i440fx_debug 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/43817/1 diff --git a/configs/config.emulation_qemu_x86_i440fx_debug b/configs/config.emulation_qemu_x86_i440fx_debug index 011f163..e39ff59 100644 --- a/configs/config.emulation_qemu_x86_i440fx_debug +++ b/configs/config.emulation_qemu_x86_i440fx_debug @@ -4,6 +4,5 @@ CONFIG_DEBUG_CBFS=y CONFIG_DEBUG_PIRQ=y CONFIG_DEBUG_MALLOC=y -CONFIG_TRACE=y CONFIG_DEBUG_BOOT_STATE=y CONFIG_DEBUG_ADA_CODE=y -- To view, visit https://review.coreboot.org/c/coreboot/+/43817 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1b76dc34b5f39d8988368f71a0a2f43d1bc4177e Gerrit-Change-Number: 43817 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newchange

3 15

Change in coreboot[master]: soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL S0ix qualification opt...
by Nico Huber (Code Review) 27 Sep '20

27 Sep '20

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45723 ) Change subject: soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL S0ix qualification optional ...................................................................... Patch Set 3: (1 comment) https://review.coreboot.org/c/coreboot/+/45723/2/src/soc/intel/cannonlake/c… File src/soc/intel/cannonlake/chip.h: https://review.coreboot.org/c/coreboot/+/45723/2/src/soc/intel/cannonlake/c… PS2, Line 61: uint8_t s0ix_allow_xtal_on; > > I'm afraid I don't follow. Intel documents both the XTAL shutdown and the SLP_S0# assertion (what is what we want to achieve with S0ix, AIUI) as effects of low-power state 3 (LP3). Not as requirements of one another. > > This is my understanding: On C10 entry, PMC checks clock requests for XTAL to determine whether it is okay to enter S0i3 (LP3) i.e. XTAL is one of the qualification factors for S0i3 entry. However, it can be ignored by setting the XTAL disqualification bit. Thus, even if XTAL remains enabled on C10 entry, PMC ignores its state when making the determination for S0i3 entry. Well, I have to repeat it: There is no XTAL disqualification bit. The exact name is: 24MHz Crystal Shutdown Qualification Disable (XTALSDQDIS). It's not about the XTAL, but about shutting the XTAL down. This confusion makes all prior discussions hard to assess. But it's ambiguous anyway: Either LP3 is qualified by the shutdown or the shutdown is qualified by LP3 *shrug* So far I'm still convinced of my original interpretation: * XTALSDQDIS == 0, XTAL will be shut down in LP3. * XTALSDQDIS == 1, XTAL won't be shut down. That would mean it doesn't block other parts from powering down. Maybe we should leave all this "qualification" reasoning aside for a moment and first discuss which setting improves power savings? > > > Where is their relation documented? IIRC, we can't have S0ix with that timer anyway. > > I never found actual documentation on this. Mostly during discussions with Intel. There was a patch series at one point that Intel had: https://review.coreboot.org/c/coreboot/+/22299 which basically indicated that: Thanks I read through the comments of these patches. Most interesting statement: "XTAL is not required for WoV since FRO/Audio PLL is a clock source to DSP." (CB:22237). This says the exact opposite of the patch that introduced this (CB:19442). If it was an accident, I guess we won't need an option. Neither of the commit messages states clearly if the new setting is supposed to save power or not, btw. > 8254 clock gating is required for XTAL shutdown > XTAL shutdown is necessary for S0ix unless disqualification bit is set. I will only start trusting the latter statement when somebody measured SLP_S0# assertion. It just doesn't make much sense. If it is optional hardware-wise, when would one want to block low-power states because the XTAL is still running? Also, CB:22237 wanted to *clear* the bit, and it seems from the comments that they wanted to save power. Which would imply the opposite of this statement. And all the power-saving questions aside: Did CB:19442 fix anything that Chromebooks need? -- To view, visit https://review.coreboot.org/c/coreboot/+/45723 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I17bac9b06e5291b1548704744e872b22b2435c9c Gerrit-Change-Number: 45723 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Naresh Solanki <naresh.solanki(a)intel.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 26 Sep 2020 23:05:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/*: add devicetree option for keeping XTAL running in s0ix
by Michael Niewöhner (Code Review) 27 Sep '20

27 Sep '20

Michael Niewöhner has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/45724 ) Change subject: mb/*: add devicetree option for keeping XTAL running in s0ix ...................................................................... Abandoned squashed -- To view, visit https://review.coreboot.org/c/coreboot/+/45724 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib0a823af7521dd763251a53e3c03ba1a183d0889 Gerrit-Change-Number: 45724 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Elyes HAOUAS Gerrit-Reviewer: Frank Wu <frank_wu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

Change in coreboot[master]: soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL S0ix qualification opt...
by Michael Niewöhner (Code Review) 27 Sep '20

27 Sep '20

Hello Felix Singer, build bot (Jenkins), Nico Huber, Furquan Shaikh, Matt DeVillier, Tim Wawrzynczak, Paul Menzel, Subrata Banik, Aamir Bohra, Naresh Solanki, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/45723 to look at the new patch set (#3). Change subject: soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL S0ix qualification optional ...................................................................... soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL S0ix qualification optional Low Power Audio and Intel Smart Sound Technology, which includes features like Wake-on-Voice, require the XTAL to run even in S0ix. This requires the XTAL S0ix qualification bit to make the PMC ignore the XTAL state, which normally blocks S0ix. However, not all devices support or use the aforementioned audio technologies and keeping XTAL running draws about 2 mW of (battery) power for no reason in low power states. This patch adds a new devicetree option for XTAL S0ix (dis)qualification to make this setting conditional. This change also adds the newly introduced devicetree option to allow S0ix being entered with XTAL running. Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de> Change-Id: I17bac9b06e5291b1548704744e872b22b2435c9c --- M src/mainboard/google/dedede/variants/baseboard/devicetree.cb M src/mainboard/google/drallion/variants/drallion/devicetree.cb M src/mainboard/google/fizz/variants/baseboard/devicetree.cb M src/mainboard/google/hatch/variants/baseboard/devicetree.cb M src/mainboard/google/poppy/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/intel/cannonlake_rvp/variants/cnl_u/devicetree.cb M src/mainboard/intel/cannonlake_rvp/variants/cnl_y/devicetree.cb M src/mainboard/intel/coffeelake_rvp/variants/cfl_u/overridetree.cb M src/mainboard/intel/jasperlake_rvp/variants/jslrvp/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb M src/soc/intel/cannonlake/chip.h M src/soc/intel/cannonlake/finalize.c M src/soc/intel/elkhartlake/chip.h M src/soc/intel/elkhartlake/finalize.c M src/soc/intel/icelake/chip.h M src/soc/intel/icelake/finalize.c M src/soc/intel/jasperlake/chip.h M src/soc/intel/jasperlake/finalize.c M src/soc/intel/skylake/chip.h M src/soc/intel/skylake/finalize.c M src/soc/intel/tigerlake/chip.h M src/soc/intel/tigerlake/finalize.c 24 files changed, 63 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/45723/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/45723 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I17bac9b06e5291b1548704744e872b22b2435c9c Gerrit-Change-Number: 45723 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Naresh Solanki <naresh.solanki(a)intel.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL in s0ix conditional
by Michael Niewöhner (Code Review) 27 Sep '20

27 Sep '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45723 ) Change subject: soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL in s0ix conditional ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/45723/2/src/soc/intel/cannonlake/c… File src/soc/intel/cannonlake/chip.h: https://review.coreboot.org/c/coreboot/+/45723/2/src/soc/intel/cannonlake/c… PS2, Line 61: uint8_t s0ix_allow_xtal_on; >> Also, we might have to take USE_LEGACY_8254_TIMER into consideration i.e. if CONFIG_USE_LEGACY_8254_TIMER is enabled, then xtal_blocks_s0ix must be false. > Where is their relation documented? IIRC, we can't have S0ix with that timer anyway. Oh yes, I agree here. Fsp.bsf for Cometlake1 says that TCO timer must be clock gated to allow S0ix. -- To view, visit https://review.coreboot.org/c/coreboot/+/45723 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I17bac9b06e5291b1548704744e872b22b2435c9c Gerrit-Change-Number: 45723 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Naresh Solanki <naresh.solanki(a)intel.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 26 Sep 2020 22:33:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL in s0ix conditional
by Michael Niewöhner (Code Review) 27 Sep '20

27 Sep '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45723 ) Change subject: soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL in s0ix conditional ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/45723/2/src/soc/intel/cannonlake/c… File src/soc/intel/cannonlake/chip.h: https://review.coreboot.org/c/coreboot/+/45723/2/src/soc/intel/cannonlake/c… PS2, Line 61: uint8_t s0ix_allow_xtal_on; > Nico: Furquan is right here. […] Nico: from Intel doc# 332691-003EN. This clearly describes the dependency IMO 24MHz Crystal Shutdown Qualification Disable (XTALSDQDIS): 0 = SLP_S0# assertion requires the 24MHz Crystal Oscillator to be shutdown. Once SLP_S0# is asserted, the Crystal oscillator should be kept off until PMC notifies it is allowed to be re-enabled. 1 = SLP_S0# assertion does not require the 24MHz Crystal to be shutdown. -- To view, visit https://review.coreboot.org/c/coreboot/+/45723 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I17bac9b06e5291b1548704744e872b22b2435c9c Gerrit-Change-Number: 45723 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Naresh Solanki <naresh.solanki(a)intel.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 26 Sep 2020 22:31:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL in s0ix conditional
by Michael Niewöhner (Code Review) 27 Sep '20

27 Sep '20

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45723 ) Change subject: soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL in s0ix conditional ...................................................................... Patch Set 2: (3 comments) https://review.coreboot.org/c/coreboot/+/45723/2//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/45723/2//COMMIT_MSG@16 PS2, Line 16: This patch adds a new devicetree option for XTAL (dis)qualification and > Disqualificaton of shutting something down is not exactly the same […] Huh? It's not (dis)qualifying of shutting down but (dis)qualifying for s0ix. AIUI When XTAL is qualified for s0ix, it is allowed to be on. When it's disqualified for s0ix, it's not allowed. https://review.coreboot.org/c/coreboot/+/45723/2/src/soc/intel/cannonlake/c… File src/soc/intel/cannonlake/chip.h: https://review.coreboot.org/c/coreboot/+/45723/2/src/soc/intel/cannonlake/c… PS2, Line 60: /* Enable XTAL oscillator qualification for S0ix */ > The comment is neither accurate nor necessary. If you want to avoid to […] IMO it is. This is *not* qualification for shutting down but qualification for s0ix. See below https://review.coreboot.org/c/coreboot/+/45723/2/src/soc/intel/cannonlake/c… PS2, Line 61: uint8_t s0ix_allow_xtal_on; Nico: Furquan is right here. That bit does not control shutting down but if XTAL is qualified (=is allowed) to be kept running. XTAL gets disabled via asl code / the OS, not the PMC. > 8254 clock gating is required for XTAL shutdown > XTAL shutdown is necessary for S0ix unless disqualification bit is set. Exactly. That's what I understood, too. -- To view, visit https://review.coreboot.org/c/coreboot/+/45723 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I17bac9b06e5291b1548704744e872b22b2435c9c Gerrit-Change-Number: 45723 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Naresh Solanki <naresh.solanki(a)intel.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 26 Sep 2020 22:21:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib/Makefile.inc: fix name of config string
by Paul Fagerburg (Code Review) 27 Sep '20

27 Sep '20

Paul Fagerburg has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45739 ) Change subject: lib/Makefile.inc: fix name of config string ...................................................................... lib/Makefile.inc: fix name of config string The config string is HAVE_SPD_IN_CBFS, without the "BIN". Signed-off-by: Paul Fagerburg <pfagerburg(a)google.com> Change-Id: I728f64b2dd93b0e3947983b9b3701e185feff571 --- M src/lib/Makefile.inc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/45739/1 diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index 73077f7..ce57f51 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -358,7 +358,7 @@ # Include spd ROM data $(LIB_SPD_BIN): $(LIB_SPD_DEPS) test -n "$(SPD_SOURCES)" || \ - (echo "HAVE_SPD_BIN_IN_CBFS is set but SPD_SOURCES is empty" && exit 1) + (echo "HAVE_SPD_IN_CBFS is set but SPD_SOURCES is empty" && exit 1) test -n "$(LIB_SPD_DEPS)" || \ (echo "SPD_SOURCES is set but no SPD file was found" && exit 1) for f in $(LIB_SPD_DEPS); \ -- To view, visit https://review.coreboot.org/c/coreboot/+/45739 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I728f64b2dd93b0e3947983b9b3701e185feff571 Gerrit-Change-Number: 45739 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-MessageType: newchange

3 3

Change in coreboot[master]: soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL in s0ix conditional
by Furquan Shaikh (Code Review) 27 Sep '20

27 Sep '20

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45723 ) Change subject: soc/intel/{skl,cnl,icl,jsl,elh,tgl}: make XTAL in s0ix conditional ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/coreboot/+/45723/2/src/soc/intel/cannonlake/c… File src/soc/intel/cannonlake/chip.h: https://review.coreboot.org/c/coreboot/+/45723/2/src/soc/intel/cannonlake/c… PS2, Line 61: uint8_t s0ix_allow_xtal_on; > I'm afraid I don't follow. Intel documents both the XTAL shutdown and the SLP_S0# assertion (what is what we want to achieve with S0ix, AIUI) as effects of low-power state 3 (LP3). Not as requirements of one another. This is my understanding: On C10 entry, PMC checks clock requests for XTAL to determine whether it is okay to enter S0i3 (LP3) i.e. XTAL is one of the qualification factors for S0i3 entry. However, it can be ignored by setting the XTAL disqualification bit. Thus, even if XTAL remains enabled on C10 entry, PMC ignores its state when making the determination for S0i3 entry. > Where is their relation documented? IIRC, we can't have S0ix with that timer anyway. I never found actual documentation on this. Mostly during discussions with Intel. There was a patch series at one point that Intel had: https://review.coreboot.org/c/coreboot/+/22299 which basically indicated that: 8254 clock gating is required for XTAL shutdown XTAL shutdown is necessary for S0ix unless disqualification bit is set. -- To view, visit https://review.coreboot.org/c/coreboot/+/45723 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I17bac9b06e5291b1548704744e872b22b2435c9c Gerrit-Change-Number: 45723 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Naresh Solanki <naresh.solanki(a)intel.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Sat, 26 Sep 2020 19:03:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2020