coreboot-gerrit June 2021

coreboot-gerrit@coreboot.org

1 participants
7127 discussions

Change in coreboot[master]: soc/intel/alderlake: Correct TCSS XHCI Port status offset
by Sridhar Siricilla (Code Review) 04 Jun '21

04 Jun '21

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Subrata Banik, Balaji Manigandan, Deepti Deshatty, Patrick Rudolph. Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55230 ) Change subject: soc/intel/alderlake: Correct TCSS XHCI Port status offset ...................................................................... Patch Set 2: (1 comment) File src/soc/intel/alderlake/xhci.c: https://review.coreboot.org/c/coreboot/+/55230/comment/b3d22fc4_6fbcc30c PS1, Line 19: 10 This needs correction as well since ADL-P CPU doesn't support USB2 ports. For CPU TCSS ports support, PCH USB2 ports are used. -- To view, visit https://review.coreboot.org/c/coreboot/+/55230 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I20c77d78f52277a9a979e11303cdb6cdabae7c59 Gerrit-Change-Number: 55230 Gerrit-PatchSet: 2 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Attention: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 04 Jun 2021 18:33:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: src/mainboard: Add Star Labs labtop series
by Angel Pons (Code Review) 04 Jun '21

04 Jun '21

Attention is currently required from: Star Labs. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55128 ) Change subject: src/mainboard: Add Star Labs labtop series ...................................................................... Patch Set 15: (33 comments) File src/mainboard/starlabs/labtop/Kconfig: https://review.coreboot.org/c/coreboot/+/55128/comment/704e7102_7f91ac3f PS15, Line 26: # select HAVE_IFD_BIN : # select HAVE_ME_BIN Remove? https://review.coreboot.org/c/coreboot/+/55128/comment/04b5cbce_04e223c6 PS15, Line 61: config MAX_CPUS : int : default 8 if BOARD_STARLABS_LABTOP_KBL : default 12 Same values are already specified in SoC Kconfig https://review.coreboot.org/c/coreboot/+/55128/comment/63bf6de2_f6289a78 PS15, Line 66: #config DRIVER_TPM_SPI_CHIP : # int : # default 2 Commented-out, remove? https://review.coreboot.org/c/coreboot/+/55128/comment/0f13da90_48ba54f9 PS15, Line 90: config IFD_BIN_PATH : string : default "3rdparty/blobs/mainboard/\$(MAINBOARDDIR)/\$(CONFIG_VARIANT_DIR)/flashregion_0_flashdescriptor.bin" : : config ME_BIN_PATH : string : default "3rdparty/blobs/mainboard/\$(MAINBOARDDIR)/\$(CONFIG_VARIANT_DIR)/flashregion_2_intel_me.bin" Do these files exist in the blobs repo? https://review.coreboot.org/c/coreboot/+/55128/comment/fae8d25d_23838581 PS15, Line 107: config EC_STARLABS_IT_BIN_PATH : string : depends on EC_STARLABS_IT_BIN : default "3rdparty/blobs/mainboard/\$(MAINBOARDDIR)/\$(CONFIG_VARIANT_DIR)/flashregion_8_ec.bin" Same here https://review.coreboot.org/c/coreboot/+/55128/comment/c34e4887_44b3ec96 PS15, Line 120: default "3rdparty/blobs/mainboard/starlabs/Logo.bmp" Same here File src/mainboard/starlabs/labtop/acpi/mainboard.asl: https://review.coreboot.org/c/coreboot/+/55128/comment/5eb80fe7_1ad20cfe PS15, Line 6: /* Power button device. */ : Device (PWRB) : { : Name (_HID, EisaId ("PNP0C0C")) : Name (PBST, One) : : Method (_STA, 0, NotSerialized) : { : Return (0x0F) : } : } This should be removed. File src/mainboard/starlabs/labtop/mainboard.c: https://review.coreboot.org/c/coreboot/+/55128/comment/efb4922c_25648899 PS15, Line 33: #if CONFIG(BOARD_STARLABS_LABTOP_CML) Preprocessor not needed. https://review.coreboot.org/c/coreboot/+/55128/comment/a375f537_fae3be68 PS15, Line 52: /* Override smbios_mainboard_board_type */ : smbios_board_type smbios_mainboard_board_type(void) : { : return SMBIOS_BOARD_TYPE_MOTHERBOARD; : } Remove. File src/mainboard/starlabs/labtop/ramstage.c: https://review.coreboot.org/c/coreboot/+/55128/comment/ddfc8fc1_91fbbeb6 PS15, Line 1: Spurious blank line? https://review.coreboot.org/c/coreboot/+/55128/comment/9e2cb0f3_f04d268b PS15, Line 10: #if CONFIG(BOARD_STARLABS_LABTOP_CML) Doing something like CB:48143 avoids the need for preprocessor. File src/mainboard/starlabs/labtop/spd/spd_util.c: https://review.coreboot.org/c/coreboot/+/55128/comment/ba45d584_7ce129cf PS15, Line 8: void mainboard_fill_dq_map_data(void *dq_map_ptr) : { : /* DQ byte map */ : const u8 dq_map[2][12] = { : {0x0F, 0xF0, 0x00, 0xF0, 0x0F, 0xF0, 0x0F, 0x00, 0xFF, 0x00, 0xFF, 0x00}, : {0x33, 0xCC, 0x00, 0xCC, 0x33, 0xCC, 0x33, 0x00, 0xFF, 0x00, 0xFF, 0x00} }; : memcpy(dq_map_ptr, dq_map, sizeof(dq_map)); : } : : void mainboard_fill_dqs_map_data(void *dqs_map_ptr) : { : /* DQS CPU<>DRAM map */ : const u8 dqs_map[2][8] = {{0, 6, 3, 1, 5, 2, 7, 4}, {7, 5, 3, 6, 2, 4, 0, 1} }; : memcpy(dqs_map_ptr, dqs_map, sizeof(dqs_map)); : } Meaningless with DDR4 File src/mainboard/starlabs/labtop/variants/cml/devicetree.cb: https://review.coreboot.org/c/coreboot/+/55128/comment/c789a88d_229b0ec1 PS15, Line 101: StarPoint This doesn't look like a valid HID https://review.coreboot.org/c/coreboot/+/55128/comment/d7309dc1_cca14fcb PS15, Line 140: device pci 00.0 on end This root port is wired to a slot, so this device may not always be present. It should not be in the devicetree. File src/mainboard/starlabs/labtop/variants/cml/include/variant/gpio.h: PS15: This file shouldn't be a header, but a compilation unit (.c file) of its own. File src/mainboard/starlabs/labtop/variants/cml/include/variant/hda_verb.h: PS15: Same here, this should be a compilation unit (.c file). https://review.coreboot.org/c/coreboot/+/55128/comment/3c3ad758_3d1b18ed PS15, Line 13: 0x0000002b This makes more sense if written in decimal. And shouldn't it be 33 instead? https://review.coreboot.org/c/coreboot/+/55128/comment/aab5965d_aa18f3e2 PS15, Line 20: 0x0017209E, : 0x00172111, : 0x001722EC, : 0x00172310, There's an AZALIA_SUBVENDOR() macro https://review.coreboot.org/c/coreboot/+/55128/comment/d04bce83_d32bef5b PS15, Line 36: /* ONE DOES NOT SIMPLY : MAKE IT WORK WITH WINDOWS */ I cna understand the frustration, but is this comment necessary? File src/mainboard/starlabs/labtop/variants/cml/romstage.c: https://review.coreboot.org/c/coreboot/+/55128/comment/48c59b57_885be030 PS15, Line 52: /* : * 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, 6, 1, 3, 5, 2, 7, 4}, : .dqs_map[DDR_CH1] = {7, 5, 3, 6, 2, 4, 0, 1}, This doesn't matter for DDR4. File src/mainboard/starlabs/labtop/variants/kbl/devicetree.cb: https://review.coreboot.org/c/coreboot/+/55128/comment/0b1ba636_d5f47e7a PS15, Line 19: # Send an extra VR mailbox command for the PS4 exit issue : # register "SendVrMbxCmd" = "2" Commented-out, remove? https://review.coreboot.org/c/coreboot/+/55128/comment/30eef020_9dcdcc4e PS15, Line 24: .up_delay_ms= 200,// T3 : .down_delay_ms= 0,// T10 : .cycle_delay_ms = 500,// T12 : .backlight_on_delay_ms=50,// T7 : .backlight_off_delay_ms = 0,// T9 : .backlight_pwm_hz = 200, nit: alignment... https://review.coreboot.org/c/coreboot/+/55128/comment/e9562999_34baab02 PS15, Line 48: : [PchSerialIoIndexI2C2]= PchSerialIoPci, : [PchSerialIoIndexI2C3]= PchSerialIoPci, Corresponding PCI devices are off. https://review.coreboot.org/c/coreboot/+/55128/comment/9c993c55_ba4d6c79 PS15, Line 52: [PchSerialIoIndexI2C5]= PchSerialIoPci, : [PchSerialIoIndexSpi0]= PchSerialIoPci, : [PchSerialIoIndexSpi1]= PchSerialIoPci, Corresponding PCI devices are off. https://review.coreboot.org/c/coreboot/+/55128/comment/23b80b6f_1cc45be6 PS15, Line 55: [PchSerialIoIndexUart0] = PchSerialIoSkipInit, If unused, can be disabled. No other function of PCI device 30 is enabled. https://review.coreboot.org/c/coreboot/+/55128/comment/8934f13d_e4dcb4b5 PS15, Line 110: StarPoint Doesn't look like a valid HID https://review.coreboot.org/c/coreboot/+/55128/comment/772bbcec_a5de3eb6 PS15, Line 159: device pci 00.0 on end Same as CML: this PCIe port is wired to a slot, and this device may not always be present. https://review.coreboot.org/c/coreboot/+/55128/comment/33b9e4f1_e7b95725 PS15, Line 198: subsystemid 0x10ec 0x111e Suspicious indentation https://review.coreboot.org/c/coreboot/+/55128/comment/8873a471_60710e6d PS15, Line 200: off Should be on File src/mainboard/starlabs/labtop/variants/kbl/include/variant/gpio.h: PS15: Same here, this should be a compilation unit (.c file). File src/mainboard/starlabs/labtop/variants/kbl/include/variant/hda_verb.h: PS15: Same here, this should be a compilation unit (.c file). https://review.coreboot.org/c/coreboot/+/55128/comment/d471ab48_4e22489f PS15, Line 13: 0x0000002b Same as CML, this makes more sense if written in decimal. And should be 18, otherwise you have a lovely buffer overflow. https://review.coreboot.org/c/coreboot/+/55128/comment/236513cb_51d533b4 PS15, Line 18: 0x0017201E, : 0x00172111, : 0x001722EC, : 0x00172310, Use the AZALIA_SUBVENDOR macro? -- To view, visit https://review.coreboot.org/c/coreboot/+/55128 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iffa6061b0e600880b0c93746f35b1731e4841e31 Gerrit-Change-Number: 55128 Gerrit-PatchSet: 15 Gerrit-Owner: Star Labs <admin(a)starlabs.systems> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Attention: Star Labs <admin(a)starlabs.systems> Gerrit-Comment-Date: Fri, 04 Jun 2021 18:33:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/starlabs/labtop: Unbreak master
by Michael Niewöhner (Code Review) 04 Jun '21

