coreboot-gerrit July 2020

coreboot-gerrit@coreboot.org

1 participants
3166 discussions

Change in coreboot[master]: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F
by Jonas Löffelholz (Code Review) 14 Jul '20

14 Jul '20

Jonas Löffelholz has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37441 ) Change subject: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F ...................................................................... Patch Set 62: (2 comments) https://review.coreboot.org/c/coreboot/+/37441/60/src/mainboard/supermicro/… File src/mainboard/supermicro/x11-lga1151v2-series/memory.c: https://review.coreboot.org/c/coreboot/+/37441/60/src/mainboard/supermicro/… PS60, Line 27: .rcomp_resistor = {121, 81, 100}, > I usually dig the resistor values from schematics for ULT and Halo, and then ask Nico if they are co […] Done https://review.coreboot.org/c/coreboot/+/37441/60/src/mainboard/supermicro/… PS60, Line 30: .rcomp_targets = {100, 40, 20, 20, 26}, > Same document, and I can confirm Angel's values. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/37441 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ab1cb9462607b9af068bc2374508d99c60d0a30 Gerrit-Change-Number: 37441 Gerrit-PatchSet: 62 Gerrit-Owner: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Guido Beyer @ Prodrive Technologies <guido.beyer(a)prodrive-technologies.com> Gerrit-Reviewer: Justin van Son <justin.van.son(a)prodrive-technologies.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Stef van Os <stef.van.os(a)prodrive-technologies.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: wouter.eckhardt(a)prodrive-technologies.com Gerrit-CC: Jonas Löffelholz Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 14 Jul 2020 08:46:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Christian Walter <christian.walter(a)9elements.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F
by Jonas Löffelholz (Code Review) 14 Jul '20

14 Jul '20

Jonas Löffelholz has uploaded a new patch set (#62) to the change originally created by Christian Walter. ( https://review.coreboot.org/c/coreboot/+/37441 ) Change subject: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F ...................................................................... mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F This commit adds initial support for the Supermicro X11SCH-F. What is working: * Aspeed AST2500 graphics output * Serial console * SuperIO devicetree config * Boots into Linux 5.5 * Tested payload EDK II * SATA slots * USB slots * IPMI KCS * LinuxBoot * No ACPI error in Windows/Linux * Windows 10 support * TPM Support (w/ patched IFD) - Patch IFD at 0x235=0x03 and 0x13E=0x84 Tested with Intel Xeon E-2186G and 64 GB ECC RAM. Change-Id: I0ab1cb9462607b9af068bc2374508d99c60d0a30 Signed-off-by: Christian Walter <christian.walter(a)9elements.com> --- M Documentation/mainboard/index.md A Documentation/mainboard/supermicro/x11-lga1151v2-series/x11-lga1151v2-series.md A Documentation/mainboard/supermicro/x11-lga1151v2-series/x11sch-f/x11sch-f.md A Documentation/mainboard/supermicro/x11-lga1151v2-series/x11sch-f/x11sch-f_flash.jpg M MAINTAINERS A src/mainboard/supermicro/x11-lga1151v2-series/Kconfig A src/mainboard/supermicro/x11-lga1151v2-series/Kconfig.name A src/mainboard/supermicro/x11-lga1151v2-series/Makefile.inc A src/mainboard/supermicro/x11-lga1151v2-series/board_info.txt A src/mainboard/supermicro/x11-lga1151v2-series/bootblock.c A src/mainboard/supermicro/x11-lga1151v2-series/devicetree.cb A src/mainboard/supermicro/x11-lga1151v2-series/dsdt.asl A src/mainboard/supermicro/x11-lga1151v2-series/memory.c A src/mainboard/supermicro/x11-lga1151v2-series/ramstage.c A src/mainboard/supermicro/x11-lga1151v2-series/romstage.c A src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/Makefile.inc A src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/board_info.txt A src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/gpio.c A src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/include/variant/gpio.h A src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/include/variant/variants.h A src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/overridetree.cb 21 files changed, 1,001 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/37441/62 -- To view, visit https://review.coreboot.org/c/coreboot/+/37441 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ab1cb9462607b9af068bc2374508d99c60d0a30 Gerrit-Change-Number: 37441 Gerrit-PatchSet: 62 Gerrit-Owner: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Guido Beyer @ Prodrive Technologies <guido.beyer(a)prodrive-technologies.com> Gerrit-Reviewer: Justin van Son <justin.van.son(a)prodrive-technologies.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Stef van Os <stef.van.os(a)prodrive-technologies.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: wouter.eckhardt(a)prodrive-technologies.com Gerrit-CC: Jonas Löffelholz Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F
by Jonas Löffelholz (Code Review) 14 Jul '20

14 Jul '20

Jonas Löffelholz has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37441 ) Change subject: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F ...................................................................... Patch Set 61: (3 comments) https://review.coreboot.org/c/coreboot/+/37441/60/MAINTAINERS File MAINTAINERS: https://review.coreboot.org/c/coreboot/+/37441/60/MAINTAINERS@384 PS60, Line 384: > The maintainers entry should go over here, with the other supermicro boards Done https://review.coreboot.org/c/coreboot/+/37441/60/src/mainboard/supermicro/… File src/mainboard/supermicro/x11-lga1151v2-series/dsdt.asl: https://review.coreboot.org/c/coreboot/+/37441/60/src/mainboard/supermicro/… PS60, Line 26: > Drop this blank line? Done https://review.coreboot.org/c/coreboot/+/37441/60/src/mainboard/supermicro/… File src/mainboard/supermicro/x11-lga1151v2-series/ramstage.c: https://review.coreboot.org/c/coreboot/+/37441/60/src/mainboard/supermicro/… PS60, Line 17: /* This must be one, otherwise FSP crashes ... */ > Drop this comment Done -- To view, visit https://review.coreboot.org/c/coreboot/+/37441 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ab1cb9462607b9af068bc2374508d99c60d0a30 Gerrit-Change-Number: 37441 Gerrit-PatchSet: 61 Gerrit-Owner: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Guido Beyer @ Prodrive Technologies <guido.beyer(a)prodrive-technologies.com> Gerrit-Reviewer: Justin van Son <justin.van.son(a)prodrive-technologies.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Stef van Os <stef.van.os(a)prodrive-technologies.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: wouter.eckhardt(a)prodrive-technologies.com Gerrit-CC: Jonas Löffelholz Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 14 Jul 2020 08:38:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F
by Jonas Löffelholz (Code Review) 14 Jul '20

