coreboot-gerrit April 2024

coreboot-gerrit@coreboot.org

1 participants
3293 discussions

[XS] Change in coreboot[main]: mb/google/brya/var/xol: Add EC_IN_RW_OD config into early_gpio_table
by SH Kim (Code Review) 29 Apr '24

29 Apr '24

Attention is currently required from: Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik. Hello Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/82099?usp=email to look at the new patch set (#2). Change subject: mb/google/brya/var/xol: Add EC_IN_RW_OD config into early_gpio_table ...................................................................... mb/google/brya/var/xol: Add EC_IN_RW_OD config into early_gpio_table Add GPP_F18 configuration in early_gpio_table. Without this, DUT cannot get the proper state of this signal on early phase. It allowed DUT to attempt to enter into dev mode when EC is in RW currently, it causes the failure of autotest/firmware_DevMode. BUG=b:337365524 TEST=built and run autotest firmware_DevMode Change-Id: I2179bb10b431547bc35f332c74915a63495b779d Signed-off-by: Seunghwan Kim <sh_.kim(a)samsung.corp-partner.google.com> --- M src/mainboard/google/brya/variants/xol/gpio.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/82099/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/82099?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2179bb10b431547bc35f332c74915a63495b779d Gerrit-Change-Number: 82099 Gerrit-PatchSet: 2 Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: mb/google/brya/var/xol: Add EC_IN_RW_OD config into early_gpio_table
by SH Kim (Code Review) 29 Apr '24

29 Apr '24

SH Kim has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82099?usp=email ) Change subject: mb/google/brya/var/xol: Add EC_IN_RW_OD config into early_gpio_table ...................................................................... mb/google/brya/var/xol: Add EC_IN_RW_OD config into early_gpio_table Add GPP_F18 configuration in early_gpio_table. Without this, DUT cannot get the proper state of this signal on early phase. It allowed DUT to attempt to enter into dev mode when EC is in RW currently, it causes the failure of autotest item - firmware_DevMode. BUG=b:337365524 TEST=built and run autotest firmware_DevMode Change-Id: I2179bb10b431547bc35f332c74915a63495b779d Signed-off-by: Seunghwan Kim <sh_.kim(a)samsung.corp-partner.google.com> --- M src/mainboard/google/brya/variants/xol/gpio.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/82099/1 diff --git a/src/mainboard/google/brya/variants/xol/gpio.c b/src/mainboard/google/brya/variants/xol/gpio.c index 168d982..39478a1 100644 --- a/src/mainboard/google/brya/variants/xol/gpio.c +++ b/src/mainboard/google/brya/variants/xol/gpio.c @@ -195,6 +195,8 @@ PAD_CFG_GPI(GPP_E13, NONE, DEEP), /* E15 : RSVD_TP ==> PCH_WP_OD */ PAD_CFG_GPI_GPIO_DRIVER(GPP_E15, NONE, DEEP), + /* F18 : EC_IN_RW_OD ==> EC_IN_RW_OD */ + PAD_CFG_GPI(GPP_F18, NONE, DEEP), /* H6 : I2C1_SDA ==> PCH_I2C_TPM_SDA */ PAD_CFG_NF(GPP_H6, NONE, DEEP, NF1), /* H7 : I2C1_SCL ==> PCH_I2C_TPM_SCL */ -- To view, visit https://review.coreboot.org/c/coreboot/+/82099?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2179bb10b431547bc35f332c74915a63495b779d Gerrit-Change-Number: 82099 Gerrit-PatchSet: 1 Gerrit-Owner: SH Kim <sh_.kim(a)samsung.corp-partner.google.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: mb/google/corsola/wugtrio: Add initialization of USB port0
by Yidi Lin (Code Review) 29 Apr '24

29 Apr '24

1 0

[S] Change in coreboot[main]: mb/google/corsola/wugtrio: Add initialization of USB port0
by Yidi Lin (Code Review) 29 Apr '24

29 Apr '24

