coreboot-gerrit October 2024

coreboot-gerrit@coreboot.org

1 participants
2622 discussions

[S] Change in coreboot[main]: drivers/efi/capsules.c: fix recording capsule size
by Nico Huber (Code Review) 01 Oct '24

01 Oct '24

Attention is currently required from: Sergii Dmytruk. Nico Huber has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/coreboot/+/84542?usp=email ) Change subject: drivers/efi/capsules.c: fix recording capsule size ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/84542/comment/8d4ba481_b9b2ca5d?us… : PS1, Line 8: > Right, not sure why I didn't add any details. Updated commit messages here and in the parent change. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/84542?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: I97a528e2611fcd711c555d0f01e9aadcd2031217 Gerrit-Change-Number: 84542 Gerrit-PatchSet: 2 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 01 Oct 2024 21:01:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>

1 0

[XS] Change in coreboot[main]: stddef.h: Introduce nullptr constant
by Nico Huber (Code Review) 01 Oct '24

01 Oct '24

Attention is currently required from: Elyes Haouas, Felix Singer, Jakub Czapiga, Martin L Roth, Maximilian Brune. Nico Huber has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/83459?usp=email ) Change subject: stddef.h: Introduce nullptr constant ...................................................................... Patch Set 26: (1 comment) File src/include/stddef.h: https://review.coreboot.org/c/coreboot/+/83459/comment/a5420c5c_32b9afbe?us… : PS26, Line 25: #define nullptr ((void *)0) > Well, the keyword `nullptr` is going to be defined for C23 and we're presumably planning to switch t […] Well, definitely not worth any further discussion. I don't see any immediate damage, even though this is against standards. -- To view, visit https://review.coreboot.org/c/coreboot/+/83459?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: I07db866bebfd25f1a60d18a3228ada2957500234 Gerrit-Change-Number: 83459 Gerrit-PatchSet: 26 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Tue, 01 Oct 2024 20:59:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>

1 0

[L] Change in coreboot[main]: mb/google/fatcat: Add GPIO settings
by Cliff Huang (Code Review) 01 Oct '24

01 Oct '24

Attention is currently required from: Jérémy Compostella, Pranava Y N, Subrata Banik, Wonkyu Kim. Cliff Huang has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/84405?usp=email ) Change subject: mb/google/fatcat: Add GPIO settings ...................................................................... Patch Set 22: (5 comments) File src/mainboard/google/fatcat/variants/fatcat/gpio.c: https://review.coreboot.org/c/coreboot/+/84405/comment/21ab9d7e_1ed1d229?us… : PS1, Line 437: /* GPP_B16: GEN5_SSD_PWREN */ : PAD_CFG_GPO(GPP_B16, 0, PLTRST), > > will move to romstage and use fw_config. […] done in https://review.coreboot.org/c/coreboot/+/84408/19 https://review.coreboot.org/c/coreboot/+/84405/comment/8a64074a_9872bde5?us… : PS1, Line 440: /* GPP_C05: CRD1_PWREN */ > > This is used for world facing camera. […] We will need fw_config for camera1 & 2. I created tables are ready in https://review.coreboot.org/c/coreboot/+/84408/19. https://review.coreboot.org/c/coreboot/+/84405/comment/cc14f2c8_26f78b95?us… : PS1, Line 452: /* M.2 WWAN */ > same, this should be inside FW config. Done https://review.coreboot.org/c/coreboot/+/84405/comment/f9929371_7d9f82df?us… : PS1, Line 459: : /* GPP_D03: M.2_WWAN_PERST_GPIO_N */ > same Done File src/mainboard/google/fatcat/variants/fatcat/gpio.c: https://review.coreboot.org/c/coreboot/+/84405/comment/f2f8e69f_288cd211?us… : PS21, Line 412: /* GPP_C00: GPP_C0_SMBCLK */ : PAD_CFG_NF(GPP_C00, NONE, DEEP, NF1), : /* GPP_C01: GPP_C1_SMBDATA */ : PAD_CFG_NF(GPP_C01, NONE, DEEP, NF1), : > this SPD SMB. should be okay to move to romstage. Let me build & test. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/84405?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: I914f73ff06bfb801fc319b45b23d7ce4cb7a6d52 Gerrit-Change-Number: 84405 Gerrit-PatchSet: 22 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.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: Cliff Huang <cliff.huang(a)intel.com> Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Tue, 01 Oct 2024 20:43:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>