04 Jun '21

Attention is currently required from: Nico Huber, Tim Wawrzynczak. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55231 ) Change subject: mb/starlabs/labtop: Unbreak master ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/55231 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I379880b1165bb9da4117aa860dee111fba38a43f Gerrit-Change-Number: 55231 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Star Labs <admin(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Fri, 04 Jun 2021 18:26:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya: Remove I2C4 usage in devicetree.cb
by Subrata Banik (Code Review) 04 Jun '21

04 Jun '21

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55170 ) Change subject: mb/google/brya: Remove I2C4 usage in devicetree.cb ...................................................................... mb/google/brya: Remove I2C4 usage in devicetree.cb I2C4 is not used pn Brya hence make below changes: 1. Disable it in SerialIoI2cMode. 2. Remove I2C4 config in common_soc_config. TEST=Make sure FSP is not programming I2C4. Change-Id: I94c72b7fac9d8a001913b5faa2c0c8a3e8b701e9 Signed-off-by: Subrata Banik <subrata.banik(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55170 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Reviewed-by: Furquan Shaikh <furquan(a)google.com> --- M src/mainboard/google/brya/variants/baseboard/devicetree.cb 1 file changed, 1 insertion(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved diff --git a/src/mainboard/google/brya/variants/baseboard/devicetree.cb b/src/mainboard/google/brya/variants/baseboard/devicetree.cb index 36635fb..5b2ec14 100644 --- a/src/mainboard/google/brya/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/brya/variants/baseboard/devicetree.cb @@ -68,7 +68,7 @@ [PchSerialIoIndexI2C1] = PchSerialIoPci, [PchSerialIoIndexI2C2] = PchSerialIoPci, [PchSerialIoIndexI2C3] = PchSerialIoPci, - [PchSerialIoIndexI2C4] = PchSerialIoPci, + [PchSerialIoIndexI2C4] = PchSerialIoDisabled, [PchSerialIoIndexI2C5] = PchSerialIoPci, }" @@ -118,9 +118,6 @@ .early_init = 1, .speed = I2C_SPEED_FAST, }, - .i2c[4] = { - .speed = I2C_SPEED_FAST, - }, .i2c[5] = { .speed = I2C_SPEED_FAST, }, -- To view, visit https://review.coreboot.org/c/coreboot/+/55170 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I94c72b7fac9d8a001913b5faa2c0c8a3e8b701e9 Gerrit-Change-Number: 55170 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/intel/common/acpi: add ACPI S0ix _DSM for Intel Power Engine Plug-in
by Michael Niewöhner (Code Review) 04 Jun '21

04 Jun '21

Attention is currently required from: V Sowmya, Cliff Huang, Tim Wawrzynczak, Paul Menzel, Balaji Manigandan, Bernardo Perez Priego, Patrick Rudolph. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55127 ) Change subject: soc/intel/common/acpi: add ACPI S0ix _DSM for Intel Power Engine Plug-in ...................................................................... Patch Set 8: (5 comments) File src/soc/intel/common/block/acpi/Kconfig: https://review.coreboot.org/c/coreboot/+/55127/comment/59e5b485_6629563d PS8, Line 34: Generate S0ix entries for Intel power engine plug-in. Linux exposes this under `/sys/kernel/debug/pmc_core/substate_requirement_registers`. reflow, please File src/soc/intel/common/block/acpi/acpi/pep.asl: https://review.coreboot.org/c/coreboot/+/55127/comment/9b90d8d2_51a3aa89 PS8, Line 143: (( nit: no need for double braces https://review.coreboot.org/c/coreboot/+/55127/comment/760c144d_c761b514 PS8, Line 143: 57a6512e-3979-4e9d-9708-ff13b2508972 this UUID is only valid for >= TGL, isn't it? maybe add a comment to make this clear? https://review.coreboot.org/c/coreboot/+/55127/comment/9c976e1a_8a458b8b PS8, Line 145: ((Arg2 == Zero)) nit: no need for double braces https://review.coreboot.org/c/coreboot/+/55127/comment/3f9209f7_a5369b72 PS8, Line 148: ((Arg2 == 0x01)) nit: no need for double braces -- To view, visit https://review.coreboot.org/c/coreboot/+/55127 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I991662cbebf63bd71139ed37ff2588ba73f30398 Gerrit-Change-Number: 55127 Gerrit-PatchSet: 8 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Sukumar Ghorai <sukumar.ghorai(a)gmail.com> Gerrit-CC: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Attention: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 04 Jun 2021 18:24:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Correct TCSS XHCI Port status offset
by Sridhar Siricilla (Code Review) 04 Jun '21

