coreboot-gerrit September 2024

coreboot-gerrit@coreboot.org

1 participants
3342 discussions

[S] Change in coreboot[main]: libpayload: add more condition to check valid PCI device id
by Subrata Banik (Code Review) 07 Sep '24

07 Sep '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Jamie Ryu, Jeremy Compostella, Wonkyu Kim. Subrata Banik has posted comments on this change by Wonkyu Kim. ( https://review.coreboot.org/c/coreboot/+/84229?usp=email ) Change subject: libpayload: add more condition to check valid PCI device id ...................................................................... Patch Set 3: (3 comments) File payloads/libpayload/drivers/usb/usbinit.c: https://review.coreboot.org/c/coreboot/+/84229/comment/3bba86af_4b3375b3?us… : PS3, Line 126: /* Check if there's a device here at all. */ : if (pci_read_config32(pci_device, REG_VENDOR_ID) == 0xffffffff) please specify the device where you have ran into this issue. IMO, checking VID itself for 0xFFFF is enough as per PCI spec. we don't need to check both VID:DID. Are you seeing platform device, that has valid VID and NULL DID? This sounds to me a bad hardware or configuration. https://review.coreboot.org/c/coreboot/+/84229/comment/6a228548_a9882b7c?us… : PS3, Line 141: why ? https://review.coreboot.org/c/coreboot/+/84229/comment/8eb0fbbf_e73b09ac?us… : PS3, Line 147: space ? -- To view, visit https://review.coreboot.org/c/coreboot/+/84229?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I772c4199c7a6c13a25590a36f1bfee17c1a44daf Gerrit-Change-Number: 84229 Gerrit-PatchSet: 3 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Jeremy Compostella <jeremy.compostella(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jeremy Compostella <jeremy.compostella(a)gmail.com> Gerrit-Comment-Date: Sat, 07 Sep 2024 08:57:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: make same cpu pyhsical address
by Subrata Banik (Code Review) 07 Sep '24

07 Sep '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Cliff Huang, Jamie Ryu, Jérémy Compostella, Lance Zhao, Tim Wawrzynczak, Wonkyu Kim. Subrata Banik has posted comments on this change by Wonkyu Kim. ( https://review.coreboot.org/c/coreboot/+/84230?usp=email ) Change subject: make same cpu pyhsical address ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/84230/comment/825182ed_cd98157c?us… : PS2, Line 7: make same cpu pyhsical address : : 2 funtions are used to get cpu physical address like below. : cpu_phys_address_size : soc_phys_address_size : And If CONFIG_SOC_PHYSICAL_ADDRESS_WIDTH is defined, : it can return different address size like below : example) : coreboot log: CPU physical address size: 46 bits : OS log : DMAR: Host address width 42 : So, need to make one API to use same physical address. : you really need to write a better commit msg to understand the problem. I believe this is refactoring of the code to use single API rather mix between CPU and SoC. I thought SoC was implemented for override, like we saw during Vpro SKU in MTL -- To view, visit https://review.coreboot.org/c/coreboot/+/84230?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I06c485511fa7f72a445c8aed56d8a470b4772092 Gerrit-Change-Number: 84230 Gerrit-PatchSet: 2 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Comment-Date: Sat, 07 Sep 2024 08:55:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: src/device: Add more condition to check valid PCI device id
by Subrata Banik (Code Review) 07 Sep '24

07 Sep '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Jamie Ryu, Jeremy Compostella, Wonkyu Kim. Subrata Banik has posted comments on this change by Wonkyu Kim. ( https://review.coreboot.org/c/coreboot/+/84228?usp=email ) Change subject: src/device: Add more condition to check valid PCI device id ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/84228/comment/4e51ec53_e96571ba?us… : PS2, Line 12: Below are invalid PCI device id cases : VID: 0x0 or 0xffff : DID: 0x0 or 0xffff can you please highlight why some device missed to update VID/DID. Best to my knowledge it only happens for simics device and not actual device. what is the problem that you have faced here ? looking at the existing code, I felt it only checks for VID and not DID. Are you seeing some issue when VID is valid but DID is zero/0xff? -- To view, visit https://review.coreboot.org/c/coreboot/+/84228?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iffabc9037a8af1b2a4ffebdf30199c4f6eae9540 Gerrit-Change-Number: 84228 Gerrit-PatchSet: 2 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Jeremy Compostella <jeremy.compostella(a)gmail.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jeremy Compostella <jeremy.compostella(a)gmail.com> Gerrit-Comment-Date: Sat, 07 Sep 2024 08:53:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in coreboot[main]: src/intel/cmm/block: Update sa_get_tseg_size function
by Subrata Banik (Code Review) 07 Sep '24