14 Jul '20

Jonas Löffelholz has uploaded a new patch set (#61) to the change originally created by Christian Walter. ( https://review.coreboot.org/c/coreboot/+/37441 ) Change subject: mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F ...................................................................... mb/supermicro/x11-lga1151v2-series: Add support for X11SCH-F This commit adds initial support for the Supermicro X11SCH-F. What is working: * Aspeed AST2500 graphics output * Serial console * SuperIO devicetree config * Boots into Linux 5.5 * Tested payload EDK II * SATA slots * USB slots * IPMI KCS * LinuxBoot * No ACPI error in Windows/Linux * Windows 10 support * TPM Support (w/ patched IFD) - Patch IFD at 0x235=0x03 and 0x13E=0x84 Tested with Intel Xeon E-2186G and 64 GB ECC RAM. Change-Id: I0ab1cb9462607b9af068bc2374508d99c60d0a30 Signed-off-by: Christian Walter <christian.walter(a)9elements.com> --- M Documentation/mainboard/index.md A Documentation/mainboard/supermicro/x11-lga1151v2-series/x11-lga1151v2-series.md A Documentation/mainboard/supermicro/x11-lga1151v2-series/x11sch-f/x11sch-f.md A Documentation/mainboard/supermicro/x11-lga1151v2-series/x11sch-f/x11sch-f_flash.jpg M MAINTAINERS A src/mainboard/supermicro/x11-lga1151v2-series/Kconfig A src/mainboard/supermicro/x11-lga1151v2-series/Kconfig.name A src/mainboard/supermicro/x11-lga1151v2-series/Makefile.inc A src/mainboard/supermicro/x11-lga1151v2-series/board_info.txt A src/mainboard/supermicro/x11-lga1151v2-series/bootblock.c A src/mainboard/supermicro/x11-lga1151v2-series/devicetree.cb A src/mainboard/supermicro/x11-lga1151v2-series/dsdt.asl A src/mainboard/supermicro/x11-lga1151v2-series/memory.c A src/mainboard/supermicro/x11-lga1151v2-series/ramstage.c A src/mainboard/supermicro/x11-lga1151v2-series/romstage.c A src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/Makefile.inc A src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/board_info.txt A src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/gpio.c A src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/include/variant/gpio.h A src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/include/variant/variants.h A src/mainboard/supermicro/x11-lga1151v2-series/variants/x11sch-f/overridetree.cb 21 files changed, 1,001 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/37441/61 -- To view, visit https://review.coreboot.org/c/coreboot/+/37441 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ab1cb9462607b9af068bc2374508d99c60d0a30 Gerrit-Change-Number: 37441 Gerrit-PatchSet: 61 Gerrit-Owner: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Guido Beyer @ Prodrive Technologies <guido.beyer(a)prodrive-technologies.com> Gerrit-Reviewer: Justin van Son <justin.van.son(a)prodrive-technologies.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Stef van Os <stef.van.os(a)prodrive-technologies.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: wouter.eckhardt(a)prodrive-technologies.com Gerrit-CC: Jonas Löffelholz Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google/wdee: Disable WLAN for Wdee
by Maulik V Vaghela (Code Review) 14 Jul '20

14 Jul '20

Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42098 ) Change subject: mb/google/wdee: Disable WLAN for Wdee ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/42098 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0ac1c9a83fbbf03066c09774324313f62d58bc38 Gerrit-Change-Number: 42098 Gerrit-PatchSet: 2 Gerrit-Owner: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Meera Ravindranath <meera.ravindranath(a)intel.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Uday M Bhat Gerrit-Comment-Date: Tue, 14 Jul 2020 07:25:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/volteer: config QS silicon devices for CSE LITE
by Nick Vaccaro (Code Review) 14 Jul '20

14 Jul '20