04 Jun '21

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Subrata Banik, Balaji Manigandan, Deepti Deshatty, Patrick Rudolph. Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55230 ) Change subject: soc/intel/alderlake: Correct TCSS XHCI Port status offset ...................................................................... Patch Set 2: (1 comment) File src/soc/intel/alderlake/xhci.c: https://review.coreboot.org/c/coreboot/+/55230/comment/f68722d2_373cdf13 PS1, Line 18: 0x490 > My bad, i was looking at XHCI controller and you have updated TCSS XHCI. […] @Tim, I'm working with our internal teams for doc update. sorry for the confusion. -- To view, visit https://review.coreboot.org/c/coreboot/+/55230 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I20c77d78f52277a9a979e11303cdb6cdabae7c59 Gerrit-Change-Number: 55230 Gerrit-PatchSet: 2 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Attention: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 04 Jun 2021 18:24:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Refactor platform_fsp_silicon_init_params_cb fun...
by Subrata Banik (Code Review) 04 Jun '21

04 Jun '21

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55233 ) Change subject: soc/intel/alderlake: Refactor platform_fsp_silicon_init_params_cb function ...................................................................... soc/intel/alderlake: Refactor platform_fsp_silicon_init_params_cb function This patch create separate helper functions to fill-in required FSP-S UPDs as per IP initialization categories. This would help to increase the code readability and in future meaningful addition of FSP-S UPDs is possible rather adding UPDs randomly. Also, align FSP-S UPD update calling function to FSP-M UPD updater function. TEST=FSP-S UPD dump shows no change without and with this code change. Change-Id: Iba51aebc74456449e24e51e2f309f14f951464a0 Signed-off-by: Subrata Banik <subrata.banik(a)intel.com> --- M src/soc/intel/alderlake/fsp_params.c 1 file changed, 253 insertions(+), 130 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/55233/1 diff --git a/src/soc/intel/alderlake/fsp_params.c b/src/soc/intel/alderlake/fsp_params.c index 8664fe6..793b394 100644 --- a/src/soc/intel/alderlake/fsp_params.c +++ b/src/soc/intel/alderlake/fsp_params.c @@ -49,24 +49,6 @@ return ctl - 1; } -static void parse_devicetree(FSP_S_CONFIG *params) -{ - const struct soc_intel_alderlake_config *config; - config = config_of_soc(); - - for (int i = 0; i < CONFIG_SOC_INTEL_I2C_DEV_MAX; i++) - params->SerialIoI2cMode[i] = config->SerialIoI2cMode[i]; - - for (int i = 0; i < CONFIG_SOC_INTEL_COMMON_BLOCK_GSPI_MAX; i++) { - params->SerialIoSpiMode[i] = config->SerialIoGSpiMode[i]; - params->SerialIoSpiCsMode[i] = config->SerialIoGSpiCsMode[i]; - params->SerialIoSpiCsState[i] = config->SerialIoGSpiCsState[i]; - } - - for (int i = 0; i < CONFIG_SOC_INTEL_UART_DEV_MAX; i++) - params->SerialIoUartMode[i] = config->SerialIoUartMode[i]; -} - static const pci_devfn_t serial_io_dev[] = { PCH_DEVFN_I2C0, PCH_DEVFN_I2C1, @@ -88,149 +70,168 @@ /* Override settings per board. */ } -/* UPD parameters to be initialized before SiliconInit */ -void platform_fsp_silicon_init_params_cb(FSPS_UPD *supd) +static void fill_lpss_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) { - int i; + for (int i = 0; i < CONFIG_SOC_INTEL_I2C_DEV_MAX; i++) + s_cfg->SerialIoI2cMode[i] = config->SerialIoI2cMode[i]; + + for (int i = 0; i < CONFIG_SOC_INTEL_COMMON_BLOCK_GSPI_MAX; i++) { + s_cfg->SerialIoSpiMode[i] = config->SerialIoGSpiMode[i]; + s_cfg->SerialIoSpiCsMode[i] = config->SerialIoGSpiCsMode[i]; + s_cfg->SerialIoSpiCsState[i] = config->SerialIoGSpiCsState[i]; + } + + for (int i = 0; i < CONFIG_SOC_INTEL_UART_DEV_MAX; i++) + s_cfg->SerialIoUartMode[i] = config->SerialIoUartMode[i]; +} + +static void fill_cpu_silicon_init_params(FSP_S_CONFIG *s_cfg) +{ const struct microcode *microcode_file; size_t microcode_len; - FSP_S_CONFIG *params = &supd->FspsConfig; - FSPS_ARCH_UPD *pfsps_arch_upd = &supd->FspsArchUpd; - uint32_t enable_mask; - struct device *dev; - struct soc_intel_alderlake_config *config; - config = config_of_soc(); - mainboard_update_soc_chip_config(config); - - /* Parse device tree and enable/disable Serial I/O devices */ - parse_devicetree(params); - + /* Locate microcode and pass to FSP-S for 2nd microcode loading */ microcode_file = cbfs_map("cpu_microcode_blob.bin", &microcode_len); if ((microcode_file != NULL) && (microcode_len != 0)) { /* Update CPU Microcode patch base address/size */ - params->MicrocodeRegionBase = (uint32_t)microcode_file; - params->MicrocodeRegionSize = (uint32_t)microcode_len; + s_cfg->MicrocodeRegionBase = (uint32_t)microcode_file; + s_cfg->MicrocodeRegionSize = (uint32_t)microcode_len; } - /* Load VBT before devicetree-specific config. */ - params->GraphicsConfigPtr = (uintptr_t)vbt_get(); - - /* Check if IGD is present and fill Graphics init param accordingly */ - dev = pcidev_path_on_root(SA_DEVFN_IGD); - params->PeiGraphicsPeimInit = CONFIG(RUN_FSP_GOP) && is_dev_enabled(dev); - params->LidStatus = CONFIG(RUN_FSP_GOP); - /* Use coreboot MP PPI services if Kconfig is enabled */ if (CONFIG(USE_INTEL_FSP_TO_CALL_COREBOOT_PUBLISH_MP_PPI)) - params->CpuMpPpi = (uintptr_t) mp_fill_ppi_services_data(); + s_cfg->CpuMpPpi = (uintptr_t) mp_fill_ppi_services_data(); +} - /* D3Hot and D3Cold for TCSS */ - params->D3HotEnable = !config->TcssD3HotDisable; - params->D3ColdEnable = !config->TcssD3ColdDisable; +static void fill_igd_silicon_init_params(FSP_S_CONFIG *s_cfg) +{ + struct device *dev = pcidev_path_on_root(SA_DEVFN_IGD); + /* Load VBT before devicetree-specific config. */ + s_cfg->GraphicsConfigPtr = (uintptr_t)vbt_get(); - params->TcssAuxOri = config->TcssAuxOri; + /* Check if IGD is present and fill Graphics init param accordingly */ + dev = pcidev_path_on_root(SA_DEVFN_IGD); + s_cfg->PeiGraphicsPeimInit = CONFIG(RUN_FSP_GOP) && is_dev_enabled(dev); + s_cfg->LidStatus = CONFIG(RUN_FSP_GOP); +} + +static void fill_tcss_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ + s_cfg->TcssAuxOri = config->TcssAuxOri; /* Explicitly clear this field to avoid using defaults */ - memset(params->IomTypeCPortPadCfg, 0, sizeof(params->IomTypeCPortPadCfg)); + memset(s_cfg->IomTypeCPortPadCfg, 0, sizeof(s_cfg->IomTypeCPortPadCfg)); /* * Set FSPS UPD ITbtConnectTopologyTimeoutInMs with value 0. FSP will * evaluate this UPD value and skip sending command. There will be no * delay for command completion. */ - params->ITbtConnectTopologyTimeoutInMs = 0; + s_cfg->ITbtConnectTopologyTimeoutInMs = 0; + /* D3Hot and D3Cold for TCSS */ + s_cfg->D3HotEnable = !config->TcssD3HotDisable; + s_cfg->D3ColdEnable = !config->TcssD3ColdDisable; +} + +static void fill_lockdown_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ /* Chipset Lockdown */ if (get_lockdown_config() == CHIPSET_LOCKDOWN_COREBOOT) { - params->PchLockDownGlobalSmi = 0; - params->PchLockDownBiosInterface = 0; - params->PchUnlockGpioPads = 1; - params->RtcMemoryLock = 0; + s_cfg->PchLockDownGlobalSmi = 0; + s_cfg->PchLockDownBiosInterface = 0; + s_cfg->PchUnlockGpioPads = 1; + s_cfg->RtcMemoryLock = 0; } else { - params->PchLockDownGlobalSmi = 1; - params->PchLockDownBiosInterface = 1; - params->PchUnlockGpioPads = 0; - params->RtcMemoryLock = 1; + s_cfg->PchLockDownGlobalSmi = 1; + s_cfg->PchLockDownBiosInterface = 1; + s_cfg->PchUnlockGpioPads = 0; + s_cfg->RtcMemoryLock = 1; } +} +static void fill_xhci_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ + int i; /* USB */ for (i = 0; i < ARRAY_SIZE(config->usb2_ports); i++) { - params->PortUsb20Enable[i] = config->usb2_ports[i].enable; - params->Usb2PhyPetxiset[i] = config->usb2_ports[i].pre_emp_bias; - params->Usb2PhyTxiset[i] = config->usb2_ports[i].tx_bias; - params->Usb2PhyPredeemp[i] = config->usb2_ports[i].tx_emp_enable; - params->Usb2PhyPehalfbit[i] = config->usb2_ports[i].pre_emp_bit; + s_cfg->PortUsb20Enable[i] = config->usb2_ports[i].enable; + s_cfg->Usb2PhyPetxiset[i] = config->usb2_ports[i].pre_emp_bias; + s_cfg->Usb2PhyTxiset[i] = config->usb2_ports[i].tx_bias; + s_cfg->Usb2PhyPredeemp[i] = config->usb2_ports[i].tx_emp_enable; + s_cfg->Usb2PhyPehalfbit[i] = config->usb2_ports[i].pre_emp_bit; if (config->usb2_ports[i].enable) - params->Usb2OverCurrentPin[i] = config->usb2_ports[i].ocpin; + s_cfg->Usb2OverCurrentPin[i] = config->usb2_ports[i].ocpin; else - params->Usb2OverCurrentPin[i] = OC_SKIP; + s_cfg->Usb2OverCurrentPin[i] = OC_SKIP; } for (i = 0; i < ARRAY_SIZE(config->usb3_ports); i++) { - params->PortUsb30Enable[i] = config->usb3_ports[i].enable; + s_cfg->PortUsb30Enable[i] = config->usb3_ports[i].enable; if (config->usb3_ports[i].enable) - params->Usb3OverCurrentPin[i] = config->usb3_ports[i].ocpin; + s_cfg->Usb3OverCurrentPin[i] = config->usb3_ports[i].ocpin; else - params->Usb3OverCurrentPin[i] = OC_SKIP; + s_cfg->Usb3OverCurrentPin[i] = OC_SKIP; if (config->usb3_ports[i].tx_de_emp) { - params->Usb3HsioTxDeEmphEnable[i] = 1; - params->Usb3HsioTxDeEmph[i] = config->usb3_ports[i].tx_de_emp; + s_cfg->Usb3HsioTxDeEmphEnable[i] = 1; + s_cfg->Usb3HsioTxDeEmph[i] = config->usb3_ports[i].tx_de_emp; } if (config->usb3_ports[i].tx_downscale_amp) { - params->Usb3HsioTxDownscaleAmpEnable[i] = 1; - params->Usb3HsioTxDownscaleAmp[i] = + s_cfg->Usb3HsioTxDownscaleAmpEnable[i] = 1; + s_cfg->Usb3HsioTxDownscaleAmp[i] = config->usb3_ports[i].tx_downscale_amp; } } for (i = 0; i < ARRAY_SIZE(config->tcss_ports); i++) { if (config->tcss_ports[i].enable) - params->CpuUsb3OverCurrentPin[i] = config->tcss_ports[i].ocpin; + s_cfg->CpuUsb3OverCurrentPin[i] = config->tcss_ports[i].ocpin; } - /* EnableMultiPhaseSiliconInit for running MultiPhaseSiInit */ - pfsps_arch_upd->EnableMultiPhaseSiliconInit = 1; +} +static void fill_xdci_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ + struct device *dev = pcidev_path_on_root(PCH_DEVFN_USBOTG); /* Enable xDCI controller if enabled in devicetree and allowed */ - dev = pcidev_path_on_root(PCH_DEVFN_USBOTG); if (dev) { if (!xdci_can_enable()) dev->enabled = 0; - params->XdciEnable = dev->enabled; + s_cfg->XdciEnable = dev->enabled; } else { - params->XdciEnable = 0; + s_cfg->XdciEnable = 0; } +} + +static void fill_uart_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ /* PCH UART selection for FSP Debug */ - params->SerialIoDebugUartNumber = CONFIG_UART_FOR_CONSOLE; - ASSERT(ARRAY_SIZE(params->SerialIoUartAutoFlow) > CONFIG_UART_FOR_CONSOLE); - params->SerialIoUartAutoFlow[CONFIG_UART_FOR_CONSOLE] = 0; + s_cfg->SerialIoDebugUartNumber = CONFIG_UART_FOR_CONSOLE; + ASSERT(ARRAY_SIZE(s_cfg->SerialIoUartAutoFlow) > CONFIG_UART_FOR_CONSOLE); + s_cfg->SerialIoUartAutoFlow[CONFIG_UART_FOR_CONSOLE] = 0; +} - /* SATA */ - dev = pcidev_path_on_root(PCH_DEVFN_SATA); - params->SataEnable = is_dev_enabled(dev); - if (params->SataEnable) { - params->SataMode = config->SataMode; - params->SataSalpSupport = config->SataSalpSupport; - memcpy(params->SataPortsEnable, config->SataPortsEnable, - sizeof(params->SataPortsEnable)); - memcpy(params->SataPortsDevSlp, config->SataPortsDevSlp, - sizeof(params->SataPortsDevSlp)); - } - +static void configure_sata_port_power(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ + int i; /* - * Power Optimizer for DMI and SATA. - * DmiPwrOptimizeDisable and SataPwrOptimizeDisable is default to 0. + * Power Optimizer for SATA. + * SataPwrOptimizeDisable is default to 0. * Boards not needing the optimizers explicitly disables them by setting * these disable variables to 1 in devicetree overrides. */ - params->PchPwrOptEnable = !(config->DmiPwrOptimizeDisable); - params->SataPwrOptEnable = !(config->SataPwrOptimizeDisable); - + s_cfg->SataPwrOptEnable = !(config->SataPwrOptimizeDisable); /* * Enable DEVSLP Idle Timeout settings DmVal and DitoVal. * SataPortsDmVal is the DITO multiplier. Default is 15. @@ -239,69 +240,191 @@ */ for (i = 0; i < ARRAY_SIZE(config->SataPortsEnableDitoConfig); i++) { if (config->SataPortsEnableDitoConfig[i]) { - params->SataPortsDmVal[i] = config->SataPortsDmVal[i]; - params->SataPortsDitoVal[i] = config->SataPortsDitoVal[i]; + s_cfg->SataPortsDmVal[i] = config->SataPortsDmVal[i]; + s_cfg->SataPortsDitoVal[i] = config->SataPortsDitoVal[i]; } } +} +static void fill_sata_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ + /* SATA */ + struct device *dev = pcidev_path_on_root(PCH_DEVFN_SATA); + s_cfg->SataEnable = is_dev_enabled(dev); + if (s_cfg->SataEnable) { + s_cfg->SataMode = config->SataMode; + s_cfg->SataSalpSupport = config->SataSalpSupport; + memcpy(s_cfg->SataPortsEnable, config->SataPortsEnable, + sizeof(s_cfg->SataPortsEnable)); + memcpy(s_cfg->SataPortsDevSlp, config->SataPortsDevSlp, + sizeof(s_cfg->SataPortsDevSlp)); + } + + configure_sata_port_power(s_cfg, config); +} + +static void fill_thermal_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ /* Enable TCPU for processor thermal control */ - dev = pcidev_path_on_root(SA_DEVFN_DPTF); - params->Device4Enable = is_dev_enabled(dev); + struct device *dev = pcidev_path_on_root(SA_DEVFN_DPTF); + s_cfg->Device4Enable = is_dev_enabled(dev); /* Set TccActivationOffset */ - params->TccActivationOffset = config->tcc_offset; + s_cfg->TccActivationOffset = config->tcc_offset; +} +static void fill_lan_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ /* LAN */ - dev = pcidev_path_on_root(PCH_DEVFN_GBE); - params->PchLanEnable = is_dev_enabled(dev); + struct device *dev = pcidev_path_on_root(PCH_DEVFN_GBE); + s_cfg->PchLanEnable = is_dev_enabled(dev); +} +static void fill_cnvi_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ /* CNVi */ - dev = pcidev_path_on_root(PCH_DEVFN_CNVI_WIFI); - params->CnviMode = is_dev_enabled(dev); - params->CnviBtCore = config->CnviBtCore; - params->CnviBtAudioOffload = config->CnviBtAudioOffload; + struct device *dev = pcidev_path_on_root(PCH_DEVFN_CNVI_WIFI); + s_cfg->CnviMode = is_dev_enabled(dev); + s_cfg->CnviBtCore = config->CnviBtCore; + s_cfg->CnviBtAudioOffload = config->CnviBtAudioOffload; /* Assert if CNVi BT is enabled without CNVi being enabled. */ - assert(params->CnviMode || !params->CnviBtCore); + assert(s_cfg->CnviMode || !s_cfg->CnviBtCore); /* Assert if CNVi BT offload is enabled without CNVi BT being enabled. */ - assert(params->CnviBtCore || !params->CnviBtAudioOffload); + assert(s_cfg->CnviBtCore || !s_cfg->CnviBtAudioOffload); +} +static void fill_vmd_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ /* VMD */ - dev = pcidev_path_on_root(SA_DEVFN_VMD); - params->VmdEnable = is_dev_enabled(dev); + struct device *dev = pcidev_path_on_root(SA_DEVFN_VMD); + s_cfg->VmdEnable = is_dev_enabled(dev); +} +static void fill_thc_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ /* THC */ - dev = pcidev_path_on_root(PCH_DEVFN_THC0); - params->ThcPort0Assignment = is_dev_enabled(dev) ? THC_0 : THC_NONE; + struct device *dev = pcidev_path_on_root(PCH_DEVFN_THC0); + s_cfg->ThcPort0Assignment = is_dev_enabled(dev) ? THC_0 : THC_NONE; dev = pcidev_path_on_root(PCH_DEVFN_THC1); - params->ThcPort1Assignment = is_dev_enabled(dev) ? THC_1 : THC_NONE; + s_cfg->ThcPort1Assignment = is_dev_enabled(dev) ? THC_1 : THC_NONE; +} +static void fill_8254_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ /* Legacy 8254 timer support */ - params->Enable8254ClockGating = !CONFIG(USE_LEGACY_8254_TIMER); - params->Enable8254ClockGatingOnS3 = !CONFIG(USE_LEGACY_8254_TIMER); + s_cfg->Enable8254ClockGating = !CONFIG(USE_LEGACY_8254_TIMER); + s_cfg->Enable8254ClockGatingOnS3 = !CONFIG(USE_LEGACY_8254_TIMER); +} +static void fill_storage_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ /* Enable Hybrid storage auto detection */ - params->HybridStorageMode = config->HybridStorageMode; + s_cfg->HybridStorageMode = config->HybridStorageMode; +} - enable_mask = pcie_rp_enable_mask(get_pch_pcie_rp_table()); +static void fill_pcie_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ + int i; + uint32_t enable_mask = pcie_rp_enable_mask(get_pch_pcie_rp_table()); for (i = 0; i < CONFIG_MAX_PCH_ROOT_PORTS; i++) { if (!(enable_mask & BIT(i))) continue; const struct pcie_rp_config *rp_cfg = &config->pch_pcie_rp[i]; - params->PcieRpL1Substates[i] = + s_cfg->PcieRpL1Substates[i] = get_l1_substate_control(rp_cfg->PcieRpL1Substates); - params->PcieRpLtrEnable[i] = !!(rp_cfg->flags & PCIE_RP_LTR); - params->PcieRpAdvancedErrorReporting[i] = !!(rp_cfg->flags & PCIE_RP_AER); - params->PcieRpHotPlug[i] = !!(rp_cfg->flags & PCIE_RP_HOTPLUG); - params->PcieRpClkReqDetect[i] = !!(rp_cfg->flags & PCIE_RP_CLK_REQ_DETECT); + s_cfg->PcieRpLtrEnable[i] = !!(rp_cfg->flags & PCIE_RP_LTR); + s_cfg->PcieRpAdvancedErrorReporting[i] = !!(rp_cfg->flags & PCIE_RP_AER); + s_cfg->PcieRpHotPlug[i] = !!(rp_cfg->flags & PCIE_RP_HOTPLUG); + s_cfg->PcieRpClkReqDetect[i] = !!(rp_cfg->flags & PCIE_RP_CLK_REQ_DETECT); } +} - params->PmSupport = 1; - params->Hwp = 1; - params->Cx = 1; - params->PsOnEnable = 1; +static void fill_misc_power_silicon_init_params(FSP_S_CONFIG *s_cfg, + const struct soc_intel_alderlake_config *config) +{ + /* + * Power Optimizer for DMI + * DmiPwrOptimizeDisable is default to 0. + * Boards not needing the optimizers explicitly disables them by setting + * these disable variables to 1 in devicetree overrides. + */ + s_cfg->PchPwrOptEnable = !(config->DmiPwrOptimizeDisable); + s_cfg->PmSupport = 1; + s_cfg->Hwp = 1; + s_cfg->Cx = 1; + s_cfg->PsOnEnable = 1; +} - mainboard_silicon_init_params(params); +static void soc_silicon_init_params(FSP_S_CONFIG *s_cfg, + struct soc_intel_alderlake_config *config) +{ + /* Override settings per board if required. */ + mainboard_update_soc_chip_config(config); + + /* Parse device tree and fill LPSS related FSP-S UPDs */ + fill_lpss_silicon_init_params(s_cfg, config); + + /* Fill CPU related FSP-S UPDs */ + fill_cpu_silicon_init_params(s_cfg); + + /* Fill IGD related FSP-M UPDs */ + fill_igd_silicon_init_params(s_cfg); + + fill_tcss_silicon_init_params(s_cfg, config); + + fill_lockdown_silicon_init_params(s_cfg, config); + + fill_xhci_silicon_init_params(s_cfg, config); + + fill_xdci_silicon_init_params(s_cfg, config); + + fill_uart_silicon_init_params(s_cfg, config); + + fill_sata_silicon_init_params(s_cfg, config); + + fill_thermal_silicon_init_params(s_cfg, config); + + fill_lan_silicon_init_params(s_cfg, config); + + fill_cnvi_silicon_init_params(s_cfg, config); + + fill_vmd_silicon_init_params(s_cfg, config); + + fill_thc_silicon_init_params(s_cfg, config); + + fill_8254_silicon_init_params(s_cfg, config); + + fill_storage_silicon_init_params(s_cfg, config); + + fill_pcie_silicon_init_params(s_cfg, config); + + fill_misc_power_silicon_init_params(s_cfg, config); +} + +/* UPD parameters to be initialized before SiliconInit */ +void platform_fsp_silicon_init_params_cb(FSPS_UPD *supd) +{ + struct soc_intel_alderlake_config *config; + FSP_S_CONFIG *s_cfg = &supd->FspsConfig; + FSPS_ARCH_UPD *s_arch_upd = &supd->FspsArchUpd; + + config = config_of_soc(); + + /* EnableMultiPhaseSiliconInit for running MultiPhaseSiInit */ + s_arch_upd->EnableMultiPhaseSiliconInit = 1; + + soc_silicon_init_params(s_cfg, config); + mainboard_silicon_init_params(s_cfg); } /* @@ -330,7 +453,7 @@ } /* Mainboard GPIO Configuration */ -__weak void mainboard_silicon_init_params(FSP_S_CONFIG *params) +__weak void mainboard_silicon_init_params(FSP_S_CONFIG *s_cfg) { printk(BIOS_DEBUG, "WEAK: %s/%s called\n", __FILE__, __func__); } -- To view, visit https://review.coreboot.org/c/coreboot/+/55233 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iba51aebc74456449e24e51e2f309f14f951464a0 Gerrit-Change-Number: 55233 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/intel/alderlake: Correct TCSS XHCI Port status offset
by Subrata Banik (Code Review) 04 Jun '21

