coreboot-gerrit March 2020

coreboot-gerrit@coreboot.org

1 participants
3326 discussions

Change in coreboot[master]: Revert "crossgcc: Upgrade Expat to version 2.2.9"
by Patrick Rudolph (Code Review) 15 Mar '20

15 Mar '20

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39573 ) Change subject: Revert "crossgcc: Upgrade Expat to version 2.2.9" ...................................................................... Revert "crossgcc: Upgrade Expat to version 2.2.9" This reverts commit a2fbddfabc447189e03793196b2ef80f4bde273a. Change-Id: Iad210500e2f23c11139cac40291158635ccaf62b Signed-off-by: Patrick Rudolph <siro(a)das-labor.org> --- M util/crossgcc/buildgcc A util/crossgcc/sum/expat-2.2.7.tar.bz2.cksum D util/crossgcc/sum/expat-2.2.9.tar.bz2.cksum 3 files changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/39573/1 diff --git a/util/crossgcc/buildgcc b/util/crossgcc/buildgcc index 9920799..a74c7c6 100755 --- a/util/crossgcc/buildgcc +++ b/util/crossgcc/buildgcc @@ -53,10 +53,10 @@ GCC_VERSION=9.2.0 GCC_AUTOCONF_VERSION=2.69 BINUTILS_VERSION=2.33.1 -GDB_VERSION=8.3.1 IASL_VERSION=20200110 PYTHON_VERSION=3.8.1 -EXPAT_VERSION=2.2.9 +GDB_VERSION=8.3.1 +EXPAT_VERSION=2.2.7 # CLANG version number CLANG_VERSION=9.0.0 MAKE_VERSION=4.2.1 diff --git a/util/crossgcc/sum/expat-2.2.7.tar.bz2.cksum b/util/crossgcc/sum/expat-2.2.7.tar.bz2.cksum new file mode 100644 index 0000000..77f6272 --- /dev/null +++ b/util/crossgcc/sum/expat-2.2.7.tar.bz2.cksum @@ -0,0 +1 @@ +9c8a268211e3f1ae31c4d550e5be7708973ec6a6 tarballs/expat-2.2.7.tar.bz2 diff --git a/util/crossgcc/sum/expat-2.2.9.tar.bz2.cksum b/util/crossgcc/sum/expat-2.2.9.tar.bz2.cksum deleted file mode 100644 index 924b412..0000000 --- a/util/crossgcc/sum/expat-2.2.9.tar.bz2.cksum +++ /dev/null @@ -1 +0,0 @@ -ef5c1c55913a6ab18496ee99166f86269c7cdc31 tarballs/expat-2.2.9.tar.bz2 -- To view, visit https://review.coreboot.org/c/coreboot/+/39573 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iad210500e2f23c11139cac40291158635ccaf62b Gerrit-Change-Number: 39573 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

3 3

Change in coreboot[master]: nb/intel/i945: Refactor sdram_set_channel_mode()
by HAOUAS Elyes (Code Review) 15 Mar '20