Nick Vaccaro has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43413 ) Change subject: mb/google/volteer: config QS silicon devices for CSE LITE ...................................................................... mb/google/volteer: config QS silicon devices for CSE LITE Configure volteer2 and delbin, the two variants currently using preQS or QS silicon, to use CSE Lite. BUG=b:158140797 TEST=cd to volteer's asset_generation folder, execute "./gen_all_variant_images.sh" and verify that all variant images are produced. Change-Id: I5e444529b090d82094b9da0df2648ea4cdb2888e Signed-off-by: Nick Vaccaro <nvaccaro(a)google.com> --- M src/mainboard/google/volteer/Kconfig.name 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/43413/1 diff --git a/src/mainboard/google/volteer/Kconfig.name b/src/mainboard/google/volteer/Kconfig.name index 8e6d884..06bbc58 100644 --- a/src/mainboard/google/volteer/Kconfig.name +++ b/src/mainboard/google/volteer/Kconfig.name @@ -31,6 +31,7 @@ bool "-> Volteer2" select BOARD_GOOGLE_BASEBOARD_VOLTEER select VARIANT_HAS_MIPI_CAMERA + select SOC_INTEL_CSE_LITE_SKU config BOARD_GOOGLE_VOXEL bool "-> Voxel" @@ -40,3 +41,4 @@ config BOARD_GOOGLE_DELBIN bool "-> Delbin" select BOARD_GOOGLE_BASEBOARD_VOLTEER + select SOC_INTEL_CSE_LITE_SKU -- To view, visit https://review.coreboot.org/c/coreboot/+/43413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5e444529b090d82094b9da0df2648ea4cdb2888e Gerrit-Change-Number: 43413 Gerrit-PatchSet: 1 Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-MessageType: newchange

3 4

Change in coreboot[master]: cpu/x86/smm: Enable SMM support for Xeon-SP
by Rocky Phagura (Code Review) 14 Jul '20

14 Jul '20

Rocky Phagura has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41829 ) Change subject: cpu/x86/smm: Enable SMM support for Xeon-SP ...................................................................... Patch Set 8: (7 comments) > Patch Set 8: > > Just wondering if it wouldn't be easier to use some linker script to generate the correct SMRAM layout and then put everything into the SMM rmodule. > > Benefits: > * You would get a build error if you try to use more space than available > * No magic calculations that need to be done, as everything is known at compile time > * Nothin to set up as everything is already in place > > Downside: Bigger SMM rmodule (hopefully compression catches that) For long term a dynamic is method is better. A linker script would solve the problem temporarily but it will be more work. A lot of effort has already been placed into fixing the existing SMM loader. There are certain places where it is necessary to take action based on the CPUID or different flags in MSRs (such as state save flags) and this cannot be done at compile time. Also, the handlers are installed based on the number of CPU threads detected in the system. In addition, the SMM handler will need to go through more changes later on. Currently the entry into SMM is flat 32 real mode. Intel now supports protected mode entries and 64 bit support in SMM. This is beneficial for server platforms mostly. Another approach is that we could call this SMM module loader version 2. Only the server platforms will use version 2 and client/desktop/etc can continue to use the old version. It will make this patch integrate much better into the existing model. https://review.coreboot.org/c/coreboot/+/41829/7//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/41829/7//COMMIT_MSG@9 PS7, Line 9: Xeon-SP a skylake based system has 36 CPU threads (18 cores). Current coreboot > Skylake Scalable Process can have 36 cpu threads (18 cores). Done https://review.coreboot.org/c/coreboot/+/41829/7/src/cpu/x86/mp_init.c File src/cpu/x86/mp_init.c: https://review.coreboot.org/c/coreboot/+/41829/7/src/cpu/x86/mp_init.c@541 PS7, Line 541: mdelay(1); > This will add to boot time. […] It's the smm relocation record that needs the delay. The delay is a workaround for now. .relocation_handler = smm_relocation_handler https://review.coreboot.org/c/coreboot/+/41829/7/src/cpu/x86/smm/smm_module… File src/cpu/x86/smm/smm_module_loader.c: https://review.coreboot.org/c/coreboot/+/41829/7/src/cpu/x86/smm/smm_module… PS7, Line 354: printk(BIOS_INFO, "%s : not enough stacks\n", __func__); > The error conditions should be printk'ed as BIOS_ERR Done https://review.coreboot.org/c/coreboot/+/41829/7/src/cpu/x86/smm/smm_module… PS7, Line 612: "%s : increase SMM_CODE_SEGMENT_SIZE\n", __func__); > print out handler_size will help to know to increase SMM_CODE_SEGMENT_SIZE Done https://review.coreboot.org/c/coreboot/+/41829/7/src/soc/intel/xeon_sp/Make… File src/soc/intel/xeon_sp/Makefile.inc: https://review.coreboot.org/c/coreboot/+/41829/7/src/soc/intel/xeon_sp/Make… PS7, Line 11: ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smmrelocate.c > The xeon_sp related changes should be made into a separate patch. It's part of enabling SMM support for Xeon-SP. Without relocation SMM wouldn't be enabled as per the commit message titie. https://review.coreboot.org/c/coreboot/+/41829/7/src/soc/intel/xeon_sp/smmr… File src/soc/intel/xeon_sp/smmrelocate.c: https://review.coreboot.org/c/coreboot/+/41829/7/src/soc/intel/xeon_sp/smmr… PS7, Line 106: * BSP to do * the final move. For APs, a relocation handler always > There is an extra '*' good catch. https://review.coreboot.org/c/coreboot/+/41829/7/src/soc/intel/xeon_sp/unco… File src/soc/intel/xeon_sp/uncore.c: https://review.coreboot.org/c/coreboot/+/41829/7/src/soc/intel/xeon_sp/unco… PS7, Line 290: void smm_region(uintptr_t *start, size_t *size) > Add an empty line above. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/41829 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78bd74c11ca42fb430f63711b5ec87d4bfe6ca2a Gerrit-Change-Number: 41829 Gerrit-PatchSet: 8 Gerrit-Owner: Rocky Phagura Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Eugene Myers <cedarhouse1(a)comcast.net> Gerrit-Reviewer: Eugene Myers <cedarhouse(a)comcast.net> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Naresh Solanki <naresh.solanki.2011(a)gmail.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Ron Minnich <rminnich(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Comment-Date: Tue, 14 Jul 2020 00:11:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/intel/mipi_camera: Add guard to protect camera resources
by Tim Wawrzynczak (Code Review) 14 Jul '20