04 Jun '21

Attention is currently required from: Maulik V Vaghela, Tim Wawrzynczak, Sridhar Siricilla, Balaji Manigandan, Deepti Deshatty, Patrick Rudolph. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55230 ) Change subject: soc/intel/alderlake: Correct TCSS XHCI Port status offset ...................................................................... Patch Set 2: (1 comment) File src/soc/intel/alderlake/xhci.c: https://review.coreboot.org/c/coreboot/+/55230/comment/e380186f_379bf42f PS1, Line 18: 0x490 > @Subrata, "Port 1 SC" refers USB2 port status, and USB3 port status starts from 0x490. […] My bad, i was looking at XHCI controller and you have updated TCSS XHCI. Looks good to me -- To view, visit https://review.coreboot.org/c/coreboot/+/55230 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I20c77d78f52277a9a979e11303cdb6cdabae7c59 Gerrit-Change-Number: 55230 Gerrit-PatchSet: 2 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Reviewer: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-Attention: Deepti Deshatty <deepti.deshatty(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 04 Jun 2021 18:20:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/starlabs/labtop: Unbreak master
by Tim Wawrzynczak (Code Review) 04 Jun '21

04 Jun '21

Attention is currently required from: Nico Huber. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55231 ) Change subject: mb/starlabs/labtop: Unbreak master ...................................................................... Patch Set 3: (1 comment) File src/mainboard/starlabs/labtop/mainboard.c: https://review.coreboot.org/c/coreboot/+/55231/comment/a304b3fc_ce6a44c0 PS1, Line 85: } > This looks like the default implementation. I don't think the author […] Good point -- To view, visit https://review.coreboot.org/c/coreboot/+/55231 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I379880b1165bb9da4117aa860dee111fba38a43f Gerrit-Change-Number: 55231 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Star Labs <admin(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Fri, 04 Jun 2021 18:11:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/starlabs/labtop: Unbreak master
by Tim Wawrzynczak (Code Review) 04 Jun '21

04 Jun '21

Attention is currently required from: Tim Wawrzynczak. Hello build bot (Jenkins), Star Labs, Patrick Georgi, Angel Pons, Michael Niewöhner, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/55231 to look at the new patch set (#3). Change subject: mb/starlabs/labtop: Unbreak master ...................................................................... mb/starlabs/labtop: Unbreak master Missed a few things in review: 1) smbios_mainboard_serial_number 2) SMBIOS_FEATURE_FLAGS_HOSTING_BOARD 3) SMBIOS_FEATURE_FLAGS_REPLACEABLE Change-Id: I379880b1165bb9da4117aa860dee111fba38a43f Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/mainboard/starlabs/labtop/mainboard.c 1 file changed, 1 insertion(+), 16 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/55231/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/55231 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I379880b1165bb9da4117aa860dee111fba38a43f Gerrit-Change-Number: 55231 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Star Labs <admin(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2021