07 Sep '24

Attention is currently required from: Anil Kumar K, Bora Guvendik, Jamie Ryu, Jeremy Compostella, Wonkyu Kim. Subrata Banik has posted comments on this change by Wonkyu Kim. ( https://review.coreboot.org/c/coreboot/+/84210?usp=email ) Change subject: src/intel/cmm/block: Update sa_get_tseg_size function ...................................................................... Patch Set 2: (1 comment) File src/soc/intel/common/block/systemagent/systemagent_early.c: https://review.coreboot.org/c/coreboot/+/84210/comment/332cab5b_3717ae88?us… : PS2, Line 155: return sa_get_gsm_base() - sa_get_tseg_base(); WDYT? I assume in absence of IGD the GSM size would be zero hence, we won't get into `if` case. ``` if (sa_get_gsm_size() > 0) return sa_get_gsm_base() - sa_get_tseg_base(); else return CONFIG_SMM_TSEG_SIZE; ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/84210?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic3d5175add1f7dc6e2c9f1c38133b36ffc59e789 Gerrit-Change-Number: 84210 Gerrit-PatchSet: 2 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Jeremy Compostella <jeremy.compostella(a)gmail.com> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jeremy Compostella <jeremy.compostella(a)gmail.com> Gerrit-Comment-Date: Sat, 07 Sep 2024 08:49:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XL] Change in coreboot[main]: soc/intel/ptl: Do initial Panther Lake SoC commit till ramstage
by Subrata Banik (Code Review) 07 Sep '24

07 Sep '24

Attention is currently required from: Cliff Huang, Jérémy Compostella, Kapil Porwal, Krishna P Bhat D, Pranava Y N, Saurabh Mishra. Subrata Banik has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83798?usp=email ) Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till ramstage ...................................................................... Patch Set 90: (1 comment) File src/soc/intel/pantherlake/tcss.c: https://review.coreboot.org/c/coreboot/+/83798/comment/7162c08d_5d843575?us… : PS48, Line 6: const struct soc_tcss_ops tcss_ops = { > Added to discuss more in TODO bug. Currently using skeleton code. Please don't use the SoC code clean up bug as dump-yard so that you can keep empty structure in PTL SoC code. That is never the purpose of SoC clean up bug. You should be use a simple software engineering practice like below to keep the skeleton code (and not the code that you call it as skeleton) ``` const struct soc_tcss_ops tcss_ops = { + /* TODO: Implement AUX BIAS PAD Programming if required */ + .configure_aux_bias_pads = NULL, + .valid_tbt_auth = NULL, }; ``` The caller function would avoid making a call into a NULL function ``` if (tcss_ops.valid_tbt_auth && !tcss_ops.valid_tbt_auth()) return; ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/83798?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Idc6fb11e9e84c28c7567ae2b7abc1ab832a88362 Gerrit-Change-Number: 83798 Gerrit-PatchSet: 90 Gerrit-Owner: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-CC: Cliff Huang <cliff.huang(a)intel.com> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-CC: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com> Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com> Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com> Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Sat, 07 Sep 2024 08:43:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Saurabh Mishra <mishra.saurabh(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>

1 0

[L] Change in coreboot[main]: ec/hp/kbc1126/acpi: Document EC RAM fields
by Nicholas Sudsgaard (Code Review) 07 Sep '24

07 Sep '24