14 Jul '20

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43003 ) Change subject: drivers/intel/mipi_camera: Add guard to protect camera resources ...................................................................... Patch Set 7: (15 comments) https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… File src/drivers/intel/mipi_camera/camera.c: https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 490: struct const struct https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 551: method_name = get_op_method_namestring(res_config, res_index); Hm, since we don't need method_name to have a very long lifetime, what about, instead of returning a strdup()'d string in get_op_method_namestring(), you have it instead accept a `char *` that it can snprintf() into? It can't use sizeof() obviously either then and would need a length argument as well. https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 562: acpigen_emit_namestring("MCON"); I'm wondering if this should include a CondRefOf op to check for MCON's existence first? what do you think? https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 566: acpigen_emit_namestring("MCOF"); I'm wondering if this should include a CondRefOf op to check for MCOF's existence first? what do you think? https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 561: if (action == ENABLE) { : acpigen_emit_namestring("MCON"); : acpigen_write_integer(clk_config->clknum); : acpigen_write_integer(clk_config->freq); : } else if (action == DISABLE) { : acpigen_emit_namestring("MCOF"); : acpigen_write_integer(clk_config->clknum); : } else { : acpigen_write_debug_string("Unsupported clock action"); : printk(BIOS_ERR, "Unsupported clock action: %x\n" : "OS camera driver will likely not work", action); : } switch-case conveys the intent more clearly, since `action`'s value is the only condition https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 579: : if (action == ENABLE) { : acpigen_soc_set_tx_gpio(gpio_config->gpio_num); : } else if (action == DISABLE) { : acpigen_soc_clear_tx_gpio(gpio_config->gpio_num); : } else { : acpigen_write_debug_string("Unsupported GPIO action"); : printk(BIOS_ERR, "Unsupported GPIO action: %x\n" : "OS camera driver will likely not work\n", action); : } switch-case conveys the intent more clearly, since `action`'s value is the only condition https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 619: 5 We're using this `5` an awful lot, maybe we should add a constant into acpigen.h? Something like `#define ACPI_SIMPLE_NAMESTRING_ALLOC 5` which would imply you should allocate 5 bytes for a simple namestring (assuming it's nul-terminated) https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 627: char method_name[5], varname[5]; nit: declare on separate lines https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 631: acpigen_write_method_serialized(method_name, 0); nit: blank line after snprintfs https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 642: struct const struct https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 644: char method_name[5], varname[5]; nit: declare on separate lines https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 648: acpigen_write_method_serialized(method_name, 0); nit: blank line after snprintfs https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 659: struct const struct https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 660: struct const struct https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… File src/drivers/intel/mipi_camera/chip.h: https://review.coreboot.org/c/coreboot/+/43003/7/src/drivers/intel/mipi_cam… PS7, Line 80: ENABLE = 1, nit: for enums like this, with an 'UNKNOWN' option, it's often convenient to have that defined as the first element of the enum, so that any data structures which contains that type and is zero-initialized (i.e., devicetree structures) will contain 'UNKNOWN' in that case and will not accidentally contain pseudo-real data. Same with ctrl_type -- To view, visit https://review.coreboot.org/c/coreboot/+/43003 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1468459d5bbb2fb07bef4e0590c96dd4dbab0d9c Gerrit-Change-Number: 43003 Gerrit-PatchSet: 7 Gerrit-Owner: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Matt Delco <delco(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Comment-Date: Mon, 13 Jul 2020 22:56:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/picasso: supply smbios type 17
by Aaron Durbin (Code Review) 13 Jul '20

13 Jul '20

Hello Rob Barnes, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/43351 to review the following change. Change subject: soc/amd/picasso: supply smbios type 17 ...................................................................... soc/amd/picasso: supply smbios type 17 Extract dimm info from AMD_FSP_DMI_HOB and store it as mem_info in cbmem with id CBMEM_ID_MEMINFO. Subsquently extract mem_info objects from cbmem to build smbios type 17 tables . BUG=b:148277751,b:160947978 TEST=dmidecode -t 17 BRANCH=none Change-Id: Iacedbb017d19516674070f89ba0aa217f55383e3 Signed-off-by: Rob Barnes <robbarnes(a)google.com> --- M src/soc/amd/picasso/Makefile.inc A src/soc/amd/picasso/dmi.c D src/vendorcode/amd/fsp/picasso/dmi_info.h 3 files changed, 201 insertions(+), 198 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/43351/1 diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index 593d17b..8b03d59 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -78,6 +78,7 @@ ramstage-y += config.c ramstage-y += update_microcode.c ramstage-y += graphics.c +ramstage-y += dmi.c smm-y += smihandler.c smm-y += smi_util.c diff --git a/src/soc/amd/picasso/dmi.c b/src/soc/amd/picasso/dmi.c new file mode 100644 index 0000000..c597c0a --- /dev/null +++ b/src/soc/amd/picasso/dmi.c @@ -0,0 +1,200 @@ +/* + * This file is part of the coreboot project. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +/** + * This code was adapted from src/soc/amd/common/block/pi/amd_late_init.c + */ + +#include <fsp/util.h> +#include <memory_info.h> +#include <console/console.h> +#include <cbmem.h> +#include <string.h> +#include <ec/google/chromeec/ec.h> +#include <bootstate.h> +#include <lib.h> +#include <dimm_info_util.h> +#include <vendorcode/amd/fsp/picasso/dmi_info.h> + +/** + * Populate dimm_info using AGESA TYPE17_DMI_INFO. + */ +static void transfer_memory_info(const TYPE17_DMI_INFO *dmi17, + struct dimm_info *dimm) +{ + hexstrtobin(dmi17->SerialNumber, dimm->serial, sizeof(dimm->serial)); + + dimm->dimm_size = + smbios_memory_size_to_mib(dmi17->MemorySize, dmi17->ExtSize); + + dimm->ddr_type = dmi17->MemoryType; + + dimm->ddr_frequency = dmi17->Speed; + + dimm->rank_per_dimm = dmi17->Attributes; + + dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->FormFactor); + + dimm->bus_width = + smbios_bus_width_to_spd_width(dmi17->TotalWidth, dmi17->DataWidth); + + dimm->mod_id = dmi17->ManufacturerIdCode; + + dimm->bank_locator = 0; + + strncpy((char *)dimm->module_part_number, dmi17->PartNumber, + sizeof(dimm->module_part_number) - 1); +} + +static void print_dimm_info(const struct dimm_info *dimm) +{ + printk(BIOS_DEBUG, + "CBMEM_ID_MEMINFO:\n" + " dimm_size: %u\n" + " ddr_type: 0x%hx\n" + " ddr_frequency: %hu\n" + " rank_per_dimm: %hhu\n" + " channel_num: %hhu\n" + " dimm_num: %hhu\n" + " bank_locator: %hhu\n" + " mod_id: %hx\n" + " mod_type: 0x%hhx\n" + " bus_width: %hhu\n" + " serial: %02hhx%02hhx%02hhx%02hhx\n" + " module_part_number(%zu): %s\n", + dimm->dimm_size, + dimm->ddr_type, + dimm->ddr_frequency, + dimm->rank_per_dimm, + dimm->channel_num, + dimm->dimm_num, + dimm->bank_locator, + dimm->mod_id, + dimm->mod_type, + dimm->bus_width, + dimm->serial[0], + dimm->serial[1], + dimm->serial[2], + dimm->serial[3], + strlen((const char *)dimm->module_part_number), + (char *)dimm->module_part_number); +} + +static void print_dmi_info(const TYPE17_DMI_INFO *dmi17) +{ + printk(BIOS_DEBUG, + "AGESA TYPE 17 DMI INFO:\n" + " Handle: %hu\n" + " TotalWidth: %hu\n" + " DataWidth: %hu\n" + " MemorySize: %hu\n" + " DeviceSet: %hhu\n" + " Speed: %hu\n" + " ManufacturerIdCode: %llx\n" + " Attributes: %hhu\n" + " ExtSize: %u\n" + " ConfigSpeed: %hu\n" + " MemoryType: 0x%x\n" + " FormFactor: 0x%x\n" + " DeviceLocator: %8s\n" + " BankLocator: %10s\n" + " SerialNumber(%zu): %9s\n" + " PartNumber(%zu): %19s\n", + dmi17->Handle, + dmi17->TotalWidth, + dmi17->DataWidth, + dmi17->MemorySize, + dmi17->DeviceSet, + dmi17->Speed, + dmi17->ManufacturerIdCode, + dmi17->Attributes, + dmi17->ExtSize, + dmi17->ConfigSpeed, + dmi17->MemoryType, + dmi17->FormFactor, + dmi17->DeviceLocator, + dmi17->BankLocator, + strlen((const char *)dmi17->SerialNumber), + dmi17->SerialNumber, + strlen((const char *)dmi17->PartNumber), + dmi17->PartNumber); +} + +/** + * Marshalls dimm info from AMD_FSP_DMI_HOB into CBMEM_ID_MEMINFO + */ +static void prepare_dmi_17(void *unused) +{ + const DMI_INFO *dmi_table; + const TYPE17_DMI_INFO *type17_dmi_info; + struct memory_info *mem_info; + struct dimm_info *dimm_info; + char cbi_part_number[DIMM_INFO_PART_NUMBER_SIZE]; + bool use_cbi_part_number = false; + int dimm_cnt = 0; + size_t amd_fsp_dmi_hob_size; + const EFI_GUID amd_fsp_dmi_hob_guid = AMD_FSP_DMI_HOB_GUID; + + printk(BIOS_DEBUG, "Saving dimm info for smbios type 17\n"); + + /* Allocate meminfo in cbmem. */ + mem_info = cbmem_add(CBMEM_ID_MEMINFO, sizeof(struct memory_info)); + if (!mem_info) { + printk(BIOS_ERR, "Failed to add memory info to CBMEM.\n"); + return; + } + memset(mem_info, 0, sizeof(struct memory_info)); + + /* Locate the memory info HOB. */ + dmi_table = fsp_find_extension_hob_by_guid( + (const uint8_t *)&amd_fsp_dmi_hob_guid, &amd_fsp_dmi_hob_size); + + if (dmi_table == NULL || amd_fsp_dmi_hob_size == 0) { + printk(BIOS_ERR, "AMD_FSP_DMI_HOB not found\n"); + return; + } + printk(BIOS_DEBUG, "AMD_FSP_DMI_HOB found\n"); + + /* Prefer DRAM part number from CBI. */ + if (google_chromeec_cbi_get_dram_part_num( + cbi_part_number, sizeof(cbi_part_number)) != 0) { + printk(BIOS_ERR, "Could not obtain DRAM part number from CBI\n"); + use_cbi_part_number = false; + } else { + use_cbi_part_number = true; + } + + for (int channel = 0; channel < MAX_CHANNELS_PER_SOCKET; channel++) { + for (int dimm = 0; dimm < MAX_DIMMS_PER_CHANNEL; dimm++) { + type17_dmi_info = &dmi_table->T17[0][channel][dimm]; + /* DIMMS that are present will have a non-zero + handle. */ + if (type17_dmi_info->Handle == 0) + continue; + print_dmi_info(type17_dmi_info); + dimm_info = &mem_info->dimm[dimm_cnt]; + dimm_info->channel_num = channel; + dimm_info->dimm_num = channel; + transfer_memory_info(type17_dmi_info, dimm_info); + if (use_cbi_part_number) { + /* mem_info is memset to 0 above, so it's + safe to assume module_part_number will be + null terminated */ + strncpy((char *)dimm_info->module_part_number, + cbi_part_number, + sizeof(dimm_info->module_part_number) + - 1); + } + print_dimm_info(dimm_info); + dimm_cnt++; + } + } + mem_info->dimm_cnt = dimm_cnt; +} + +// AMD_FSP_DMI_HOB is initialized very late, +// so check it just in time for writing tables. +BOOT_STATE_INIT_ENTRY(BS_WRITE_TABLES, BS_ON_ENTRY, prepare_dmi_17, NULL); diff --git a/src/vendorcode/amd/fsp/picasso/dmi_info.h b/src/vendorcode/amd/fsp/picasso/dmi_info.h deleted file mode 100644 index adab2f9..0000000 --- a/src/vendorcode/amd/fsp/picasso/dmi_info.h +++ /dev/null @@ -1,198 +0,0 @@ - /***************************************************************************** - * - * Copyright (c) 2008 - 2017, Advanced Micro Devices, Inc. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * * Neither the name of Advanced Micro Devices, Inc. nor the names of - * its contributors may be used to endorse or promote products derived - * from this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED - * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL ADVANCED MICRO DEVICES, INC. BE LIABLE FOR ANY - * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND - * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - ***************************************************************************/ - -/** - * This code was copied from src/vendorcode/amd/pi/00670F00/AGESA.h - */ - -#define AMD_FSP_DMI_HOB_GUID {0x4118FC0E, 0x353D, 0x4726, { 0x97, 0xC0, 0x53, 0xCD, 0x92, 0xB6, 0x49, 0x25}} - -// Our ACPI HOB max payload, accounting for the size of the HOB header as well as the information structure -#define HOB_MAX_SIZE 0xFFF8 -#define HOB_GUID_EXTENSION_SIZE (HOB_MAX_SIZE - sizeof (EFI_HOB_GUID_TYPE)) - -#define MAX_SOCKETS_SUPPORTED 1 ///< Max number of sockets in system -#define MAX_CHANNELS_PER_SOCKET 4 ///< Max Channels per sockets -#define MAX_DIMMS_PER_CHANNEL 4 ///< Max DIMMs on a memory channel (independent of platform) - -/// DMI Type 16 offset 04h - Location -typedef enum { - OtherLocation = 0x01, ///< Assign 01 to Other - UnknownLocation, ///< Assign 02 to Unknown - SystemboardOrMotherboard, ///< Assign 03 to systemboard or motherboard - IsaAddonCard, ///< Assign 04 to ISA add-on card - EisaAddonCard, ///< Assign 05 to EISA add-on card - PciAddonCard, ///< Assign 06 to PCI add-on card - McaAddonCard, ///< Assign 07 to MCA add-on card - PcmciaAddonCard, ///< Assign 08 to PCMCIA add-on card - ProprietaryAddonCard, ///< Assign 09 to proprietary add-on card - NuBus, ///< Assign 0A to NuBus - Pc98C20AddonCard, ///< Assign 0A0 to PC-98/C20 add-on card - Pc98C24AddonCard, ///< Assign 0A1 to PC-98/C24 add-on card - Pc98EAddoncard, ///< Assign 0A2 to PC-98/E add-on card - Pc98LocalBusAddonCard ///< Assign 0A3 to PC-98/Local bus add-on card -} DMI_T16_LOCATION; - -/// DMI Type 16 offset 05h - Memory Error Correction -typedef enum { - OtherUse = 0x01, ///< Assign 01 to Other - UnknownUse, ///< Assign 02 to Unknown - SystemMemory, ///< Assign 03 to system memory - VideoMemory, ///< Assign 04 to video memory - FlashMemory, ///< Assign 05 to flash memory - NonvolatileRam, ///< Assign 06 to non-volatile RAM - CacheMemory ///< Assign 07 to cache memory -} DMI_T16_USE; - -/// DMI Type 16 offset 07h - Maximum Capacity -typedef enum { - Dmi16OtherErrCorrection = 0x01, ///< Assign 01 to Other - Dmi16UnknownErrCorrection, ///< Assign 02 to Unknown - Dmi16NoneErrCorrection, ///< Assign 03 to None - Dmi16Parity, ///< Assign 04 to parity - Dmi16SingleBitEcc, ///< Assign 05 to Single-bit ECC - Dmi16MultiBitEcc, ///< Assign 06 to Multi-bit ECC - Dmi16Crc ///< Assign 07 to CRC -} DMI_T16_ERROR_CORRECTION; - -/// DMI Type 16 - Physical Memory Array -typedef struct { - OUT DMI_T16_LOCATION Location; ///< The physical location of the Memory Array, - ///< whether on the system board or an add-in board. - OUT DMI_T16_USE Use; ///< Identifies the function for which the array - ///< is used. - OUT DMI_T16_ERROR_CORRECTION MemoryErrorCorrection; ///< The primary hardware error correction or - ///< detection method supported by this memory array. - OUT UINT16 NumberOfMemoryDevices; ///< The number of slots or sockets available - ///< for memory devices in this array. -} TYPE16_DMI_INFO; - -/// DMI Type 17 offset 0Eh - Form Factor -typedef enum { - OtherFormFactor = 0x01, ///< Assign 01 to Other - UnknowFormFactor, ///< Assign 02 to Unknown - SimmFormFactor, ///< Assign 03 to SIMM - SipFormFactor, ///< Assign 04 to SIP - ChipFormFactor, ///< Assign 05 to Chip - DipFormFactor, ///< Assign 06 to DIP - ZipFormFactor, ///< Assign 07 to ZIP - ProprietaryCardFormFactor, ///< Assign 08 to Proprietary Card - DimmFormFactorFormFactor, ///< Assign 09 to DIMM - TsopFormFactor, ///< Assign 10 to TSOP - RowOfChipsFormFactor, ///< Assign 11 to Row of chips - RimmFormFactor, ///< Assign 12 to RIMM - SodimmFormFactor, ///< Assign 13 to SODIMM - SrimmFormFactor, ///< Assign 14 to SRIMM - FbDimmFormFactor ///< Assign 15 to FB-DIMM -} DMI_T17_FORM_FACTOR; - -/// DMI Type 17 offset 12h - Memory Type -typedef enum { - OtherMemType = 0x01, ///< Assign 01 to Other - UnknownMemType, ///< Assign 02 to Unknown - DramMemType, ///< Assign 03 to DRAM - EdramMemType, ///< Assign 04 to EDRAM - VramMemType, ///< Assign 05 to VRAM - SramMemType, ///< Assign 06 to SRAM - RamMemType, ///< Assign 07 to RAM - RomMemType, ///< Assign 08 to ROM - FlashMemType, ///< Assign 09 to Flash - EepromMemType, ///< Assign 10 to EEPROM - FepromMemType, ///< Assign 11 to FEPROM - EpromMemType, ///< Assign 12 to EPROM - CdramMemType, ///< Assign 13 to CDRAM - ThreeDramMemType, ///< Assign 14 to 3DRAM - SdramMemType, ///< Assign 15 to SDRAM - SgramMemType, ///< Assign 16 to SGRAM - RdramMemType, ///< Assign 17 to RDRAM - DdrMemType, ///< Assign 18 to DDR - Ddr2MemType, ///< Assign 19 to DDR2 - Ddr2FbdimmMemType, ///< Assign 20 to DDR2 FB-DIMM - Ddr3MemType = 0x18, ///< Assign 24 to DDR3 - Fbd2MemType, ///< Assign 25 to FBD2 - Ddr4MemType, ///< Assign 26 to DDR4 - LpDdrMemType, ///< Assign 27 to LPDDR - LpDdr2MemType, ///< Assign 28 to LPDDR2 - LpDdr3MemType, ///< Assign 29 to LPDDR3 - LpDdr4MemType, ///< Assign 30 to LPDDR4 -} DMI_T17_MEMORY_TYPE; - -/// DMI Type 17 offset 13h - Type Detail -typedef struct { - OUT UINT16 Reserved1:1; ///< Reserved - OUT UINT16 Other:1; ///< Other - OUT UINT16 Unknown:1; ///< Unknown - OUT UINT16 FastPaged:1; ///< Fast-Paged - OUT UINT16 StaticColumn:1; ///< Static column - OUT UINT16 PseudoStatic:1; ///< Pseudo-static - OUT UINT16 Rambus:1; ///< RAMBUS - OUT UINT16 Synchronous:1; ///< Synchronous - OUT UINT16 Cmos:1; ///< CMOS - OUT UINT16 Edo:1; ///< EDO - OUT UINT16 WindowDram:1; ///< Window DRAM - OUT UINT16 CacheDram:1; ///< Cache Dram - OUT UINT16 NonVolatile:1; ///< Non-volatile - OUT UINT16 Registered:1; ///< Registered (Buffered) - OUT UINT16 Unbuffered:1; ///< Unbuffered (Unregistered) - OUT UINT16 LRDIMM:1; ///< LRDIMM -} DMI_T17_TYPE_DETAIL; - -/// DMI Type 17 - Memory Device -typedef struct { - OUT UINT16 Handle; ///< The temporary handle, or instance number, associated with the structure - OUT UINT16 TotalWidth; ///< Total Width, in bits, of this memory device, including any check or error-correction bits. - OUT UINT16 DataWidth; ///< Data Width, in bits, of this memory device. - OUT UINT16 MemorySize; ///< The size of the memory device. - OUT DMI_T17_FORM_FACTOR FormFactor; ///< The implementation form factor for this memory device. - OUT UINT8 DeviceSet; ///< Identifies when the Memory Device is one of a set of - ///< Memory Devices that must be populated with all devices of - ///< the same type and size, and the set to which this device belongs. - OUT CHAR8 DeviceLocator[8]; ///< The string number of the string that identifies the physically labeled socket or board position where the memory device is located. - OUT CHAR8 BankLocator[13]; ///< The string number of the string that identifies the physically labeled bank where the memory device is located. - OUT DMI_T17_MEMORY_TYPE MemoryType; ///< The type of memory used in this device. - OUT DMI_T17_TYPE_DETAIL TypeDetail; ///< Additional detail on the memory device type - OUT UINT16 Speed; ///< Identifies the speed of the device, in megahertz (MHz). - OUT UINT32 _Reserved1_; - OUT UINT64 ManufacturerIdCode; ///< Manufacturer ID code. - OUT CHAR8 SerialNumber[9]; ///< Serial Number. - OUT CHAR8 PartNumber[21]; ///< Part Number. - OUT UINT8 Attributes; ///< Bits 7-4: Reserved, Bits 3-0: rank. - OUT UINT32 ExtSize; ///< Extended Size. - OUT UINT16 ConfigSpeed; ///< Configured memory clock speed - OUT UINT16 MinimumVoltage; ///< Minimum operating voltage for this device, in millivolts - OUT UINT16 MaximumVoltage; ///< Maximum operating voltage for this device, in millivolts - OUT UINT16 ConfiguredVoltage; ///< Configured voltage for this device, in millivolts - OUT UINT32 _Reserved2_; -} TYPE17_DMI_INFO; - -/// Collection of pointers to the DMI records -typedef struct { - OUT TYPE16_DMI_INFO T16; ///< Type 16 struc - OUT TYPE17_DMI_INFO T17[MAX_SOCKETS_SUPPORTED][MAX_CHANNELS_PER_SOCKET][MAX_DIMMS_PER_CHANNEL]; ///< Type 17 struc -} DMI_INFO; -- To view, visit https://review.coreboot.org/c/coreboot/+/43351 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iacedbb017d19516674070f89ba0aa217f55383e3 Gerrit-Change-Number: 43351 Gerrit-PatchSet: 1 Gerrit-Owner: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-MessageType: newchange

7 21

Change in coreboot[master]: mb/google/zork: Fix check for variant_uses_v3_schematics() in variant...
by Furquan Shaikh (Code Review) 13 Jul '20

13 Jul '20

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43415 ) Change subject: mb/google/zork: Fix check for variant_uses_v3_schematics() in variant_audio_update() ...................................................................... mb/google/zork: Fix check for variant_uses_v3_schematics() in variant_audio_update() CB:43224 ("mb/google/zork: Add helpers for v3 schematics and wifi power enable") added helper functions for determining if a board uses v3 schematics. However, it introduced a regression by adding a wrong check for variant_uses_v3_schematics() in variant_audio_update(). This change fixes the check to ensure that dmic_gpio is updated when variant is not using v3 schematics. BUG=b:161141258,b:161128964 TEST=Verified on trembyle that trackpad works again (it was broken because of the regression). Signed-off-by: Furquan Shaikh <furquan(a)google.com> Change-Id: I0e6ad844f68cface7b545f1547bd94470c30dde4 --- M src/mainboard/google/zork/variants/baseboard/ramstage_common.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/43415/1 diff --git a/src/mainboard/google/zork/variants/baseboard/ramstage_common.c b/src/mainboard/google/zork/variants/baseboard/ramstage_common.c index 143c1b4..f7f11b8 100644 --- a/src/mainboard/google/zork/variants/baseboard/ramstage_common.c +++ b/src/mainboard/google/zork/variants/baseboard/ramstage_common.c @@ -10,7 +10,7 @@ struct soc_amd_picasso_config *cfg = config_of_soc(); struct acpi_gpio *gpio = &cfg->dmic_select_gpio; - if (!variant_uses_v3_schematics()) + if (variant_uses_v3_schematics()) return; if (CONFIG(BOARD_GOOGLE_BASEBOARD_TREMBYLE)) -- To view, visit https://review.coreboot.org/c/coreboot/+/43415 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0e6ad844f68cface7b545f1547bd94470c30dde4 Gerrit-Change-Number: 43415 Gerrit-PatchSet: 1 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: newchange

5 8

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2020