Attention is currently required from: Hung-Te Lin, Paul Menzel, Wentao Qin, Yang Wu, Yu-Ping Wu. Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/82089?usp=email ) Change subject: mb/google/corsola/wugtrio: Add initialization of USB port0 ...................................................................... Patch Set 4: (4 comments) Patchset: PS4: @Mingjin, is it expected to run through USB3 settings for USB2 configuration ? File src/mainboard/google/corsola/devicetree.cb: https://review.coreboot.org/c/coreboot/+/82089/comment/e3a51f44_f46381ee : PS4, Line 3: USB2_PORT SUPPORT_SECONDARY_USB https://review.coreboot.org/c/coreboot/+/82089/comment/291aa8a3_59574be7 : PS4, Line 4: USB2_PORT1 option SECONDARY_USB_DISABLED 0 option SECONDARY_USB_ENABLED 1 File src/soc/mediatek/common/usb.c: https://review.coreboot.org/c/coreboot/+/82089/comment/96e42fe7_22e037aa : PS4, Line 15: : #ifdef SOC_MEDIATEK_MT8186_USB_H : void switch_usb_port0_host(void) : { : ippc_regs = (void *)(SSUSB_IPPC_BASE_P0); : phy_ports = (void *)(SSUSB_SIF_BASE_P0); : } : #endif : Please consider the following approach. common/usb.c ``` void update_usb_base_regs(uintptr_t ippc_base, uintptr_t sif_base) { ippc_regs = (void *)ippc_base; phy_ports = (void *)sif_base; } ``` common/usb_secondary.c ``` void setup_usb_secondary_host(void) { /* We always consider USB2 port as the secondary UBS host regardless of the register naming */ update_usb_base_regs(SSUSB_IPPC_BASE_P0, SSUSB_SIF_BASE_P0); setup_usb_host(); } ``` mt8186/Makeifle.mk ``` ramstage-y += ../common/usb.c ../common/usb_secondary.c usb.c ``` corsola/mainboard.c ``` if (fw_config_probe(FW_CONFIG(SUPPORT_SECONDARY_USB, SECONDARY_USB_ENABLED))) { setup_usb_secondary_host(); } ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/82089?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I725b80593f5fc498a204bf47f943c36ccbd78134 Gerrit-Change-Number: 82089 Gerrit-PatchSet: 4 Gerrit-Owner: Wentao Qin <qinwentao(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Mingjin Ge <mingjin.ge(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Xuxin Xiong <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Wentao Qin <qinwentao(a)huaqin.corp-partner.google.com> Gerrit-Comment-Date: Mon, 29 Apr 2024 02:00:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XL] Change in coreboot[main]: soc/intel/xeon_sp: Add Granite Rapids initial codes
by Shuo Liu (Code Review) 29 Apr '24

29 Apr '24