1 0

[L] Change in coreboot[main]: mb/google/fatcat: Add GPIO settings
by Cliff Huang (Code Review) 01 Oct '24

01 Oct '24

Attention is currently required from: Jérémy Compostella, Pranava Y N, Subrata Banik, Wonkyu Kim. Cliff Huang has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/84405?usp=email ) Change subject: mb/google/fatcat: Add GPIO settings ...................................................................... Patch Set 22: (1 comment) File src/mainboard/google/fatcat/variants/fatcat/gpio.c: https://review.coreboot.org/c/coreboot/+/84405/comment/0a374bec_e76b566a?us… : PS21, Line 253: overridden via fw_config > this is overdoing IMO. Subrata, How about changing to 'fw_config: <field name>'? Initially, I thought we wanted to remove those pads from the ramstage table and then found out that the pad needs to be in the table in order for fw_config to override. If the pad does not in the table, the override won't happen. I can also remove these additional comments entirely. In fact, it makes us easier since the pads including their comment are generated by a script. -- To view, visit https://review.coreboot.org/c/coreboot/+/84405?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: I914f73ff06bfb801fc319b45b23d7ce4cb7a6d52 Gerrit-Change-Number: 84405 Gerrit-PatchSet: 22 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.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: Cliff Huang <cliff.huang(a)intel.com> Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Tue, 01 Oct 2024 20:37:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>

1 0

[XS] Change in coreboot[main]: stddef.h: Introduce nullptr constant
by Julius Werner (Code Review) 01 Oct '24

01 Oct '24

Attention is currently required from: Elyes Haouas, Felix Singer, Jakub Czapiga, Martin L Roth, Maximilian Brune, Nico Huber. Julius Werner has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/83459?usp=email ) Change subject: stddef.h: Introduce nullptr constant ...................................................................... Patch Set 26: (1 comment) File src/include/stddef.h: https://review.coreboot.org/c/coreboot/+/83459/comment/8776a690_53596985?us… : PS26, Line 25: #define nullptr ((void *)0) > To be clear, my concern is only about the `#define nullptr` on the old path, not the C23 path. […] Well, the keyword `nullptr` is going to be defined for C23 and we're presumably planning to switch to that eventually. So code that uses that name as an identifier is going to have a problem anyway. I'm not sure why we should need to argue so much about code that works differently in C17 vs C23 anyway... don't we usually just change the coreboot C standard, rather than maintain support for multiple standards? I assume CB:84489 is supposed to be a test change and will eventually go away before merging — I don't think there's a point in letting users decide this via Kconfig. At the point where we decide that we want to switch to C23 we would just do it, and then we could also remove the arguable #else clause here again. (Or we just squash all of these patches together and change things in one go, don't really care. The only important part here is that once we've switched to C23, it makes sense to redefine NULL via nullptr.) -- To view, visit https://review.coreboot.org/c/coreboot/+/83459?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: I07db866bebfd25f1a60d18a3228ada2957500234 Gerrit-Change-Number: 83459 Gerrit-PatchSet: 26 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Tue, 01 Oct 2024 20:18:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr>

1 0

[L] Change in coreboot[main]: soc/xeon_sp: Initially add N-1 IBL codes
by Lean Sheng Tan (Code Review) 01 Oct '24

01 Oct '24

Attention is currently required from: Alicja Michalska, Angel Pons, Arthur Heymans, Christian Walter, Felix Held, Felix Singer, Johnny Lin, Jonathan Zhang, Patrick Rudolph, Shuo Liu, Tim Chu. Lean Sheng Tan has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/84306?usp=email ) Change subject: soc/xeon_sp: Initially add N-1 IBL codes ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/84306?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: I6bd5a2ed973ff91750c5ed1f9a57d30e41d8b97e Gerrit-Change-Number: 84306 Gerrit-PatchSet: 1 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 01 Oct 2024 20:11:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: soc/intel/xeon_sp: Use MemoryMapDataHob to add high RAM resources
by Lean Sheng Tan (Code Review) 01 Oct '24

01 Oct '24

