coreboot-gerrit April 2025

coreboot-gerrit@coreboot.org

1 participants
3140 discussions

[S] Change in coreboot[main]: mb/google/nissa/var/guren: Add touch screen ILIT2901 support
by Daniel Peng (Code Review) 08 Apr '25

08 Apr '25

Attention is currently required from: Dinesh Gehlot, Eric Lai, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Subrata Banik. Daniel Peng has posted comments on this change by Daniel Peng. ( https://review.coreboot.org/c/coreboot/+/87196?usp=email ) Change subject: mb/google/nissa/var/guren: Add touch screen ILIT2901 support ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Hi Googler, I had re-uploaded the CL after sync code. Please help to set CR if no concern. Thanks. The settings were also confirm with our EE. -- To view, visit https://review.coreboot.org/c/coreboot/+/87196?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: I3435a8852b1fdb897d5f89c580b5f9bfca03dd8e Gerrit-Change-Number: 87196 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Tue, 08 Apr 2025 03:02:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com>

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/guren: Add touch screen ILIT2901 support
by Daniel Peng (Code Review) 08 Apr '25

08 Apr '25

Attention is currently required from: Dinesh Gehlot, Eric Lai, Jayvik Desai, Kapil Porwal, Nick Vaccaro, Subrata Banik. Daniel Peng has posted comments on this change by Daniel Peng. ( https://review.coreboot.org/c/coreboot/+/87196?usp=email ) Change subject: mb/google/nissa/var/guren: Add touch screen ILIT2901 support ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: Hi Googler, I had re-uploaded the CL after sync code. Please help to set CR if no concern. Thanks. -- To view, visit https://review.coreboot.org/c/coreboot/+/87196?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: I3435a8852b1fdb897d5f89c580b5f9bfca03dd8e Gerrit-Change-Number: 87196 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Tue, 08 Apr 2025 03:00:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/guren: Add WFC for AZWAVE/AM-3BA5533-B module
by Subrata Banik (Code Review) 08 Apr '25

08 Apr '25

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/87054?usp=email ) Change subject: mb/google/nissa/var/guren: Add WFC for AZWAVE/AM-3BA5533-B module ...................................................................... mb/google/nissa/var/guren: Add WFC for AZWAVE/AM-3BA5533-B module New WFC function for Guren360 project. ID of Vendor and Product for AZWAVE/AM-3BA5533-B module are 13d3:5533. 1. Add WFC fw_config setting 2. Used USB2 Port7 for WFC BUG=b:406164678 BRANCH=firmware-nissa-15217.B TEST=1. emerge-nissa coreboot chromeos-bootimage 2. Verified with local build bios via command lsusb for 13d3:5533 and SSFC WFC enable Change-Id: Ia2ab019fe76c503cc669ebf4aa8b3d7ae72269c1 Signed-off-by: Daniel Peng <Daniel_Peng(a)pegatron.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/87054 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Eric Lai <ericllai(a)google.com> Reviewed-by: Jayvik Desai <jayvik(a)google.com> Reviewed-by: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Reviewed-by: Subrata Banik <subratabanik(a)google.com> --- M src/mainboard/google/brya/variants/guren/overridetree.cb M src/mainboard/google/brya/variants/guren/variant.c 2 files changed, 14 insertions(+), 2 deletions(-) Approvals: Subrata Banik: Looks good to me, approved Eric Lai: Looks good to me, approved Jayvik Desai: Looks good to me, approved Daniel Peng: Looks good to me, but someone else must approve build bot (Jenkins): Verified diff --git a/src/mainboard/google/brya/variants/guren/overridetree.cb b/src/mainboard/google/brya/variants/guren/overridetree.cb index 77ec260..8b1adb1 100644 --- a/src/mainboard/google/brya/variants/guren/overridetree.cb +++ b/src/mainboard/google/brya/variants/guren/overridetree.cb @@ -21,6 +21,10 @@ option WIFI_SAR_ID_INTEL_CONVERTIBLE 0 option WIFI_SAR_ID_INTEL_CLAMSHELL 1 end + field WFC 17 18 + option WFC_ABSENT 0 + option WFC_PRESENT 1 + end field TOUCHSCREEN_SOURCE 32 34 option TOUCHSCREEN_UNPROVISIONED 0 option TOUCHSCREEN_ELAN0001 1 @@ -87,6 +91,7 @@ register "typec_aux_bias_pads[0]" = "{.pad_auxp_dc = GPP_E22, .pad_auxn_dc = GPP_E23}" register "typec_aux_bias_pads[1]" = "{.pad_auxp_dc = GPP_A21, .pad_auxn_dc = GPP_A22}" + register "usb2_ports[6]" = "USB2_PORT_MID(OC_SKIP)" # WFC register "usb2_ports[9]" = "USB2_PORT_MID(OC_SKIP)" # Bluetooth port for CNVi WLAN # Configure external V1P05/Vnn/VnnSx Rails @@ -595,9 +600,11 @@ device ref usb2_port6 on end end chip drivers/usb/acpi - register "desc" = ""USB2 Camera"" + register "desc" = ""USB2 WFC"" register "type" = "UPC_TYPE_INTERNAL" - device ref usb2_port7 on end + device ref usb2_port7 on + probe WFC WFC_PRESENT + end end chip drivers/usb/acpi register "desc" = ""USB2 Bluetooth"" diff --git a/src/mainboard/google/brya/variants/guren/variant.c b/src/mainboard/google/brya/variants/guren/variant.c index c2c49f8..4949083 100644 --- a/src/mainboard/google/brya/variants/guren/variant.c +++ b/src/mainboard/google/brya/variants/guren/variant.c @@ -18,4 +18,9 @@ printk(BIOS_INFO, "Enable DDI PORT 2 for HPD and DDC.\n"); config->ddi_ports_config[DDI_PORT_2] = DDI_ENABLE_HPD | DDI_ENABLE_DDC; } + + if (!fw_config_probe(FW_CONFIG(WFC, WFC_PRESENT))) { + printk(BIOS_INFO, "Disable usb2_port7 of WFC.\n"); + config->usb2_ports[6] = (struct usb2_port_config) USB2_PORT_EMPTY; + } } -- To view, visit https://review.coreboot.org/c/coreboot/+/87054?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: Ia2ab019fe76c503cc669ebf4aa8b3d7ae72269c1 Gerrit-Change-Number: 87054 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in coreboot[main]: mb/var/uldrenite: Configure descriptor for either MBVR or FIVR
by Subrata Banik (Code Review) 08 Apr '25

08 Apr '25

Attention is currently required from: Ben Kao, Dtrain Hsu, Eric Lai, Intel coreboot Reviewers, Jamie Chen, Jayvik Desai, John Su, Nick Vaccaro. Subrata Banik has posted comments on this change by John Su. ( https://review.coreboot.org/c/coreboot/+/87033?usp=email ) Change subject: mb/var/uldrenite: Configure descriptor for either MBVR or FIVR ...................................................................... Patch Set 11: (1 comment) Patchset: PS11: > Hi Reviewers > > I’ve already chained the CL that fixes the "incompatible pointer-to-integer conversion" issue using Relation chain. > However, it seems the build bot is unable to +1 the verification for this CL. > Would it be possible to help proceed with the merge or advise how to resolve this? Can you please try to resolve the build error https://qa.coreboot.org/job/coreboot-gerrit/275635/testReport/junit/(root)/… -- To view, visit https://review.coreboot.org/c/coreboot/+/87033?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: I337574c8c55889ceb49b9f33625feadb48bd8890 Gerrit-Change-Number: 87033 Gerrit-PatchSet: 11 Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Reviewer: Ben Kao <ben.kao(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jamie Chen <jamie.chen(a)intel.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-CC: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Jamie Chen <jamie.chen(a)intel.com> Gerrit-Attention: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Ben Kao <ben.kao(a)intel.com> Gerrit-Comment-Date: Tue, 08 Apr 2025 02:54:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: John Su <john_su(a)compal.corp-partner.google.com>

1 0

[S] Change in coreboot[main]: mb/google/nissa/var/guren: Add WFC for AZWAVE/AM-3BA5533-B module
by Daniel Peng (Code Review) 08 Apr '25

08 Apr '25

Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Nick Vaccaro. Daniel Peng has posted comments on this change by Daniel Peng. ( https://review.coreboot.org/c/coreboot/+/87054?usp=email ) Change subject: mb/google/nissa/var/guren: Add WFC for AZWAVE/AM-3BA5533-B module ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: Hi Googler, Please help to submit the CL if no other concern. Thanks. -- To view, visit https://review.coreboot.org/c/coreboot/+/87054?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: Ia2ab019fe76c503cc669ebf4aa8b3d7ae72269c1 Gerrit-Change-Number: 87054 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Tue, 08 Apr 2025 02:40:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in coreboot[main]: Documentation/lib: Update Timestamp documentation
by Nicholas Chin (Code Review) 08 Apr '25

08 Apr '25

Attention is currently required from: Martin L Roth. Nicholas Chin has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/87186?usp=email ) Change subject: Documentation/lib: Update Timestamp documentation ...................................................................... Patch Set 4: (1 comment) File Documentation/lib/timestamp.md: https://review.coreboot.org/c/coreboot/+/87186/comment/ea50a51a_95863ebc?us… : PS4, Line 192: ### Core Functions > The problem with directly listing functions or structures in the documentation folder is that they w […] I don't think markdown itself has a way to pull in the header file, though MyST Parser has some wrappers for reStructuredText directives. For example, ```{literalinclude} ../../src/include/timestamp.h ``` will include a code block containing that entire file. https://mystmd.org/guide/code#docs-literalinclude Parsing out function prototypes and docstrings is something usually associated with something like Doxygen. coreboot used to have a Make target for that, though we dropped support for generating it a while ago; refer to commit 619086d1056c ("Treewide: Remove doxygen config files and targets") I did find https://breathe.readthedocs.io/en/latest/index.html which seems to be able to bridge doxygen with Sphinx, though I don't know if that's something we want to explore. Not all our docstrings are formatted for Doxygen anyway. -- To view, visit https://review.coreboot.org/c/coreboot/+/87186?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: I6e4cc244edf6cc860cc66165173f134a30a81589 Gerrit-Change-Number: 87186 Gerrit-PatchSet: 4 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Comment-Date: Tue, 08 Apr 2025 02:36:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org>

1 0

[S] Change in coreboot[main]: mb/var/uldrenite: Configure descriptor for either MBVR or FIVR
by John Su (Code Review) 08 Apr '25

08 Apr '25

Attention is currently required from: Ben Kao, Dtrain Hsu, Eric Lai, Intel coreboot Reviewers, Jamie Chen, Jayvik Desai, Nick Vaccaro, Subrata Banik. John Su has posted comments on this change by John Su. ( https://review.coreboot.org/c/coreboot/+/87033?usp=email ) Change subject: mb/var/uldrenite: Configure descriptor for either MBVR or FIVR ...................................................................... Patch Set 11: (1 comment) Patchset: PS11: Hi Reviewers I’ve already chained the CL that fixes the "incompatible pointer-to-integer conversion" issue using Relation chain. However, it seems the build bot is unable to +1 the verification for this CL. Would it be possible to help proceed with the merge or advise how to resolve this? -- To view, visit https://review.coreboot.org/c/coreboot/+/87033?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: I337574c8c55889ceb49b9f33625feadb48bd8890 Gerrit-Change-Number: 87033 Gerrit-PatchSet: 11 Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Reviewer: Ben Kao <ben.kao(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jamie Chen <jamie.chen(a)intel.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-CC: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Jamie Chen <jamie.chen(a)intel.com> Gerrit-Attention: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Ben Kao <ben.kao(a)intel.com> Gerrit-Comment-Date: Tue, 08 Apr 2025 02:26:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in coreboot[main]: Documentation/lib: Update Timestamp documentation
by Julius Werner (Code Review) 08 Apr '25

08 Apr '25

Attention is currently required from: Martin L Roth, Nicholas Chin. Julius Werner has posted comments on this change by Martin L Roth. ( https://review.coreboot.org/c/coreboot/+/87186?usp=email ) Change subject: Documentation/lib: Update Timestamp documentation ...................................................................... Patch Set 4: (10 comments) File Documentation/lib/timestamp.md: https://review.coreboot.org/c/coreboot/+/87186/comment/8d44da9f_e4732495?us… : PS4, Line 79: (default 30, configurable via : the Kconfig option `CONFIG_MAX_TIMESTAMPS`) This seems incorrect — the current code just has the number 192 hardcoded. https://review.coreboot.org/c/coreboot/+/87186/comment/248a80e3_2e9a42c4?us… : PS4, Line 86: uint64_t Looks like this was changed to `int64_t` with the addition of pre-CPU timestamps. https://review.coreboot.org/c/coreboot/+/87186/comment/df603849_859f7646?us… : PS4, Line 112: enum { I think this entire enum doesn't exist anymore. It seems we just maintain a global pointer to the currently active table now that is changed when CBMEM is initialized. https://review.coreboot.org/c/coreboot/+/87186/comment/43467640_bc18dbed?us… : PS4, Line 119: if the cache is stored in local stash (bss area) I think this is out of date, I can't find that in the code anymore. I think we used to do that in early ramstage (or am I confusing that with CBMEM console?) but I guess we don't anymore. Nowadays it seems we just don't log any timestamps before CBMEM reinit in ramstage anymore. The only two remaining backends are `_timestamp` and CBMEM. https://review.coreboot.org/c/coreboot/+/87186/comment/a6d4178b_141ee56e?us… : PS4, Line 141: CONFIG_MAX_TIMESTAMP_CACHE_ENTRIES This Kconfig doesn't seem to exist anymore? I'm not sure if this section is talking about pre-RAM or CBMEM timestamps. Pre-RAM the size is determined by platform depending on how much space is allocated in memlayout. For CBMEM, the size seems to be hardcoded to 192 entries. https://review.coreboot.org/c/coreboot/+/87186/comment/cfbd6a66_05e98290?us… : PS4, Line 192: ### Core Functions The problem with directly listing functions or structures in the documentation folder is that they will inevitably go out of date again eventually when people change the code and don't realize there's separate documentation somewhere. We already have (or should have) good docstrings inside the header files in many cases which people usually do remember to update when they make changes. I wonder if it would be better to just link to the appropriate headers here? (I don't suppose markdown has a way to just pull in a whole header file into the document, or parse out the function prototypes and docstrings or something like that?) https://review.coreboot.org/c/coreboot/+/87186/comment/ad7fec2b_11847702?us… : PS4, Line 200: On x86 with CAR backed memory in romstage, this means : romstage before memory init No, it's actually bootblock for all architectures nowadays. (If you want, you could also mention how platform setup code on x86 or the decompressor on Arm can measure some timestamps earlier and pass them in to `bootblock_main_with_timestamp()`.) https://review.coreboot.org/c/coreboot/+/87186/comment/b51fe47a_0a4f1aab?us… : PS4, Line 252: - 100-115: Miscellaneous coreboot operations (e.g., I'm not sure if there really exists a difference between 1-99 and 100-199? I think that's all just the general coreboot kitchen sink range. https://review.coreboot.org/c/coreboot/+/87186/comment/da4ec2dd_a33670e2?us… : PS4, Line 254: - 500-600: Google/ChromeOS specific (e.g., `TS_VBOOT_START`, nit: maybe mention that many of the existing timestamps here are no longer Google-specific since many features originally added for Google vendorcode have since been migrated into general coreboot code. https://review.coreboot.org/c/coreboot/+/87186/comment/5f1a4ccf_16066802?us… : PS4, Line 327: . = ALIGN(8);\n _etimestamp = .;\n} nit: should usually use the `TIMESTAMP()` macro from `memlayout.h`, i.e. `TIMESTAMP(., 0x200)` -- To view, visit https://review.coreboot.org/c/coreboot/+/87186?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: I6e4cc244edf6cc860cc66165173f134a30a81589 Gerrit-Change-Number: 87186 Gerrit-PatchSet: 4 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Tue, 08 Apr 2025 01:29:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: cpu/x86: Conditionally invalidate caches based on self-snooping support
by Subrata Banik (Code Review) 08 Apr '25

08 Apr '25

Attention is currently required from: Jérémy Compostella, Paul Menzel. Subrata Banik has posted comments on this change by Subrata Banik. ( https://review.coreboot.org/c/coreboot/+/87182?usp=email ) Change subject: cpu/x86: Conditionally invalidate caches based on self-snooping support ...................................................................... Patch Set 4: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/87182/comment/4b941051_0e795328?us… : PS3, Line 16: Rationale: : : - > I don't think the highlighted text brings anything. Acknowledged https://review.coreboot.org/c/coreboot/+/87182/comment/52693c85_ffd3b474?us… : PS3, Line 37: Able to reduce : boot time by ~19-25ms. > On all three platforms? yes, that's right -- To view, visit https://review.coreboot.org/c/coreboot/+/87182?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: If32439752d0ceaa03b1d81873ea0bc562092e9d5 Gerrit-Change-Number: 87182 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Tue, 08 Apr 2025 01:19:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>

1 0

[S] Change in coreboot[main]: cpu/x86: Conditionally invalidate caches based on self-snooping support
by Subrata Banik (Code Review) 08 Apr '25

08 Apr '25

Attention is currently required from: Paul Menzel, Subrata Banik. Hello Jérémy Compostella, Shuo Liu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/87182?usp=email to look at the new patch set (#4). Change subject: cpu/x86: Conditionally invalidate caches based on self-snooping support ...................................................................... cpu/x86: Conditionally invalidate caches based on self-snooping support The code currently unconditionally flushes or invalidates the entire cache (using `clflush_region` or `wbinvd`) after loading the SIPI vector for APs and after loading SMM handlers. This commit modifies this behavior to only perform these cache operations if the CPU does *not* support self-snooping. Self-snooping CPUs can maintain cache coherency within the core/ complex more efficiently. CPU with self-snoop enabled does not necessarily need to perform wbinvd to ensure data written to the cache is reflected in main memory. Self-snooping CPUs employ a write-back caching policy, combined with a cache coherence protocol, to manage data writes and ensure consistency between cache and main memory. When the BSP writes the SIPI vector or SMM handlers to memory, other units within the same CPU that might be caching these regions should be aware of the updates through the self-snooping mechanism. A full cache flush or invalidate to ensure cache contains reaches to main memory might be unnecessary and could negatively impact performance. By conditionally performing these cache operations based on `self_snooping_supported()`, we can optimize the boot process for CPUs that have advanced cache coherency features while maintaining correct behavior on older or simpler CPUs. TEST=Boot google/rex, brox and fatcat with this patch. Able to reduce boot time by ~19-25ms. Change-Id: If32439752d0ceaa03b1d81873ea0bc562092e9d5 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/cpu/x86/cache/cache.c M src/cpu/x86/mp_init.c 2 files changed, 18 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/82/87182/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/87182?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: If32439752d0ceaa03b1d81873ea0bc562092e9d5 Gerrit-Change-Number: 87182 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2025