Attention is currently required from: Angel Pons, Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Felix Held, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Nico Huber, Patrick Rudolph, Paul Menzel, TangYiwei, Tim Chu. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81316?usp=email ) Change subject: soc/intel/xeon_sp: Add Granite Rapids initial codes ...................................................................... Patch Set 57: (10 comments) File src/soc/intel/xeon_sp/Makefile.mk: https://review.coreboot.org/c/coreboot/+/81316/comment/60ce7cef_fcfbd48e : PS56, Line 8: ## GNR IBL codes are initially reused from EBG, will update later. > nit: Add a "TODO: " prefix? Done File src/soc/intel/xeon_sp/chip_gen6.c: https://review.coreboot.org/c/coreboot/+/81316/comment/461fc425_0e836dd1 : PS56, Line 35: static void iio_pci_domain_read_resources(struct device *dev) > I'd recommend using the constructors in https://github. […] I updated legacy IO resources with constructors, however, for PCI domains we have some restrictions as below, 1. We cannot use IORESOURCE_STORED because, https://github.com/coreboot/coreboot/blob/main/src/acpi/acpigen_pci_root_re… 2. For IORESOURCE_FIXED, which is conflict with below logics, https://github.com/coreboot/coreboot/blob/main/src/device/resource_allocato… For existing constructors which are based on fixed_resource_range_idx, they are not filling resource limits and hence cannot be correctly handled by https://github.com/coreboot/coreboot/blob/main/src/acpi/acpigen_pci_root_re… which is based on {base, limits}. Besides, they are with IORESOURCE_FIXED resource set by default, which cannot correctly work with resource allocators. Maybe we could have constructor utils for domain resource creation in device_util.c? (we can discuss in a separate thread) https://review.coreboot.org/c/coreboot/+/81316/comment/0e4eed9a_32c695bb : PS56, Line 56: res->flags = IORESOURCE_IO | IORESOURCE_SUBTRACTIVE | IORESOURCE_ASSIGNED; > I asked on IRC, this resource should be `IORESOURCE_FIXED` so that the allocator doesn't try to allo […] Good suggestion! Updated. File src/soc/intel/xeon_sp/gnr/Kconfig: https://review.coreboot.org/c/coreboot/+/81316/comment/37f7a43b_05f17ef3 : PS56, Line 17: select FSP_SPEC_VIOLATION_XEON_SP_HEAP_WORKAROUND > Intel said this issue is not present on GNR, so this is wrong. GNR still needs this ... the related discussion history is at: https://review.coreboot.org/c/coreboot/+/80579 https://review.coreboot.org/c/coreboot/+/80328 File src/soc/intel/xeon_sp/gnr/chip.c: https://review.coreboot.org/c/coreboot/+/81316/comment/b1937c2d_2cce8044 : PS56, Line 13: intelblocks > do you need all those headers? Good catch! The list can be largely reduced. Updated. File src/soc/intel/xeon_sp/gnr/cpu.c: https://review.coreboot.org/c/coreboot/+/81316/comment/fb1e4cfb_c89ad987 : PS56, Line 9: common > do you need all those headers? Good catch! The list can be largely reduced. Updated. File src/soc/intel/xeon_sp/gnr/romstage.c: https://review.coreboot.org/c/coreboot/+/81316/comment/d8e5d3a7_a146220c : PS56, Line 5: #include <console/console.h> > do you need all those headers? Good catch! The list can be largely reduced. Updated. https://review.coreboot.org/c/coreboot/+/81316/comment/8448bae8_dbf5781b : PS56, Line 21: switch (base_addr) { > OK, these are a bit tricky to handle because decimals, but how about: […] It would make sense to align with UPD definition which is in decimal form. Updated. File src/soc/intel/xeon_sp/spr/cpu.c: PS56: > nit: Maybe move these changes to a separate commit? Done File src/soc/intel/xeon_sp/spr/soc_acpi.c: https://review.coreboot.org/c/coreboot/+/81316/comment/87862398_d1cb1cb2 : PS56, Line 158: unsigned long xeonsp_acpi_create_madt_lapics(unsigned long current) > Where did this go? Now the common acpi codes can handle madt creation well. unsigned long acpi_arch_fill_madt(acpi_madt_t *madt, unsigned long current) { madt->lapic_addr = cpu_get_lapic_addr(); if (CONFIG(ACPI_HAVE_PCAT_8259)) madt->flags |= 1; if (CONFIG(ACPI_COMMON_MADT_LAPIC)) -> ACPI_COMMON_MADT_LAPIC will be set current = acpi_create_madt_lapics_with_nmis(current); if (CONFIG(ACPI_COMMON_MADT_IOAPIC)) current = acpi_create_madt_ioapic_gsi0_default(current); return current; } -- To view, visit https://review.coreboot.org/c/coreboot/+/81316?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3084e1b5abf25d8d9504bebeaed2a15b916ed56b Gerrit-Change-Number: 81316 Gerrit-PatchSet: 57 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: TangYiwei Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Mon, 29 Apr 2024 00:29:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: soc/intel/xeon_sp: Use fixed BDF for IBL
by Shuo Liu (Code Review) 29 Apr '24

29 Apr '24