Attention is currently required from: Iru Cai. Hello Iru Cai, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84242?usp=email to look at the new patch set (#2). Change subject: ec/hp/kbc1126/acpi: Document EC RAM fields ...................................................................... ec/hp/kbc1126/acpi: Document EC RAM fields This is not perfect, as documentation for some fields are missing or speculative. However, I think it is enough to be a good starting point. This was done by only analyzing the ACPI dump from vendor firmware, we could understand more by looking deeper into the vendor firmware, but this would take more time and effort. I'll leave that as a task for me or somebody else in the future. Change-Id: Ic411a7a821d7b03be45d4bda645a03f82c7241c5 Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> --- M src/ec/hp/kbc1126/acpi/ac.asl M src/ec/hp/kbc1126/acpi/battery.asl M src/ec/hp/kbc1126/acpi/ec.asl 3 files changed, 178 insertions(+), 149 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/84242/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84242?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic411a7a821d7b03be45d4bda645a03f82c7241c5 Gerrit-Change-Number: 84242 Gerrit-PatchSet: 2 Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: Iru Cai <mytbk920423(a)gmail.com> Gerrit-Attention: Iru Cai <mytbk920423(a)gmail.com>

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Consolidate `BUILDING_WITH_DEBUG_FSP` option
by Subrata Banik (Code Review) 07 Sep '24

07 Sep '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/84220?usp=email ) Change subject: drivers/intel/fsp2_0: Consolidate `BUILDING_WITH_DEBUG_FSP` option ...................................................................... drivers/intel/fsp2_0: Consolidate `BUILDING_WITH_DEBUG_FSP` option Move the `BUILDING_WITH_DEBUG_FSP` Kconfig option from SoC-specific files to the FSP2_0 driver Kconfig to avoid duplication. Also slightly improves the option's prompt and help text. TEST=Built and booted google/rex successfully. Change-Id: I5c3dce59c396f6c1665a3ed1b8c1bb5df0f5a8d4 Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/84220 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Eric Lai <ericllai(a)google.com> --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/alderlake/Kconfig M src/soc/intel/meteorlake/Kconfig M src/soc/intel/pantherlake/Kconfig 4 files changed, 7 insertions(+), 18 deletions(-) Approvals: build bot (Jenkins): Verified Eric Lai: Looks good to me, approved diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index bbf0710..8e9dcdc 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -533,4 +533,11 @@ ChromeOS devices typically do not require the MBP information, hence it is disabled by default on ChromeOS. +config BUILDING_WITH_DEBUG_FSP + bool "Use Debug FSP for Build" + default n + help + Enable this option if you are using a debug build of the FSP (Firmware Support Package) + in your project. + endif diff --git a/src/soc/intel/alderlake/Kconfig b/src/soc/intel/alderlake/Kconfig index c1cc993..2c441fc 100644 --- a/src/soc/intel/alderlake/Kconfig +++ b/src/soc/intel/alderlake/Kconfig @@ -493,12 +493,6 @@ help Size of Descriptor Region in the FMAP -config BUILDING_WITH_DEBUG_FSP - bool "Debug FSP is used for the build" - default n - help - Set this option if debug build of FSP is used. - config INTEL_GMA_BCLV_OFFSET default 0xc8258 diff --git a/src/soc/intel/meteorlake/Kconfig b/src/soc/intel/meteorlake/Kconfig index 753af7c..b5c0bcf 100644 --- a/src/soc/intel/meteorlake/Kconfig +++ b/src/soc/intel/meteorlake/Kconfig @@ -380,12 +380,6 @@ hex default 0x800000 -config BUILDING_WITH_DEBUG_FSP - bool "Debug FSP is used for the build" - default n - help - Set this option if debug build of FSP is used. - config DROP_CPU_FEATURE_PROGRAM_IN_FSP bool default y if MP_SERVICES_PPI_V2_NOOP || CHROMEOS diff --git a/src/soc/intel/pantherlake/Kconfig b/src/soc/intel/pantherlake/Kconfig index e5f7c92..98248fe 100644 --- a/src/soc/intel/pantherlake/Kconfig +++ b/src/soc/intel/pantherlake/Kconfig @@ -242,10 +242,4 @@ int default 16 -config BUILDING_WITH_DEBUG_FSP - bool "Debug FSP is used for the build" - default n - help - Set this option if debug build of FSP is used. - endif -- To view, visit https://review.coreboot.org/c/coreboot/+/84220?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5c3dce59c396f6c1665a3ed1b8c1bb5df0f5a8d4 Gerrit-Change-Number: 84220 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Add Kconfig option to control MBP HOB creation
by Subrata Banik (Code Review) 07 Sep '24

07 Sep '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/84217?usp=email ) Change subject: drivers/intel/fsp2_0: Add Kconfig option to control MBP HOB creation ...................................................................... drivers/intel/fsp2_0: Add Kconfig option to control MBP HOB creation This patch adds a new Kconfig option `FSP_PUBLISH_MBP_HOB` to control the creation of the ME_BIOS_PAYLOAD_HOB (MBP HOB) by FSP. Disabling this option can improve boot time on platforms that do not utilize the MBP HOB, such as ChromeOS devices. The option is disabled by default on ChromeOS and enabled by default on other platforms. On ADL-P based platforms, this option is forced to be enabled as ADL-P FSP relies on MBP HOB for ChipsetInit version for ChipsetInit sync. Removed SoC specific implementation of `FSP_PUBLISH_MBP_HOB` config from MTL and TGL config file. TEST=Tested on ADL-P and ADL-N platforms. Verified that MBP HOB is created when `FSP_PUBLISH_MBP_HOB` is enabled and not created when it is disabled. Also verified that the system boots successfully in both cases. Change-Id: I21da00259c0b9bcca6f545291a6259e9cce8d900 Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/84217 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Eric Lai <ericllai(a)google.com> --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/alderlake/Kconfig M src/soc/intel/meteorlake/Kconfig M src/soc/intel/pantherlake/Kconfig 4 files changed, 16 insertions(+), 26 deletions(-) Approvals: build bot (Jenkins): Verified Eric Lai: Looks good to me, approved diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index 2ad3209..bbf0710 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -522,4 +522,15 @@ reported with Alder Lake and Raptor Lake FSP where MultiPhaseSiInit API is unable to return any ERROR status. +config FSP_PUBLISH_MBP_HOB + bool + default n if CHROMEOS + default y + help + This is to control creation of ME_BIOS_PAYLOAD_HOB (MBP HOB) by FSP. + Disabling it for the platforms, which do not use MBP HOB, can improve the boot time. + + ChromeOS devices typically do not require the MBP information, hence it is disabled + by default on ChromeOS. + endif diff --git a/src/soc/intel/alderlake/Kconfig b/src/soc/intel/alderlake/Kconfig index f7c585a..c1cc993 100644 --- a/src/soc/intel/alderlake/Kconfig +++ b/src/soc/intel/alderlake/Kconfig @@ -511,18 +511,13 @@ config INTEL_GMA_BCLM_WIDTH default 32 +# Note: It cannot be disabled for ADL-P based platforms because ADL-P FSP relies on +# MBP HOB for ChipsetInit version for ChipsetInit sync. As ChipsetInit sync doesn't +# occur if no MBP HOB, so it results S0ix issue. This limitation is addressed in the +# later platforms so creation of MBP HOB can be skipped for ADL-N based platforms. config FSP_PUBLISH_MBP_HOB bool - default n if CHROMEOS && (SOC_INTEL_ALDERLAKE_PCH_N) - default y - help - This is to control creation of ME_BIOS_PAYLOAD_HOB (MBP HOB) by FSP. - Disabling it for the platforms, which do not use MBP HOB, can improve the boot time. - - Note: It cannot be disabled for ADL-P based platforms because ADL-P FSP relies on - MBP HOB for ChipsetInit version for ChipsetInit sync. As ChipsetInit sync doesn't - occur if no MBP HOB, so it results S0ix issue. This limitation is addressed in the - later platforms so creation of MBP HOB can be skipped for ADL-N based platforms. + default y if !SOC_INTEL_ALDERLAKE_PCH_N config INCLUDE_HSPHY_IN_FMAP bool "Include PCIe 5.0 HSPHY firmware in flash" diff --git a/src/soc/intel/meteorlake/Kconfig b/src/soc/intel/meteorlake/Kconfig index 79ef3e8..753af7c 100644 --- a/src/soc/intel/meteorlake/Kconfig +++ b/src/soc/intel/meteorlake/Kconfig @@ -380,14 +380,6 @@ hex default 0x800000 -config FSP_PUBLISH_MBP_HOB - bool - default n if CHROMEOS - default y - help - This is to control creation of ME_BIOS_PAYLOAD_HOB (MBP HOB) by FSP. - Disabling it for the platforms, which do not use MBP HOB, can improve the boot time. - config BUILDING_WITH_DEBUG_FSP bool "Debug FSP is used for the build" default n diff --git a/src/soc/intel/pantherlake/Kconfig b/src/soc/intel/pantherlake/Kconfig index 9a5fc61..e5f7c92 100644 --- a/src/soc/intel/pantherlake/Kconfig +++ b/src/soc/intel/pantherlake/Kconfig @@ -248,12 +248,4 @@ help Set this option if debug build of FSP is used. -config FSP_PUBLISH_MBP_HOB - bool - default n if CHROMEOS - default y - help - This is to control creation of ME_BIOS_PAYLOAD_HOB (MBP HOB) by FSP. - Disabling it for the platforms, which do not use MBP HOB, can improve the boot time. - endif -- To view, visit https://review.coreboot.org/c/coreboot/+/84217?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I21da00259c0b9bcca6f545291a6259e9cce8d900 Gerrit-Change-Number: 84217 Gerrit-PatchSet: 3 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Rishika Raj <rishikaraj(a)google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[L] Change in coreboot[main]: ec/hp/kbc1126/acpi: Document EC RAM fields
by Nicholas Sudsgaard (Code Review) 07 Sep '24

07 Sep '24

Nicholas Sudsgaard has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84242?usp=email ) Change subject: ec/hp/kbc1126/acpi: Document EC RAM fields ...................................................................... ec/hp/kbc1126/acpi: Document EC RAM fields This is not perfect, as documentation for some fields are missing or speculative. However, I think it is enough to be a good starting point. This was done by only analyzing the ACPI dump from vendor firmware, we could understand more by looking deeper into the vendor firmware, but this would take more time and effort. I'll leave that as a task for me or somebody else in the future. Change-Id: Ic411a7a821d7b03be45d4bda645a03f82c7241c5 Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> --- M src/ec/hp/kbc1126/acpi/ac.asl M src/ec/hp/kbc1126/acpi/battery.asl M src/ec/hp/kbc1126/acpi/ec.asl 3 files changed, 178 insertions(+), 149 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/84242/1 diff --git a/src/ec/hp/kbc1126/acpi/ac.asl b/src/ec/hp/kbc1126/acpi/ac.asl index 9743045..4b04996 100644 --- a/src/ec/hp/kbc1126/acpi/ac.asl +++ b/src/ec/hp/kbc1126/acpi/ac.asl @@ -3,13 +3,6 @@ Name (ACST, 0x01) Name (SMAR, 0x00) -Field (ECRM, ByteAcc, NoLock, Preserve) -{ - Offset (0x84), - ADP, 1, - ADID, 2 -} - Method (UPAD, 0, Serialized) { Acquire (BTMX, 0xFFFF) diff --git a/src/ec/hp/kbc1126/acpi/battery.asl b/src/ec/hp/kbc1126/acpi/battery.asl index ff542a1..b1e9f2d 100644 --- a/src/ec/hp/kbc1126/acpi/battery.asl +++ b/src/ec/hp/kbc1126/acpi/battery.asl @@ -1,34 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -Field (ECRM, ByteAcc, NoLock, Preserve) -{ - Offset (0x84), - , 3, - LCTV, 1, - BATP, 4, - BPU, 1, - Offset (0x86), - BSEL, 4, /* battery select */ - Offset (0x87), - LB1, 8, - LB2, 8, - BDC, 16, - Offset (0x8D), - BFC, 16, /* battery full capacity */ - BRTE, 16, - BTC, 1, - Offset (0x92), - BME, 16, - BDN, 8, - BDV, 16, /* battery design voltage */ - BCV1, 16, - BST, 4, - Offset (0xC9), - BSN, 16, /* battery serial number */ - BDAT, 16, /* battery date */ - BMF, 8, -} - Name (NGBF, 0xFF) Name (NGBT, 0xFF) Name (GACP, 0x07) diff --git a/src/ec/hp/kbc1126/acpi/ec.asl b/src/ec/hp/kbc1126/acpi/ec.asl index ef68cc4..1922bd1 100644 --- a/src/ec/hp/kbc1126/acpi/ec.asl +++ b/src/ec/hp/kbc1126/acpi/ec.asl @@ -15,142 +15,207 @@ OperationRegion (ECRM, EmbeddedControl, 0x00, 0xFF) Field (ECRM, ByteAcc, NoLock, Preserve) { - PMCD, 32, - S0FL, 8, - SXF0, 8, - SXF1, 8, - CPWR, 16, - CVLT, 16, - CCUR, 16, - DIDX, 8, - CIDX, 8, - PMCC, 8, - PMEP, 8, + PMCD, 32, // PMC data? + S0FL, 8, // ? + SXF0, 8, // ? + SXF1, 8, // ? + CPWR, 16, // ... power? + CVLT, 16, // ... voltage? + CCUR, 16, // ... current? + DIDX, 8, // PMC data index? + CIDX, 8, // ... index? + PMCC, 8, // PMC command? + PMEP, 8, // ? Offset (0x22), - CRZN, 8, - THTA, 8, - HYST, 8, - CRIT, 8, - TEMP, 8, - TENA, 8, + CRZN, 8, // Select thermal zone + // 0 = PCHZ + // 1 = CPUZ + // 2 = GFXZ + // 3 = EXTZ + // 4 = LOCZ + // 5 = BATZ + // 6 = CHGZ + THTA, 8, // ? + HYST, 8, // ? + CRIT, 8, // ? + TEMP, 8, // Thermal zone temperature (unit: celcius) + TENA, 8, // ? Offset (0x29), - TOAD, 8, - PHTP, 8, - THEM, 8, - TMPO, 8, + TOAD, 8, // ? + PHTP, 8, // High trip point (unit: celcius) + // Vendor ACPI stores the next trip point temperature here. + THEM, 8, // Thermal zones with meaningful temperature change + // [0] = CPUZ + // [1] = GFXZ + // [2] = EXTZ + // [3] = LOCZ + // [4] = BATZ + // [5] = CHGZ + TMPO, 8, // ? Offset (0x2E), - FRDC, 8, - FTGC, 8, - PLTP, 8, + FRDC, 8, // (Thermal related?) + FTGC, 8, // (Thermal related?) + PLTP, 8, // Low trip point (unit: celcius) + // Vendor ACPI always stores the passive temperature here. Offset (0x32), - DTMP, 8, + DTMP, 8, // ... temperature (unit: celcius)? Offset (0x35), - PL1, 8, - PL2, 8, - BCVD, 8, + PL1, 8, // ? + PL2, 8, // ? + BCVD, 8, // ? Offset (0x40), - ABDI, 8, - ABAD, 8, - ABIX, 8, - ABDA, 8, - ABST, 8, - PORI, 8, + ABDI, 8, // Accelerometer device? + ABAD, 8, // Accelerometer mode? + // 0 = write + // 1 = read + ABIX, 8, // Accelerometer index? + ABDA, 8, // Accelerometer data? + ABST, 8, // Accelerometer status? + // [7] = operation failed? + PORI, 8, // Offset (0x4C), - PSSB, 8, + PSSB, 8, // (Battery related?) Offset (0x80), Offset (0x81), - , 4, - SLPT, 4, - FNSW, 1, - SFNC, 1, - ACPI, 1, - , 1, - , 1, - , 1, - , 1, - DETF, 1, - LIDS, 1, - TBLT, 1, - , 1, - , 1, - , 1, - COMM, 1, - PME, 1, - SBVD, 1, - - /* 0x84 to 0x9A is in battery.asl */ - + , 4, + SLPT, 4, // Sleep type + FNSW, 1, // Launch hotkeys without Fn keypress + SFNC, 1, // Swap Fn and Ctrl keys + ACPI, 1, // Enable ACPI + , 1, + , 1, + , 1, + , 1, + DETF, 1, // ? + LIDS, 1, // Lid status (opened/closed) + TBLT, 1, // Tablet (maybe related to touchscreens)? + , 1, + , 1, + , 1, + COMM, 1, // ? + PME, 1, // ? + SBVD, 1, // ? + ADP, 1, // AC presence + ADID, 2, // (Power source related?) + LCTV, 1, // ? + BATP, 4, // Battery presence + // [0] = primary + // [1] = secondary + BPU, 1, // ? + Offset (0x86), + BSEL, 4, // Select battery + // 0 = primary + // 1 = secondary + Offset (0x87), + LB1, 8, // Battery design capacity of warning + // There is a conversion from the ACPI value (used in _BIF) to the value + // stored in the EC: (X + 9) / 10 + // The speculation is that the EC stores in units of 10 mAh. The addition + // of 9 is to prevent truncating to 0 (e.g. 5/10 = 0 <=> (5 + 9)/10 = 1). + LB2, 8, // Battery design capacity of low + // See LB1 for more information (uses the same logic). + BDC, 16, // Battery design capacity + Offset (0x8D), + BFC, 16, // Battery last full charge capacity + BRTE, 16, // (Battery related?) + BTC, 1, // ? + Offset (0x92), + BME, 16, // (Battery related?) + BDN, 8, // Battery model (ASCII: max 16 characters) + // Reading this field will move the pointer to the next character. + // Setting BSEL will reset the pointer to the beginning. + BDV, 16, // Battery design voltage + BCV1, 16, // Battery cell voltage 1 + BST, 4, // Battery state Offset (0x9B), - BATE, 16, - BPR, 16, - BCR, 16, - BRC, 16, - BCC, 16, - BPV, 16, - BCV2, 16, - BCV3, 16, - BCV4, 16, - BCW, 16, - BATF, 16, - BCL, 16, - MAXC, 16, - BCG1, 8, - BT1I, 1, - BT2I, 1, - , 2, - BATN, 4, - BSTS, 16, - BCG2, 8, + BATE, 16, // (Battery related?) + BPR, 16, // Battery present rate + BCR, 16, // (Battery related?) + BRC, 16, // Battery remaining capacity + BCC, 16, // Battery cycle count + BPV, 16, // Battery present voltage + BCV2, 16, // Battery cell voltage 2 + BCV3, 16, // Battery cell voltage 3 + BCV4, 16, // Battery cell voltage 4 + BCW, 16, // ? + BATF, 16, // (Battery related?) + BCL, 16, // ? + MAXC, 16, // (Battery related?) + BCG1, 8, // ? + BT1I, 1, // ? + BT2I, 1, // ? + , 2, + BATN, 4, // (Battery related?) + BSTS, 16, // (Battery related?) + BCG2, 8, // ? Offset (0xBD), - BMO, 8, + BMO, 8, // ? Offset (0xBF), - BRCV, 8, + BRCV, 8, // (Battery related?) Offset (0xC1), - BIF, 8, - BRCC, 8, - - /* 0xC9 to 0xCD in battery.asl */ - + BIF, 8, // ? + BRCC, 8, // (Battery related?) + Offset (0xC9), + BSN, 16, // Battery serial number + BDAT, 16, // Battery date + // [0:4] = day + // [5:8] = month + // [9:15] = year ? 1980 (e.g. 2015 would be 35) + BMF, 8, // Battery manufacturer (ASCII: max 16 characters) + // See BDN for more information (uses the same logic). Offset (0xCF), - CTLB, 8, + CTLB, 8, // Battery commodity tracking number (ASCII: max 16 characters) + // See BDN for more information (uses the same logic). Offset (0xD1), - BTY, 8, + BTY, 8, // Battery type (ASCII: max 4 characters) + // See BDN for more information (uses the same logic). Offset (0xD5), - MFAC, 8, - CFAN, 8, - PFAN, 8, - OCPS, 8, - OCPR, 8, - OCPE, 8, - TMP1, 8, - TMP2, 8, - NABT, 4, - BCM, 4, - CCBQ, 16, - CBT, 16, + MFAC, 8, // ? + CFAN, 8, // ? + PFAN, 8, // ? + OCPS, 8, // ? + OCPR, 8, // ? + OCPE, 8, // ? + TMP1, 8, // ? + TMP2, 8, // ? + NABT, 4, // ? + BCM, 4, // ? + CCBQ, 16, // ? + CBT, 16, // Current battery temperature (unit: decikelvin) Offset (0xE3), - OST, 4, + OST, 4, // OS type + // 1 = Linux + // 4 = Windows 2001 + // 4 = Windows 2001 SP1 + // 5 = Windows 2001 SP2 + // 6 = Windows 2006 + // 7 = Windows 2009 + // 8 = Windows 2012 + // 9 = Windows 2013 + // 10 = Windows 2015 + // 11 = Windows 2016 + // 12 = Windows 2017 Offset (0xE4), Offset (0xE5), - TPTE, 1, - TBBN, 1, - , 3, - TP, 1, + TPTE, 1, // ? + TBBN, 1, // (Brightness related?) + , 3, + TP, 1, // ? Offset (0xE6), - SHK, 8, - AUDS, 1, - SPKR, 1, + SHK, 8, // (Hotkey related?) + AUDS, 1, // ? + SPKR, 1, // ? Offset (0xE8), - HSEN, 4, - HSST, 4, + HSEN, 4, // ? + HSST, 4, // ? Offset (0xEA), - , 2, - WWP, 1, - WLP, 1, + , 2, + WWP, 1, // (Battery related?) + WLP, 1, // (Battery related?) Offset (0xEF), - INCH, 2, - IDIS, 2, - INAC, 1 + INCH, 2, // (Battery related?) + IDIS, 2, // (Battery related?) + INAC, 1, // (Battery related?) } Method (ECRI, 0, Serialized) -- To view, visit https://review.coreboot.org/c/coreboot/+/84242?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic411a7a821d7b03be45d4bda645a03f82c7241c5 Gerrit-Change-Number: 84242 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com>