15 Mar '20

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39537 ) Change subject: nb/intel/i945: Refactor sdram_set_channel_mode() ...................................................................... nb/intel/i945: Refactor sdram_set_channel_mode() Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/northbridge/intel/i945/raminit.c 1 file changed, 10 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/39537/1 diff --git a/src/northbridge/intel/i945/raminit.c b/src/northbridge/intel/i945/raminit.c index dd7030e..972e71f 100644 --- a/src/northbridge/intel/i945/raminit.c +++ b/src/northbridge/intel/i945/raminit.c @@ -1572,21 +1572,19 @@ reg32 = MCHBAR32(DCC); reg32 &= ~(7 << 0); - if (sysinfo->interleaved) { - /* Dual Channel Interleaved */ - printk(BIOS_DEBUG, "Dual Channel Interleaved.\n"); - reg32 |= (1 << 1); - } else if (sysinfo->dimm[0] == SYSINFO_DIMM_NOT_POPULATED && - sysinfo->dimm[1] == SYSINFO_DIMM_NOT_POPULATED) { + if (sdram_capabilities_dual_channel() { + if (sysinfo->interleaved) { + printk(BIOS_DEBUG, "Dual Channel Interleaved.\n"); + reg32 |= (1 << 1); + } else { + /* Dual Channel Asymmetric */ + printk(BIOS_DEBUG, "Dual Channel Asymmetric.\n"); + reg32 |= (1 << 0); + } + } else if (sysinfo->dimm[0] == SYSINFO_DIMM_NOT_POPULATED { /* Channel 1 only */ printk(BIOS_DEBUG, "Single Channel 1 only.\n"); reg32 |= (1 << 2); - } else if (sdram_capabilities_dual_channel() && - (sysinfo->dimm[2] != SYSINFO_DIMM_NOT_POPULATED || - sysinfo->dimm[3] != SYSINFO_DIMM_NOT_POPULATED)) { - /* Dual Channel Asymmetric */ - printk(BIOS_DEBUG, "Dual Channel Asymmetric.\n"); - reg32 |= (1 << 0); } else { /* All bits 0 means Single Channel 0 operation */ printk(BIOS_DEBUG, "Single Channel 0 only.\n"); -- To view, visit https://review.coreboot.org/c/coreboot/+/39537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7574bc4a1dbb55d12dec9be17e403a2f448c16c0 Gerrit-Change-Number: 39537 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

2 10

Change in coreboot[master]: nb/intel/i945: Program CxODT value for each channel
by HAOUAS Elyes (Code Review) 15 Mar '20

15 Mar '20

Hello build bot (Jenkins), Nico Huber, Paul Menzel, Angel Pons, Arthur Heymans, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/18548 to look at the new patch set (#35). Change subject: nb/intel/i945: Program CxODT value for each channel ...................................................................... nb/intel/i945: Program CxODT value for each channel There is no reason to program both C0ODT and C1ODT with the same value when channel 0 and 1 are not equally populated. Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/northbridge/intel/i945/raminit.c 1 file changed, 14 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/18548/35 -- To view, visit https://review.coreboot.org/c/coreboot/+/18548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c Gerrit-Change-Number: 18548 Gerrit-PatchSet: 35 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: nb/intel/i945: Program CxODT value for each channel
by HAOUAS Elyes (Code Review) 15 Mar '20

15 Mar '20

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18548 ) Change subject: nb/intel/i945: Program CxODT value for each channel ...................................................................... Patch Set 34: (4 comments) https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94… File src/northbridge/intel/i945/raminit.c: https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94… PS34, Line 2435: u8 populated_dimms = 0; > As this is only used in two cases, in my opinion the explicit use of `sysinfo-dimm[i]` in each if co […] Ack https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94… PS34, Line 2437: populated_dimms : > Remove the space before the colon. Ack https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94… PS34, Line 2450: one > Although mathematicians know, that implicitly *at least* is assumed, make it clear, that at least on […] here we are not looking for *at least* but *only one* http://nowni.tistory.com/attachment/4900606d18872AU.pdf - Case Single module per channel -> 150 ohms (can be 75ohms if DDR2-400/533MHz, or 50ohms for 667/800MHz ?) - Case 2 memory modules per channel --> 75 ohms if DDR2-400/533MHz 50 ohms if DDR2-667/800MHz (populated_dimms & 3) == 1 != (populated_dimms & 3) == 0 != (populated_dimms & 3) == 2) ... https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94… PS34, Line 2457: populated_dimms = populated_dimms >> 2; > I wouldn’t rewrite the variable here, and change it’s meaning. […] Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/18548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c Gerrit-Change-Number: 18548 Gerrit-PatchSet: 34 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Mon, 16 Mar 2020 05:49:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/intel/jasperlake_rvp: add JSLRVP DDR4 variant
by Aamir Bohra (Code Review) 15 Mar '20

15 Mar '20

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39400 ) Change subject: mb/intel/jasperlake_rvp: add JSLRVP DDR4 variant ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/c/coreboot/+/39400/1/src/mainboard/intel/jasper… File src/mainboard/intel/jasperlake_rvp/Kconfig: https://review.coreboot.org/c/coreboot/+/39400/1/src/mainboard/intel/jasper… PS1, Line 1: BOARD_INTEL_JASPERLAKE_RVP_DDR4 can we do it based on board_id read? would save an extra variant. -- To view, visit https://review.coreboot.org/c/coreboot/+/39400 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id96d25fc13a4e6e2b6a58615e7ef58dd3c118089 Gerrit-Change-Number: 39400 Gerrit-PatchSet: 1 Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 16 Mar 2020 03:34:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake:add support to read spd data from SMBUS
by Aamir Bohra (Code Review) 15 Mar '20

15 Mar '20

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39401 ) Change subject: soc/intel/tigerlake:add support to read spd data from SMBUS ...................................................................... Patch Set 1: (3 comments) https://review.coreboot.org/c/coreboot/+/39401/1/src/soc/intel/tigerlake/me… File src/soc/intel/tigerlake/meminit_jsl.c: https://review.coreboot.org/c/coreboot/+/39401/1/src/soc/intel/tigerlake/me… PS1, Line 50: memset(&mem_cfg->SpdAddressTable, 0, sizeof(mem_cfg->SpdAddressTable)); two instances not needed, we can keep it as original place, the sbmus would re-initialize it anyway. https://review.coreboot.org/c/coreboot/+/39401/1/src/soc/intel/tigerlake/me… PS1, Line 120: spd_data_ptr this is not getting initialized now, meminit_channels would set the invalid values for MemorySpdPtrXX for non SMBUS case. https://review.coreboot.org/c/coreboot/+/39401/1/src/soc/intel/tigerlake/me… PS1, Line 122: get_spd_data We can handle the SMBUS case here instead of changing the func to accommodate SMBUS, get_spd_data, should be limited to non SMBUS case, since for SMBUS it is read dynamically: something like this: if (spd_info->read_type != READ_SPD_CBFS) { get_spd_data(spd_info, &spd_data_ptr, &spd_data_len); print_spd_info((unsigned char *)spd_data_ptr); mem_cfg->MemorySpdDataLen = spd_data_len; } else { for (int i = 0; i < NUM_DIMM_SLOT; i++) mem_cfg->SpdAddressTable[i]= spd_info->spd_spec.spd_smbus_address[i]; spd_data_ptr = 0; } meminit_channels(mem_cfg, board_cfg, spd_data_ptr, half_populated); -- To view, visit https://review.coreboot.org/c/coreboot/+/39401 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I94f8707c731c8afa1106e387a246c000bd53a654 Gerrit-Change-Number: 39401 Gerrit-PatchSet: 1 Gerrit-Owner: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Comment-Date: Mon, 16 Mar 2020 03:33:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mainboard/google/hatch: Add Palkia support
by Kane Chen (Code Review) 15 Mar '20

15 Mar '20

Kane Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39096 ) Change subject: mainboard/google/hatch: Add Palkia support ...................................................................... mainboard/google/hatch: Add Palkia support Add Palkia as a variant of Hatch. BUG=b:none BRANCH=None TEST=util/abuild/abuild -p none -t google/hatch -x -a attempts to build a Helios target. Signed-off-by: Kane Chen <kane_chen(a)pegatron.corp-partner.google.com> Change-Id: Ifeb1e2808f3779a1a1c4da9b71b535b02fb4e1e0 --- M src/mainboard/google/hatch/Kconfig M src/mainboard/google/hatch/Kconfig.name 2 files changed, 9 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/96/39096/1 diff --git a/src/mainboard/google/hatch/Kconfig b/src/mainboard/google/hatch/Kconfig index 783ec73..3add7ab 100644 --- a/src/mainboard/google/hatch/Kconfig +++ b/src/mainboard/google/hatch/Kconfig @@ -98,6 +98,7 @@ default "Kindred" if BOARD_GOOGLE_KINDRED default "Kohaku" if BOARD_GOOGLE_KOHAKU default "Mushu" if BOARD_GOOGLE_MUSHU + default "Palkia" if BOARD_GOOGLE_PALKIA default "Puff" if BOARD_GOOGLE_PUFF default "Stryke" if BOARD_GOOGLE_STRYKE @@ -121,6 +122,7 @@ default "kindred" if BOARD_GOOGLE_KINDRED default "kohaku" if BOARD_GOOGLE_KOHAKU default "mushu" if BOARD_GOOGLE_MUSHU + default "palkia" if BOARD_GOOGLE_PALKIA default "puff" if BOARD_GOOGLE_PUFF default "stryke" if BOARD_GOOGLE_STRYKE diff --git a/src/mainboard/google/hatch/Kconfig.name b/src/mainboard/google/hatch/Kconfig.name index e216135..c2dd7ca 100644 --- a/src/mainboard/google/hatch/Kconfig.name +++ b/src/mainboard/google/hatch/Kconfig.name @@ -60,3 +60,10 @@ bool "-> Stryke" select BOARD_GOOGLE_BASEBOARD_HATCH select BOARD_ROMSIZE_KB_16384 + +config BOARD_GOOGLE_PALKIA + bool "-> Palkia" + select BOARD_GOOGLE_BASEBOARD_HATCH + select BOARD_ROMSIZE_KB_16384 + select CHROMEOS_DSM_CALIB + select DRIVERS_I2C_RT1011 -- To view, visit https://review.coreboot.org/c/coreboot/+/39096 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifeb1e2808f3779a1a1c4da9b71b535b02fb4e1e0 Gerrit-Change-Number: 39096 Gerrit-PatchSet: 1 Gerrit-Owner: Kane Chen <kane_chen(a)pegatron.corp-partner.google.com> Gerrit-MessageType: newchange

8 17

Change in coreboot[master]: nb/intel/i945: Program CxODT value for each channel
by Paul Menzel (Code Review) 15 Mar '20

15 Mar '20

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18548 ) Change subject: nb/intel/i945: Program CxODT value for each channel ...................................................................... Patch Set 34: (4 comments) https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94… File src/northbridge/intel/i945/raminit.c: https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94… PS34, Line 2435: u8 populated_dimms = 0; As this is only used in two cases, in my opinion the explicit use of `sysinfo-dimm[i]` in each if condition is still justified. https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94… PS34, Line 2437: populated_dimms : Remove the space before the colon. https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94… PS34, Line 2450: one Although mathematicians know, that implicitly *at least* is assumed, make it clear, that at least one DIMM is there. > C00DT: Channel 0 has at least one DIMM. or change the condition to (populated_dimms & 3) == 1 != (populated_dimms & 3) == 2) Maybe elaborate: C0ODT: Channel 0 has %i DIMM(s).\n https://review.coreboot.org/c/coreboot/+/18548/34/src/northbridge/intel/i94… PS34, Line 2457: populated_dimms = populated_dimms >> 2; I wouldn’t rewrite the variable here, and change it’s meaning. Just change the mask in the if condition. -- To view, visit https://review.coreboot.org/c/coreboot/+/18548 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7aec35f45250da554ddc5a68f5add157c313399c Gerrit-Change-Number: 18548 Gerrit-PatchSet: 34 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Sun, 15 Mar 2020 21:46:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/skylake: Configure ASPM and L1 substates for PCH root ports
by Angel Pons (Code Review) 15 Mar '20

