coreboot-gerrit April 2019

coreboot-gerrit@coreboot.org

1 participants
1219 discussions

Change in ...coreboot[master]: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode
by Frans Hendriks (Code Review) 29 Apr '19

29 Apr '19

1 0

Change in ...coreboot[master]: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode
by Michał Żygowski (Code Review) 29 Apr '19

29 Apr '19

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29284 ) Change subject: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode ...................................................................... Patch Set 8: > If LPSS (SIO DMA1) pci device 0/30/0 is configured in ACPI mode, PWM, HS2UARTS, and SPI controller needs to be configured in ACPI mode. > If LPSS (SIO DMA2) pci device 0/24/0 is configured in ACPI mode, I2C controller needs to be configured in ACPI mode. > In other words: If function > 1 in PCI mode, function 0 must be PCI mode to be PCI-enumerated. > (See 33.1 of Intel Braswell BIOS Writes Guide) > (See 24.3 of Intel Braswell External Design Guide Volume 2) > > This is NOT a bug in FSP. I expect it's responsibility of coreboot/developer to provide correct parameters API of FSP. IMO it is still a BUG in FSP docs. The FSP docs do not document ACPI mode for DMAs and the dependency for child devices following the parent device setting. How a developer can integrate the binary correctly when the documentation and API have bugs? Not everyone has access to FSP source code, let's keep that in mind. -- To view, visit https://review.coreboot.org/c/coreboot/+/29284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie271d8cb9f30f0c0ba538f1530cfb82f1306fea8 Gerrit-Change-Number: 29284 Gerrit-PatchSet: 8 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Comment-Date: Mon, 29 Apr 2019 07:56:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...libhwbase[master]: mutime: Move to common/
by Reto Buerki (Code Review) 29 Apr '19

29 Apr '19