Attention is currently required from: Alicja Michalska, Angel Pons, Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Patrick Rudolph, Shuo Liu, Tim Chu. Lean Sheng Tan has posted comments on this change by Shuo Liu. ( https://review.coreboot.org/c/coreboot/+/84304?usp=email ) Change subject: soc/intel/xeon_sp: Use MemoryMapDataHob to add high RAM resources ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84304?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: Ie5fbc5735704d95c7ad50740ff0e35737afdbd80 Gerrit-Change-Number: 84304 Gerrit-PatchSet: 4 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 01 Oct 2024 20:10:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: soc/intel/pantherlake: Remove soc_info.[hc] interface
by Jérémy Compostella (Code Review) 01 Oct '24

01 Oct '24

Attention is currently required from: Kapil Porwal, Pranava Y N, Subrata Banik. Hello Kapil Porwal, Pranava Y N, Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84616?usp=email to look at the new patch set (#2). Change subject: soc/intel/pantherlake: Remove soc_info.[hc] interface ...................................................................... soc/intel/pantherlake: Remove soc_info.[hc] interface This commit removes the unnecessary layer provided by soc_info.[hc]. It was providing an abstraction which only was resulting in extra function calls without any added value as the returned constants are well identified and could be used directly. More importantly, and this is the actual selling point in my opinion, this extra indirection was preventing the compiler from detecting array overflows. BUG=348678529 TEST=Build is successful Change-Id: Iea26d962748116fa84afdb4afcba1098a64b6986 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/soc/intel/pantherlake/Makefile.mk M src/soc/intel/pantherlake/cpu.c M src/soc/intel/pantherlake/elog.c D src/soc/intel/pantherlake/include/soc/soc_info.h M src/soc/intel/pantherlake/pcie_rp.c M src/soc/intel/pantherlake/romstage/fsp_params.c D src/soc/intel/pantherlake/soc_info.c 7 files changed, 3 insertions(+), 84 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/84616/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84616?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: Iea26d962748116fa84afdb4afcba1098a64b6986 Gerrit-Change-Number: 84616 Gerrit-PatchSet: 2 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(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-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>

1 0

[L] Change in coreboot[main]: soc/intel/pantherlake: Add FSP-S programming
by Jérémy Compostella (Code Review) 01 Oct '24

01 Oct '24

Attention is currently required from: Kapil Porwal, Paul Menzel, Pranava Y N, Ronak Kanabar, Subrata Banik. Jérémy Compostella has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/84552?usp=email ) Change subject: soc/intel/pantherlake: Add FSP-S programming ...................................................................... Patch Set 14: (1 comment) File src/soc/intel/pantherlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/84552/comment/f73cc184_6228a0d8?us… : PS11, Line 301: max_port = get_max_tcss_port(); : for (i = 0; i < max_port; i++) > > It would definitely save few CPU cycles. […] There was no way I would introduce inconsistent code as I would have if I had followed your proposal so I created [84616: soc/intel/pantherlake: Remove soc_info.[hc] interface](https://review.coreboot.org/c/coreboot/+/84616) instead. Obviously, the CPU cycles saving is completely ridiculous and not my concern here. However, I agree that these functions were not adding any values and more importantly, this was the final selling point for me, it was preventing the compiler from detecting arrays overflow. -- To view, visit https://review.coreboot.org/c/coreboot/+/84552?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: Iea26d962748116fa84afdb4afcba1098a64b6989 Gerrit-Change-Number: 84552 Gerrit-PatchSet: 14 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Tue, 01 Oct 2024 18:23:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>

1 0

[M] Change in coreboot[main]: soc/intel/pantherlake: Remove soc_info.[hc] interface
by Jérémy Compostella (Code Review) 01 Oct '24

01 Oct '24

Attention is currently required from: Kapil Porwal, Pranava Y N, Subrata Banik. Jérémy Compostella has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84616?usp=email ) Change subject: soc/intel/pantherlake: Remove soc_info.[hc] interface ...................................................................... soc/intel/pantherlake: Remove soc_info.[hc] interface This commit removes the unnecessary layer provided by soc_info.[hc]. It was providing an abstraction which only was resulting in extra function calls without any added value as the return constant are well identified and could be used directly. More importantly, and this is the actual selling point in my opinion, this extra indirection was preventing the compiler from detecting array overflows. BUG=348678529 TEST=Build is successful Change-Id: Iea26d962748116fa84afdb4afcba1098a64b6986 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/soc/intel/pantherlake/Makefile.mk M src/soc/intel/pantherlake/cpu.c M src/soc/intel/pantherlake/elog.c D src/soc/intel/pantherlake/include/soc/soc_info.h M src/soc/intel/pantherlake/pcie_rp.c M src/soc/intel/pantherlake/romstage/fsp_params.c D src/soc/intel/pantherlake/soc_info.c 7 files changed, 3 insertions(+), 84 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/84616/1 diff --git a/src/soc/intel/pantherlake/Makefile.mk b/src/soc/intel/pantherlake/Makefile.mk index 10ef8e9..f3403b5 100644 --- a/src/soc/intel/pantherlake/Makefile.mk +++ b/src/soc/intel/pantherlake/Makefile.mk @@ -18,14 +18,12 @@ bootblock-y += bootblock/pcd.c bootblock-y += bootblock/report_platform.c bootblock-y += espi.c -bootblock-y += soc_info.c romstage-$(CONFIG_SOC_INTEL_CSE_PRE_CPU_RESET_TELEMETRY) += cse_telemetry.c romstage-y += espi.c romstage-y += meminit.c romstage-y += pcie_rp.c romstage-y += reset.c -romstage-y += soc_info.c ramstage-y += acpi.c ramstage-y += chip.c @@ -41,7 +39,6 @@ ramstage-y += pmc.c ramstage-y += reset.c ramstage-y += retimer.c -ramstage-y += soc_info.c ramstage-y += soundwire.c ramstage-y += systemagent.c ramstage-y += tcss.c @@ -52,7 +49,6 @@ smm-y += p2sb.c smm-y += pmutil.c smm-y += smihandler.c -smm-y += soc_info.c smm-y += uart.c smm-y += xhci.c CPPFLAGS_common += -I$(src)/soc/intel/pantherlake diff --git a/src/soc/intel/pantherlake/cpu.c b/src/soc/intel/pantherlake/cpu.c index 7856383..1b81856 100644 --- a/src/soc/intel/pantherlake/cpu.c +++ b/src/soc/intel/pantherlake/cpu.c @@ -20,7 +20,6 @@ #include <soc/msr.h> #include <soc/pci_devs.h> #include <soc/soc_chip.h> -#include <soc/soc_info.h> bool cpu_soc_is_in_untrusted_mode(void) { diff --git a/src/soc/intel/pantherlake/elog.c b/src/soc/intel/pantherlake/elog.c index fce408f..7750058 100644 --- a/src/soc/intel/pantherlake/elog.c +++ b/src/soc/intel/pantherlake/elog.c @@ -8,7 +8,6 @@ #include <intelblocks/xhci.h> #include <soc/pci_devs.h> #include <soc/pm.h> -#include <soc/soc_info.h> #include <types.h> struct pme_map { @@ -30,9 +29,6 @@ static void pch_log_rp_wake_source(void) { - size_t i; - uint8_t max_port = get_max_pcie_port(); - const struct pme_map pme_map[] = { { PCI_DEVFN_PCIE1, ELOG_WAKE_SOURCE_PME_PCIE1 }, { PCI_DEVFN_PCIE2, ELOG_WAKE_SOURCE_PME_PCIE2 }, @@ -50,7 +46,7 @@ #endif }; - for (i = 0; i < MIN(max_port, ARRAY_SIZE(pme_map)); ++i) { + for (size_t i = 0; i < MIN(CONFIG_MAX_ROOT_PORTS, ARRAY_SIZE(pme_map)); i++) { if (pci_dev_is_wake_source(PCI_DEV(0, PCI_SLOT(pme_map[i].devfn), PCI_FUNC(pme_map[i].devfn)))) elog_add_event_wake(pme_map[i].wake_source, 0); diff --git a/src/soc/intel/pantherlake/include/soc/soc_info.h b/src/soc/intel/pantherlake/include/soc/soc_info.h deleted file mode 100644 index c90eb44..0000000 --- a/src/soc/intel/pantherlake/include/soc/soc_info.h +++ /dev/null @@ -1,16 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef _SOC_PANTHERLAKE_SOC_INFO_H_ -#define _SOC_PANTHERLAKE_SOC_INFO_H_ - -uint8_t get_max_usb20_port(void); -uint8_t get_max_usb30_port(void); -uint8_t get_max_tcss_port(void); -uint8_t get_max_tbt_pcie_port(void); -uint8_t get_max_pcie_port(void); -uint8_t get_max_pcie_clock(void); -uint8_t get_max_uart_port(void); -uint8_t get_max_i2c_port(void); -uint8_t get_max_gspi_port(void); - -#endif /* _SOC_PANTHERLAKE_SOC_INFO_H_ */ diff --git a/src/soc/intel/pantherlake/pcie_rp.c b/src/soc/intel/pantherlake/pcie_rp.c index 374c3aa..1b302fd 100644 --- a/src/soc/intel/pantherlake/pcie_rp.c +++ b/src/soc/intel/pantherlake/pcie_rp.c @@ -3,7 +3,6 @@ #include <intelblocks/pcie_rp.h> #include <soc/pci_devs.h> #include <soc/pcie.h> -#include <soc/soc_info.h> /* * TBT's LCAP registers are returning port index which starts from 0x10 (Usually for other PCIe diff --git a/src/soc/intel/pantherlake/romstage/fsp_params.c b/src/soc/intel/pantherlake/romstage/fsp_params.c index 2a7891b..f6ba498 100644 --- a/src/soc/intel/pantherlake/romstage/fsp_params.c +++ b/src/soc/intel/pantherlake/romstage/fsp_params.c @@ -9,7 +9,6 @@ #include <soc/msr.h> #include <soc/pcie.h> #include <soc/romstage.h> -#include <soc/soc_info.h> #define FSP_CLK_NOTUSED 0xff #define FSP_CLK_LAN 0x70 @@ -197,9 +196,7 @@ const struct soc_intel_pantherlake_config *config) { /* Disable all PCIe clock sources by default. And set RP irrelevant clock. */ - uint8_t max_clock = get_max_pcie_clock(); - - for (size_t i = 0; i < max_clock; i++) { + for (size_t i = 0; i < CONFIG_MAX_PCIE_CLOCK_SRC; i++) { if (config->pcie_clk_config_flag[i] & PCIE_CLK_FREE_RUNNING) m_cfg->PcieClkSrcUsage[i] = FSP_CLK_FREE_RUNNING; else if (config->pcie_clk_config_flag[i] & PCIE_CLK_LAN) @@ -212,7 +209,7 @@ /* PCIE ports */ m_cfg->PcieRpEnableMask = pcie_rp_enable_mask(get_pcie_rp_table()); pcie_rp_init(m_cfg, m_cfg->PcieRpEnableMask, config->pcie_rp, - get_max_pcie_port()); + CONFIG_MAX_ROOT_PORTS); } static void fill_fspm_ish_params(FSP_M_CONFIG *m_cfg, diff --git a/src/soc/intel/pantherlake/soc_info.c b/src/soc/intel/pantherlake/soc_info.c deleted file mode 100644 index 275d568..0000000 --- a/src/soc/intel/pantherlake/soc_info.c +++ /dev/null @@ -1,52 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <soc/pci_devs.h> -#include <device/pci_ids.h> -#include <device/pci_ops.h> -#include <soc/soc_info.h> -#include <intelblocks/tcss.h> - -uint8_t get_max_usb20_port(void) -{ - return CONFIG_SOC_INTEL_USB2_DEV_MAX; -} - -uint8_t get_max_usb30_port(void) -{ - return CONFIG_SOC_INTEL_USB3_DEV_MAX; -} - -uint8_t get_max_tcss_port(void) -{ - return MAX_TYPE_C_PORTS; -} - -uint8_t get_max_tbt_pcie_port(void) -{ - return CONFIG_MAX_TBT_ROOT_PORTS; -} - -uint8_t get_max_pcie_port(void) -{ - return CONFIG_MAX_ROOT_PORTS; -} - -uint8_t get_max_pcie_clock(void) -{ - return CONFIG_MAX_PCIE_CLOCK_SRC; -} - -uint8_t get_max_uart_port(void) -{ - return CONFIG_SOC_INTEL_UART_DEV_MAX; -} - -uint8_t get_max_i2c_port(void) -{ - return CONFIG_SOC_INTEL_I2C_DEV_MAX; -} - -uint8_t get_max_gspi_port(void) -{ - return CONFIG_SOC_INTEL_COMMON_BLOCK_GSPI_MAX; -} -- To view, visit https://review.coreboot.org/c/coreboot/+/84616?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: Iea26d962748116fa84afdb4afcba1098a64b6986 Gerrit-Change-Number: 84616 Gerrit-PatchSet: 1 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(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-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit October 2024