15 Mar '20

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39538 ) Change subject: soc/intel/skylake: Configure ASPM and L1 substates for PCH root ports ...................................................................... Patch Set 6: (1 comment) https://review.coreboot.org/c/coreboot/+/39538/6/src/soc/intel/skylake/chip… File src/soc/intel/skylake/chip.c: https://review.coreboot.org/c/coreboot/+/39538/6/src/soc/intel/skylake/chip… PS6, Line 221: PcieRpAspm > This disabled ASPM on both my root ports. […] Yes, you probably want something like the L1 substate thing -- To view, visit https://review.coreboot.org/c/coreboot/+/39538 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I36150858485715016158595c832c142b0582ddb8 Gerrit-Change-Number: 39538 Gerrit-PatchSet: 6 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Sun, 15 Mar 2020 21:42:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/skylake: Configure ASPM and L1 substates for PCH root ports
by Paul Menzel (Code Review) 15 Mar '20

15 Mar '20

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39538 ) Change subject: soc/intel/skylake: Configure ASPM and L1 substates for PCH root ports ...................................................................... Patch Set 6: (5 comments) https://review.coreboot.org/c/coreboot/+/39538/6//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/39538/6//COMMIT_MSG@9 PS6, Line 9: Port of CB:39412 for Skylake. Port commit 84b4882b (soc/intel/tigerlake: Configure L1Substates for PCH Root ports), CB:39412) to Skylake. https://review.coreboot.org/c/coreboot/+/39538/6//COMMIT_MSG@10 PS6, Line 10: to devicetree to allow boards to set these options Please add a dot/period at the end of sentences please. https://review.coreboot.org/c/coreboot/+/39538/6//COMMIT_MSG@12 PS6, Line 12: Chip config parameter PcieRpL1Substates uses (UPD value + 1) : because UPD value of 0 for PcieRpL1Substates means disabled for FSP. : In order to ensure that mainboard setting does not disable L1 substates : incorrectly, chip config parameter values are offset by 1 with 0 meaning : use FSP UPD default. Please re-flow for 72 characters. https://review.coreboot.org/c/coreboot/+/39538/6/src/soc/intel/skylake/chip… File src/soc/intel/skylake/chip.c: https://review.coreboot.org/c/coreboot/+/39538/6/src/soc/intel/skylake/chip… PS6, Line 135: * Chip config parameter PcieRpL1Substates uses (UPD value + 1) : * because UPD value of 0 for PcieRpL1Substates means disabled for FSP. : * In order to ensure that mainboard setting does not disable L1 substates : * incorrectly, chip config parameter values are offset by 1 with 0 meaning : * use FSP UPD default. get_l1_substate_control() ensures that the right UPD : * value is set in fsp_params. Please re-flow for 96 characters. https://review.coreboot.org/c/coreboot/+/39538/6/src/soc/intel/skylake/chip… PS6, Line 224: get_l1_substate_control(config->PcieRpL1Substates[i]); Why not combine both loops? -- To view, visit https://review.coreboot.org/c/coreboot/+/39538 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I36150858485715016158595c832c142b0582ddb8 Gerrit-Change-Number: 39538 Gerrit-PatchSet: 6 Gerrit-Owner: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Sun, 15 Mar 2020 21:30:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2020