Attention is currently required from: Angel Pons, Arthur Heymans, Christian Walter, David Hendricks, Johnny Lin, Jonathan Zhang, Jérémy Compostella, Lean Sheng Tan, Patrick Rudolph, TangYiwei, Tim Chu. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81321?usp=email ) Change subject: soc/intel/xeon_sp: Use fixed BDF for IBL ...................................................................... Patch Set 21: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/81321/comment/3cb999d7_7498feea : PS20, Line 11: dyanmically > typo: dynamically Done -- To view, visit https://review.coreboot.org/c/coreboot/+/81321?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3975cb00e215c4984c63bb8510e8aef7d4cc85a4 Gerrit-Change-Number: 81321 Gerrit-PatchSet: 21 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Mon, 29 Apr 2024 00:29:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: mb/intel/beechnutcity_crb: Add GNR/SRF-SP 2S server board Beechnut City
by Shuo Liu (Code Review) 29 Apr '24

29 Apr '24

Attention is currently required from: Angel Pons, Arthur Heymans, Chen, Gang C, David Hendricks, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Paul Menzel, TangYiwei. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81322?usp=email ) Change subject: mb/intel/beechnutcity_crb: Add GNR/SRF-SP 2S server board Beechnut City ...................................................................... Patch Set 29: (1 comment) File configs/builder/config.intel.crb.bnc: https://review.coreboot.org/c/coreboot/+/81322/comment/ba7155d4_3cb4290b : PS11, Line 27: # : CONFIG_IFD_BIN_PATH="site-local/beechnutcity/descriptor.bin" : CONFIG_CPU_UCODE_BINARIES="site-local/beechnutcity/ucode.mcb" : CONFIG_FSP_T_FILE="site-local/beechnutcity/Server_T.fd" : CONFIG_FSP_M_FILE="site-local/beechnutcity/Server_M.fd" : CONFIG_FSP_S_FILE="site-local/beechnutcity/Server_S.fd" : CONFIG_FSP_HEADER_PATH="src/vendorcode/intel/fsp/fsp2_0/graniterapids/sp/" > This describes the settings needed to build a bootable image for this board. […] Yes ... site-local is used as placeholder at the moment. -- To view, visit https://review.coreboot.org/c/coreboot/+/81322?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3f6a0fb97b62baadb438fb9f11fdd78fccb3f89a Gerrit-Change-Number: 81322 Gerrit-PatchSet: 29 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: TangYiwei Gerrit-Comment-Date: Mon, 29 Apr 2024 00:29:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: mb/intel/avenuecity_crb: Add GNR/SRF-AP 2S server board Avenue City
by Shuo Liu (Code Review) 29 Apr '24

29 Apr '24