1 0

[XS] Change in coreboot[main]: mb/google/brox/var/lotso: Enable ASPM for PCIe4 SSD of CPU
by Wentao Qin (Code Review) 07 Sep '24

07 Sep '24

Attention is currently required from: Jian Tong, wen zhang. Wentao Qin has posted comments on this change by Wentao Qin. ( https://review.coreboot.org/c/coreboot/+/84241?usp=email ) Change subject: mb/google/brox/var/lotso: Enable ASPM for PCIe4 SSD of CPU ...................................................................... Patch Set 2: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/84241/comment/937de94b_81bcb1e5?us… : PS1, Line 17: lotso-rev0 ~ # lspci -vv | grep -A30 "KIOXIA" | grep -E "LnkCap|LnkCtl" > > `Possible unwrapped commit description (prefer a maximum 72 chars per line)` […] Done https://review.coreboot.org/c/coreboot/+/84241/comment/e8df0bd9_9231a7d6?us… : PS1, Line 24: lotso-rev0 ~ # lspci -vv | grep -A30 "KIOXIA" | grep -E "LnkCap|LnkCtl" > > `Possible unwrapped commit description (prefer a maximum 72 chars per line)` […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/84241?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8a7f69bb82ad24b29566541d7694f87f9c6458d6 Gerrit-Change-Number: 84241 Gerrit-PatchSet: 2 Gerrit-Owner: Wentao Qin <qinwentao(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Jian Tong <tongjian(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: wen zhang <zhangwen6(a)huaqin.corp-partner.google.com> Gerrit-CC: Jinfang Mao <maojinfang(a)huaqin.corp-partner.google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: wen zhang <zhangwen6(a)huaqin.corp-partner.google.com> Gerrit-Attention: Jian Tong <tongjian(a)huaqin.corp-partner.google.com> Gerrit-Comment-Date: Sat, 07 Sep 2024 07:56:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Wentao Qin <qinwentao(a)huaqin.corp-partner.google.com>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2024