Reto Buerki has posted comments on this change. ( https://review.coreboot.org/c/libhwbase/+/26839 ) Change subject: mutime: Move to common/ ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/libhwbase/+/26839 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: libhwbase Gerrit-Branch: master Gerrit-Change-Id: I03bc3fa7d9bea79680888c13679cfd871e772000 Gerrit-Change-Number: 26839 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Adrian-Ken Rueegsegger <ken(a)codelabs.ch> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Reto Buerki <reet(a)codelabs.ch> Gerrit-Comment-Date: Mon, 29 Apr 2019 07:49:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...libhwbase[master]: time: Revise state abstraction
by Reto Buerki (Code Review) 29 Apr '19

29 Apr '19

Reto Buerki has posted comments on this change. ( https://review.coreboot.org/c/libhwbase/+/26838 ) Change subject: time: Revise state abstraction ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/libhwbase/+/26838 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: libhwbase Gerrit-Branch: master Gerrit-Change-Id: I35af2d0db1acbf9652ea00763aa288545746bb79 Gerrit-Change-Number: 26838 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Adrian-Ken Rueegsegger <ken(a)codelabs.ch> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Reto Buerki <reet(a)codelabs.ch> Gerrit-Comment-Date: Mon, 29 Apr 2019 07:46:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode
by Frans Hendriks (Code Review) 29 Apr '19

29 Apr '19

Hello Patrick Rudolph, Aaron Durbin, Piotr Król, Paul Menzel, build bot (Jenkins), Hannah Williams, Nico Huber, Michał Żygowski, Patrick Georgi, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/29284 to look at the new patch set (#9). Change subject: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode ...................................................................... soc/intel/braswell/chip.c: Configure LPSS devices in correct mode LPSS (SIO 0/24/0 and 0/30/0) are configured to ACPI mode if 'lpss_acpi_mode' = '1' or PcdEnableDma0/PcdEnableDma1 is configured in ACPI mode. When function 0 is configured in ACPI mode, functions > 1 are not available on the PCI bus, even pcdEnableXXXXX is set to PCI mode. For SIO DMA0: - HSUARTS ports not visible on PCI bus. For SIO DMA1: - I2C ports not visible on PCI bus. Force 'function > 1' devices in ACPI mode when function 0 is configured in ACPI mode. BUG=N/A TEST=Intel Cherry Hill Change-Id: Ie271d8cb9f30f0c0ba538f1530cfb82f1306fea8 Signed-off-by: Frans Hendriks <fhendriks(a)eltan.com> --- M src/soc/intel/braswell/chip.c 1 file changed, 23 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/29284/9 -- To view, visit https://review.coreboot.org/c/coreboot/+/29284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie271d8cb9f30f0c0ba538f1530cfb82f1306fea8 Gerrit-Change-Number: 29284 Gerrit-PatchSet: 9 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in ...libhwbase[master]: Makefile.proof: Remove cvc4 from prover list
by Reto Buerki (Code Review) 29 Apr '19

29 Apr '19

Reto Buerki has posted comments on this change. ( https://review.coreboot.org/c/libhwbase/+/26837 ) Change subject: Makefile.proof: Remove cvc4 from prover list ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/libhwbase/+/26837 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: libhwbase Gerrit-Branch: master Gerrit-Change-Id: Ib0458bfe53fd999c60f8d1b7cf8709e6195d0a42 Gerrit-Change-Number: 26837 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Adrian-Ken Rueegsegger <ken(a)codelabs.ch> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Reto Buerki <reet(a)codelabs.ch> Gerrit-Comment-Date: Mon, 29 Apr 2019 07:40:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/intel/braswell: Move LPE ACPI code to mainboard
by Michał Żygowski (Code Review) 29 Apr '19

29 Apr '19

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29414 ) Change subject: soc/intel/braswell: Move LPE ACPI code to mainboard ...................................................................... Patch Set 12: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/29414 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic8acf9ea9e9b0ba9b272e20beb2023b7a4716a73 Gerrit-Change-Number: 29414 Gerrit-PatchSet: 12 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Mon, 29 Apr 2019 07:37:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode
by Frans Hendriks (Code Review) 29 Apr '19

29 Apr '19

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/29284 ) Change subject: soc/intel/braswell/chip.c: Configure LPSS devices in correct mode ...................................................................... Patch Set 8: > Patch Set 8: > > > > > FSP MR2 does a check for ACPI mode and change the device mode. Supplying incorrect configuration will be corrected by FSP, but to be futher proof supply correct configuration to FSP. > > > > > > I can't say that I understand this yet. You are saying FSP > > > allows a value of 2 (PCH_ACPI_MODE). But then you say FSP > > > would be correcting incorrect configuration, which translates > > > to me as it reads the hardware state and ignores the diffe- > > > rence between a 1 and a 2. > > > > > > And about being future proof. That's generally a good idea, > > > but how can we anticipate the future here? How can we tell > > > that if FSP is being corrected, if the code will be made to > > > match the header file or the other way around? > > > > The goal is to supply correct mode settings to FSP. For this reason the modification is made. > > On the other hand FSP (MR2) will modify settings of the child devices if incorrect. > > What FSP is doing is not wrong, but for those without access to FSP sources, we should ensure that coreboot is configuring the correct mode. > > Ah, there's that about child devices again. Sometimes you say > it's corrected (see topmost quote here) and the change is only > needed to be future proof, sometimes you say it's corrected for > child devices (only?). That's confusing. > > But let's back up a little. How does the device hierarchy look > like here? I couldn't find anything about child devices and > actually nothing about the DMA devices in general. Can you > point to a datasheet that explains this a little? > > I think the first thing to do in such a case (documentation / > comments in header files don't match the code) is to file a bug > report for FSP. Was that done yet? And in case we can't get > the official documentation fixed, we have to document things > on our own, e.g. by adding a reasonable enum to our code and > comment that the documentation is wrong. If LPSS (SIO DMA1) pci device 0/30/0 is configured in ACPI mode, PWM, HS2UARTS, and SPI controller needs to be configured in ACPI mode. If LPSS (SIO DMA2) pci device 0/24/0 is configured in ACPI mode, I2C controller needs to be configured in ACPI mode. In other words: If function > 1 in PCI mode, function 0 must be PCI mode to be PCI-enumerated. (See 33.1 of Intel Braswell BIOS Writes Guide) (See 24.3 of Intel Braswell External Design Guide Volume 2) This is NOT a bug in FSP. I expect it's responsibility of coreboot/developer to provide correct parameters API of FSP. -- To view, visit https://review.coreboot.org/c/coreboot/+/29284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie271d8cb9f30f0c0ba538f1530cfb82f1306fea8 Gerrit-Change-Number: 29284 Gerrit-PatchSet: 8 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Comment-Date: Mon, 29 Apr 2019 07:30:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: mb/google/hatch/variants/baseboard: remove unused dqs_map
by Paul Fagerburg (Code Review) 29 Apr '19

29 Apr '19

Paul Fagerburg has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32484 Change subject: mb/google/hatch/variants/baseboard: remove unused dqs_map ...................................................................... mb/google/hatch/variants/baseboard: remove unused dqs_map The dqs_map array is used only for LPDDR3 and LPDDR4. It is not used for DDR4, and so it can be removed from the baseboard memory initialization code. BRANCH=none BUG=b:129706819 TEST=ensure the firmware builds without error; I don't have hardware available to test this just yet. Change-Id: I07fac3097d68f37b4630d3f0010f987da2f03bd7 Signed-off-by: Paul Fagerburg <pfagerburg(a)chromium.org> --- M src/mainboard/google/hatch/variants/baseboard/memory.c 1 file changed, 0 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/32484/1 diff --git a/src/mainboard/google/hatch/variants/baseboard/memory.c b/src/mainboard/google/hatch/variants/baseboard/memory.c index b6a6615..580bdc9 100644 --- a/src/mainboard/google/hatch/variants/baseboard/memory.c +++ b/src/mainboard/google/hatch/variants/baseboard/memory.c @@ -20,16 +20,6 @@ #include <string.h> static const struct cnl_mb_cfg baseboard_memcfg = { - /* - * The dqs_map arrays map the ddr4 pins to the SoC pins - * for both channels. - * - * the index = pin number on ddr4 part - * the value = pin number on SoC - */ - .dqs_map[DDR_CH0] = { 0, 1, 4, 5, 2, 3, 6, 7 }, - .dqs_map[DDR_CH1] = { 0, 1, 4, 5, 2, 3, 6, 7 }, - /* Baseboard uses 121, 81 and 100 rcomp resistors */ .rcomp_resistor = { 121, 81, 100 }, -- To view, visit https://review.coreboot.org/c/coreboot/+/32484 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I07fac3097d68f37b4630d3f0010f987da2f03bd7 Gerrit-Change-Number: 32484 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-MessageType: newchange

3 3

Change in ...coreboot[master]: soc/intel/cannonlake: Modify dq_map to provide for 6 entries
by Paul Fagerburg (Code Review) 29 Apr '19

29 Apr '19

Paul Fagerburg has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32475 Change subject: soc/intel/cannonlake: Modify dq_map to provide for 6 entries ...................................................................... soc/intel/cannonlake: Modify dq_map to provide for 6 entries Intel's DQ_DQS_RComp_Info_Utility generates data for 6 entries. MRC will return errors if we don't have all 6 entries in the map. BRANCH=none BUG=b:131103736 TEST=ensure the firmware builds without error; I don't have hardware available to test this just yet. Change-Id: I20a768de0e4440d7dde7b717794c4e2d0c62819c Signed-off-by: Paul Fagerburg <pfagerburg(a)chromium.org> --- M src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h 1 file changed, 8 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/32475/1 diff --git a/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h b/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h index c602180..87cb6fb 100644 --- a/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h +++ b/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h @@ -52,17 +52,19 @@ /* Board-specific memory dq mapping information */ struct cnl_mb_cfg { /* - * For each channel, there are 3 sets of DQ byte mappings, + * For each channel, there are 6 sets of DQ byte mappings, * where each set has a package 0 and a package 1 value (package 0 * represents the first 64-bit lpddr4 chip combination, and package 1 * represents the second 64-bit lpddr4 chip combination). * The first three sets are for CLK, CMD, and CTL. - * The fsp package actually expects 6 sets, but the last 3 sets are - * not used in CNL, so we only define the three sets that are used - * and let the meminit_lpddr4() routine take care of clearing the + * The fsp package actually expects 6 sets, even though the last 3 sets + * are not used in CNL. + * We let the meminit_lpddr4() routine take care of clearing the * unused fields for the caller. + * Note that dq_map is only used by LPDDR; it does not need to be + * initialized for designs using DDR4. */ - uint8_t dq_map[DDR_NUM_CHANNELS][3][DDR_NUM_PACKAGES]; + uint8_t dq_map[DDR_NUM_CHANNELS][6][DDR_NUM_PACKAGES]; /* * DQS CPU<>DRAM map Ch0 and Ch1. Each array entry represents a @@ -70,6 +72,7 @@ * the memory part. The array index represents the dqs bit number * on the memory part, and the values in the array represent which * pin on the CPU that DRAM pin connects to. + * dqs_map is only used by LPDDR; same comments apply as for dq_map above. */ uint8_t dqs_map[DDR_NUM_CHANNELS][DQ_BITS_PER_DQS]; -- To view, visit https://review.coreboot.org/c/coreboot/+/32475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I20a768de0e4440d7dde7b717794c4e2d0c62819c Gerrit-Change-Number: 32475 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-MessageType: newchange

4 7

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2019