Attention is currently required from: Angel Pons, Arthur Heymans, Chen, Gang C, David Hendricks, Jincheng Li, Jonathan Zhang, Lean Sheng Tan, Nicholas Chin, Patrick Rudolph, Paul Menzel, TangYiwei, Varshit Pandya. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81319?usp=email ) Change subject: mb/intel/avenuecity_crb: Add GNR/SRF-AP 2S server board Avenue City ...................................................................... Patch Set 51: (3 comments) File src/drivers/ocp/include/vpd.h: https://review.coreboot.org/c/coreboot/+/81319/comment/429f0eac_1b60e62e : PS50, Line 6: #include <include/types.h> > Huh? This header doesn't need `types. […] This is because this header defines some functions that use the types, e.g., int get_int_from_vpd_range(const char *const key, const int fallback, const int min, const int max); If we do not include the <include/types.h> inside this file, we need to include <include/types.h> ahead of this file <drivers/ocp/include/vpd.h> in the file uses these logics. However, put <include/types.h> ahead of <driver/..../vpd.h> conflicts with alphabetical order and not straightforward in usage. Hence we still need the change here ... File src/mainboard/intel/avenuecity_crb/config/iio.c: https://review.coreboot.org/c/coreboot/+/81319/comment/2677fe8c_28d33575 : PS36, Line 14: _IIO_PORT_CFG_STRUCT_X8(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x1), : _IIO_PORT_CFG_STRUCT_DISABLED, : _IIO_PORT_CFG_STRUCT_DISABLED, : _IIO_PORT_CFG_STRUCT_DISABLED, : _IIO_PORT_CFG_STRUCT_X2(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x2), : _IIO_PORT_CFG_STRUCT_X2(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x3), : _IIO_PORT_CFG_STRUCT_X2(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x4), : _IIO_PORT_CFG_STRUCT_X2(0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, : 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x4B, 0x5) > If it's just the macros, we could define some macros to use in coreboot that use left-to-right order […] Good suggestion! I added it into https://review.coreboot.org/c/coreboot/+/81318 and updated the macro reference here as well. File src/mainboard/intel/avenuecity_crb/config/iio.c: https://review.coreboot.org/c/coreboot/+/81319/comment/6352d8d3_554a6ec3 : PS50, Line 28: (sizeof(iio_config_table)/sizeof(struct iio_pe_config)) > `ARRAY_SIZE(iio_config_table)` Done -- To view, visit https://review.coreboot.org/c/coreboot/+/81319?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I64fdd5388aadf7732f6d3daa600c1455d3672a46 Gerrit-Change-Number: 81319 Gerrit-PatchSet: 51 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Mon, 29 Apr 2024 00:28:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: device_util: Set resource limit in fixed_resource_range_idx
by Shuo Liu (Code Review) 29 Apr '24

29 Apr '24

Shuo Liu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82098?usp=email ) Change subject: device_util: Set resource limit in fixed_resource_range_idx ...................................................................... device_util: Set resource limit in fixed_resource_range_idx Change-Id: Idbd7cf21b4375a891f9149194445ba44af02642a Signed-off-by: Shuo Liu <shuo.liu(a)intel.com> --- M src/device/device_util.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/82098/1 diff --git a/src/device/device_util.c b/src/device/device_util.c index 7dcf081..312ef17 100644 --- a/src/device/device_util.c +++ b/src/device/device_util.c @@ -786,6 +786,7 @@ resource = new_resource(dev, index); resource->base = base; resource->size = size; + resource->limit = base + size - 1; resource->flags = IORESOURCE_FIXED | IORESOURCE_ASSIGNED; resource->flags |= flags; -- To view, visit https://review.coreboot.org/c/coreboot/+/82098?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Idbd7cf21b4375a891f9149194445ba44af02642a Gerrit-Change-Number: 82098 Gerrit-PatchSet: 1 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: soc/intel/xeon_sp/spr: Improves CPU codes
by Shuo Liu (Code Review) 29 Apr '24

29 Apr '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu. Shuo Liu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/82097?usp=email ) Change subject: soc/intel/xeon_sp/spr: Improves CPU codes ...................................................................... soc/intel/xeon_sp/spr: Improves CPU codes 1. xeonsp_acpi_create_madt_lapics is removed due to ACPI common code can handle this well with ACPI_COMMON_MADT_LAPIC set. 2. Improvement in log prints, comments and the file include list. Change-Id: I2bc94391f060cec9de91183021da03bc5c7438c0 Signed-off-by: Shuo Liu <shuo.liu(a)intel.com> --- M src/soc/intel/xeon_sp/include/soc/acpi.h M src/soc/intel/xeon_sp/spr/cpu.c M src/soc/intel/xeon_sp/spr/soc_acpi.c 3 files changed, 10 insertions(+), 34 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/82097/1 diff --git a/src/soc/intel/xeon_sp/include/soc/acpi.h b/src/soc/intel/xeon_sp/include/soc/acpi.h index e374544..e11b19e 100644 --- a/src/soc/intel/xeon_sp/include/soc/acpi.h +++ b/src/soc/intel/xeon_sp/include/soc/acpi.h @@ -18,7 +18,6 @@ unsigned long northbridge_write_acpi_tables(const struct device *device, unsigned long current, struct acpi_rsdp *rsdp); -unsigned long xeonsp_acpi_create_madt_lapics(unsigned long current); unsigned long acpi_fill_cedt(unsigned long current); unsigned long acpi_fill_hmat(unsigned long current); unsigned long cxl_fill_srat(unsigned long current); diff --git a/src/soc/intel/xeon_sp/spr/cpu.c b/src/soc/intel/xeon_sp/spr/cpu.c index f9c8e26..1cb16e5 100644 --- a/src/soc/intel/xeon_sp/spr/cpu.c +++ b/src/soc/intel/xeon_sp/spr/cpu.c @@ -1,28 +1,17 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#include <acpi/acpigen.h> -#include <acpi/acpi.h> -#include <console/console.h> #include <console/debug.h> -#include <cpu/cpu.h> -#include <cpu/intel/cpu_ids.h> #include <cpu/intel/common/common.h> -#include <cpu/intel/em64t101_save_state.h> #include <cpu/intel/microcode.h> #include <cpu/intel/smm_reloc.h> #include <cpu/intel/turbo.h> -#include <cpu/x86/lapic.h> #include <cpu/x86/mp.h> -#include <cpu/x86/mtrr.h> #include <cpu/x86/topology.h> -#include <device/pci_mmio_cfg.h> #include <intelblocks/cpulib.h> #include <intelblocks/mp_init.h> #include <intelpch/lockdown.h> #include <soc/msr.h> -#include <soc/pci_devs.h> #include <soc/pm.h> -#include <soc/soc_util.h> #include <soc/smmrelocate.h> #include <soc/util.h> @@ -235,6 +224,12 @@ { unsigned int num_phys = 0, num_virts = 0; + /* + * This call calculates the thread count which is corresponding to num_virts + * (logical cores), while num_phys is corresponding to physical cores (in SMT + * system, one physical core has multiple threads, a.k.a. logical cores). + * Hence num_phys is not actually used. + */ cpu_read_topology(&num_phys, &num_virts); printk(BIOS_SPEW, "Detected %u cores and %u threads\n", num_phys, num_virts); return num_virts * soc_get_num_cpus(); @@ -275,10 +270,11 @@ microcode_patch = intel_microcode_find(); if (!microcode_patch) - printk(BIOS_ERR, "microcode not found in CBFS!\n"); + printk(BIOS_WARNING, "microcode not found in CBFS and the update will be skipped!\n"); intel_microcode_load_unlocked(microcode_patch); - if (mp_init_with_smm(bus, &mp_ops) < 0) - printk(BIOS_ERR, "MP initialization failure.\n"); + enum cb_err ret = mp_init_with_smm(bus, &mp_ops); + if (ret < 0) + printk(BIOS_ERR, "MP initialization failure %d.\n", ret); } diff --git a/src/soc/intel/xeon_sp/spr/soc_acpi.c b/src/soc/intel/xeon_sp/spr/soc_acpi.c index 1249b8f..c939dee 100644 --- a/src/soc/intel/xeon_sp/spr/soc_acpi.c +++ b/src/soc/intel/xeon_sp/spr/soc_acpi.c @@ -155,25 +155,6 @@ acpigen_pop_len(); } -unsigned long xeonsp_acpi_create_madt_lapics(unsigned long current) -{ - struct device *cpu; - uint8_t num_cpus = 0; - - for (cpu = all_devices; cpu; cpu = cpu->next) { - if ((cpu->path.type != DEVICE_PATH_APIC) - || (cpu->upstream->dev->path.type != DEVICE_PATH_CPU_CLUSTER)) { - continue; - } - if (!cpu->enabled) - continue; - current = acpi_create_madt_one_lapic(current, num_cpus, cpu->path.apic.apic_id); - num_cpus++; - } - - return current; -} - unsigned long acpi_fill_cedt(unsigned long current) { const IIO_UDS *hob = get_iio_uds(); -- To view, visit https://review.coreboot.org/c/coreboot/+/82097?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2bc94391f060cec9de91183021da03bc5c7438c0 Gerrit-Change-Number: 82097 Gerrit-PatchSet: 1 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newchange

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2024