coreboot-gerrit February 2025

coreboot-gerrit@coreboot.org

1 participants
2887 discussions

[S] Change in coreboot[main]: vc/google/chromeos: Refactor Makefile to use a macro for CBFS logo
by Jérémy Compostella (Code Review) 03 Feb '25

03 Feb '25

Jérémy Compostella has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86237?usp=email ) Change subject: vc/google/chromeos: Refactor Makefile to use a macro for CBFS logo ...................................................................... vc/google/chromeos: Refactor Makefile to use a macro for CBFS logo This commit introduces a new macro, cbfs_add_bmp_file, to the ChromeOS vendor code Makefile. The macro simplifies the process of adding BMP files to the CBFS (coreboot Filesystem) by encapsulating the repetitive tasks of specifying file attributes such as file path, type, and compression flag. TEST:Both 'cb_logo.bmp' and 'cb_plus_logo.bmp' files are included with the same properties, within the coreboot firmware image. Change-Id: I827451da79931c09768965c3ad071ecdd918d367 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/86237 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Subrata Banik <subratabanik(a)google.com> --- M src/vendorcode/google/chromeos/Makefile.mk 1 file changed, 10 insertions(+), 8 deletions(-) Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved diff --git a/src/vendorcode/google/chromeos/Makefile.mk b/src/vendorcode/google/chromeos/Makefile.mk index 44d4d2b..67b50f6b 100644 --- a/src/vendorcode/google/chromeos/Makefile.mk +++ b/src/vendorcode/google/chromeos/Makefile.mk @@ -30,12 +30,14 @@ BMP_LOGO_COMPRESS_FLAG := LZ4 endif -cbfs-files-$(CONFIG_CHROMEOS_FW_SPLASH_SCREEN) += cb_logo.bmp -cb_logo.bmp-file := $(call strip_quotes,$(CONFIG_CHROMEOS_LOGO_PATH)) -cb_logo.bmp-type := raw -cb_logo.bmp-compression := $(BMP_LOGO_COMPRESS_FLAG) +define add_bmp_logo_file_to_cbfs +cbfs-files-$$($(1)) += $(2) +$(2)-file := $$(call strip_quotes,$$($(3))) +$(2)-type := raw +$(2)-compression := $$(BMP_LOGO_COMPRESS_FLAG) +endef -cbfs-files-$(CONFIG_CHROMEOS_FW_SPLASH_SCREEN) += cb_plus_logo.bmp -cb_plus_logo.bmp-file := $(call strip_quotes,$(CONFIG_CHROMEBOOK_PLUS_LOGO_PATH)) -cb_plus_logo.bmp-type := raw -cb_plus_logo.bmp-compression := $(BMP_LOGO_COMPRESS_FLAG) +$(eval $(call add_bmp_logo_file_to_cbfs,CONFIG_CHROMEOS_FW_SPLASH_SCREEN, \ + cb_logo.bmp,CONFIG_CHROMEOS_LOGO_PATH)) +$(eval $(call add_bmp_logo_file_to_cbfs,CONFIG_CHROMEOS_FW_SPLASH_SCREEN, \ + cb_plus_logo.bmp,CONFIG_CHROMEBOOK_PLUS_LOGO_PATH)) -- To view, visit https://review.coreboot.org/c/coreboot/+/86237?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: I827451da79931c09768965c3ad071ecdd918d367 Gerrit-Change-Number: 86237 Gerrit-PatchSet: 3 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[M] Change in coreboot[main]: drivers/amd/opensil/memmap.c: Factor out common memmap code to driver
by Nick Kochlowski (Code Review) 03 Feb '25

03 Feb '25

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Varshit Pandya. Nick Kochlowski has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/85634?usp=email ) Change subject: drivers/amd/opensil/memmap.c: Factor out common memmap code to driver ...................................................................... Patch Set 20: (1 comment) File src/vendorcode/amd/opensil/genoa_poc/memmap.c: https://review.coreboot.org/c/coreboot/+/85634/comment/49757531_078617a5?us… : PS19, Line 16: > i'd add a check to make sure that the size of the type field is what we expect: […] The first alternative works well! -- To view, visit https://review.coreboot.org/c/coreboot/+/85634?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: I80b9bdd7fd633c7b12d695ced5d4b9b518570d80 Gerrit-Change-Number: 85634 Gerrit-PatchSet: 20 Gerrit-Owner: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Mon, 03 Feb 2025 16:42:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[M] Change in coreboot[main]: drivers/amd/opensil/memmap.c: Factor out common memmap code to driver
by Nick Kochlowski (Code Review) 03 Feb '25

03 Feb '25

Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Nick Kochlowski, Varshit Pandya. Hello Felix Held, Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Varshit Pandya, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85634?usp=email to look at the new patch set (#20). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: drivers/amd/opensil/memmap.c: Factor out common memmap code to driver ...................................................................... drivers/amd/opensil/memmap.c: Factor out common memmap code to driver Refactor the vendercode openSIL memory map code and move all common calls that do not require any openSIL headers to the driver. Improve the legibility of the logic to return memory hole type string. Change-Id: I80b9bdd7fd633c7b12d695ced5d4b9b518570d80 Signed-off-by: Nicolas Kochlowski <nickkochlowski(a)gmail.com> --- M src/drivers/amd/opensil/Makefile.mk A src/drivers/amd/opensil/memmap.c M src/drivers/amd/opensil/opensil.h M src/soc/amd/genoa_poc/domain.c M src/soc/amd/phoenix/memmap.c M src/vendorcode/amd/opensil/genoa_poc/memmap.c M src/vendorcode/amd/opensil/opensil.h M src/vendorcode/amd/opensil/stub/ramstage.c 8 files changed, 124 insertions(+), 90 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/85634/20 -- To view, visit https://review.coreboot.org/c/coreboot/+/85634?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: I80b9bdd7fd633c7b12d695ced5d4b9b518570d80 Gerrit-Change-Number: 85634 Gerrit-PatchSet: 20 Gerrit-Owner: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com>

1 0

[S] Change in coreboot[main]: mb/starlabs/byte_adl: Disconnect unused GPIOs
by Sean Rhodes (Code Review) 03 Feb '25

03 Feb '25

Sean Rhodes has submitted this change. ( https://review.coreboot.org/c/coreboot/+/86134?usp=email ) Change subject: mb/starlabs/byte_adl: Disconnect unused GPIOs ...................................................................... mb/starlabs/byte_adl: Disconnect unused GPIOs Change-Id: Ie4c2c32c0740e2ad763881f3f4d5920387f49c79 Signed-off-by: Sean Rhodes <sean(a)starlabs.systems> Reviewed-on: https://review.coreboot.org/c/coreboot/+/86134 Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/starlabs/byte_adl/variants/mk_ii/gpio.c 1 file changed, 10 insertions(+), 10 deletions(-) Approvals: Matt DeVillier: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/mainboard/starlabs/byte_adl/variants/mk_ii/gpio.c b/src/mainboard/starlabs/byte_adl/variants/mk_ii/gpio.c index ecd4082..f17e3ae 100644 --- a/src/mainboard/starlabs/byte_adl/variants/mk_ii/gpio.c +++ b/src/mainboard/starlabs/byte_adl/variants/mk_ii/gpio.c @@ -27,11 +27,11 @@ /* GPD3: Power Button */ PAD_CFG_NF(GPD3, UP_20K, DEEP, NF1), /* GPD4: Sleep S3 */ - PAD_CFG_NF(GPD4, NONE, DEEP, NF1), + PAD_NC(GPD4, NONE), /* GPD5: Sleep S4 */ - PAD_CFG_NF(GPD5, NONE, DEEP, NF1), + PAD_NC(GPD5, NONE), /* GPD6: Sleep A */ - PAD_CFG_NF(GPD6, NONE, DEEP, NF1), + PAD_NC(GPD6, NONE), /* GPD7: Power Adapter Disable */ PAD_CFG_GPO(GPD7, 0, PWROK), /* GPD8: Suspend Clock */ @@ -39,7 +39,7 @@ /* GPD9: Wireless LAN Sleep */ PAD_CFG_NF(GPD9, NONE, DEEP, NF1), /* GPD10: Sleep S5 */ - PAD_CFG_NF(GPD10, NONE, DEEP, NF1), + PAD_NC(GPD10, NONE), /* GPD11: LAN PHY Enable */ PAD_NC(GPD11, NONE), @@ -58,7 +58,7 @@ PAD_NC(GPP_A8, NONE), /* A9: ESPI Clock */ /* A10: ESPI Reset */ - /* A11: Not Connected */ + /* A11: Webcam Camera Reset */ PAD_NC(GPP_A11, NONE), /* A12: PCH M.2 SSD PEDET */ PAD_CFG_NF(GPP_A12, NONE, DEEP, NF1), @@ -111,7 +111,7 @@ /* B11: I2C PMC PD Interrupt Test Point 28 */ PAD_CFG_NF(GPP_B11, NONE, DEEP, NF1), /* B12: PM SLP S0 */ - PAD_CFG_NF(GPP_B12, NONE, DEEP, NF1), + PAD_NC(GPP_B12, NONE), /* B13: PLT RST */ PAD_CFG_NF(GPP_B13, NONE, DEEP, NF1), /* B14: Top Swap Override Weak Internal PD 20K @@ -265,8 +265,8 @@ PAD_NC(GPP_E11, NONE), /* E12: Touchpad Interrupt */ PAD_NC(GPP_E12, NONE), - /* E13: PWD Amplifier Input */ - PAD_CFG_GPO(GPP_E13, 0, PLTRST), + /* E13: Not connected */ + PAD_NC(GPP_E13, NONE), /* E14: EDP HPD */ PAD_CFG_NF(GPP_E14, NONE, DEEP, NF1), /* E15: Not Connected LAN RESET */ @@ -342,7 +342,7 @@ /* H0: PCH M.2 SSD Reset */ PAD_CFG_GPO_GPIO_DRIVER(GPP_H0, 1, PLTRST, NONE), /* H1: BFX Strap 2 Bit 3 Weak Internal PD 20K */ - PAD_CFG_GPO(GPP_H1, 1, PLTRST), + PAD_CFG_GPO(GPP_H1, 0, DEEP), /* H2: Wireless LAN Reset */ PAD_CFG_GPO(GPP_H2, 1, PLTRST), /* H3: Not Connected */ @@ -362,7 +362,7 @@ /* H12: Not Connected */ PAD_NC(GPP_H12, NONE), /* H13: PCH M.2 SSD Device Sleep */ - PAD_CFG_NF(GPP_H13, NONE, DEEP, NF5), + PAD_NC(GPP_H13, NONE), /* H14: Not Connected */ PAD_NC(GPP_H14, NONE), /* H15: DDPB Control Clock */ -- To view, visit https://review.coreboot.org/c/coreboot/+/86134?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: Ie4c2c32c0740e2ad763881f3f4d5920387f49c79 Gerrit-Change-Number: 86134 Gerrit-PatchSet: 5 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in coreboot[main]: mb/google/trulo/var/uldrenite: Add fw_config probe for Cellular
by John Su (Code Review) 03 Feb '25

03 Feb '25

John Su has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/86255?usp=email ) Change subject: mb/google/trulo/var/uldrenite: Add fw_config probe for Cellular ...................................................................... mb/google/trulo/var/uldrenite: Add fw_config probe for Cellular Use fw_config to probe Cellular. BUG=b:392040004 BRANCH=firmware-trulo-15217.771.B TEST=emerge-nissa coreboot chromeos-bootimage Change-Id: Ib664f543c6012b44a0a604d0943416519d92a057 Signed-off-by: John Su <john_su(a)compal.corp-partner.google.com> --- M src/mainboard/google/brya/variants/uldrenite/overridetree.cb M src/mainboard/google/brya/variants/uldrenite/variant.c 2 files changed, 32 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/86255/1 diff --git a/src/mainboard/google/brya/variants/uldrenite/overridetree.cb b/src/mainboard/google/brya/variants/uldrenite/overridetree.cb index 3c06750..82ebc72 100644 --- a/src/mainboard/google/brya/variants/uldrenite/overridetree.cb +++ b/src/mainboard/google/brya/variants/uldrenite/overridetree.cb @@ -1,4 +1,8 @@ fw_config + field DB_Cellular 1 2 + option Cellular_ABSENT 0 + option Cellular_RW350R 1 + end field TOUCHSCREEN 4 4 option TOUCHSCREEN_UNKNOWN 0 option TOUCHSCREEN_NONE 1 diff --git a/src/mainboard/google/brya/variants/uldrenite/variant.c b/src/mainboard/google/brya/variants/uldrenite/variant.c index 96b2291..cb01f82 100644 --- a/src/mainboard/google/brya/variants/uldrenite/variant.c +++ b/src/mainboard/google/brya/variants/uldrenite/variant.c @@ -37,6 +37,17 @@ PAD_NC(GPP_E17, NONE), }; +static const struct pad_config lte_disable_pads[] = { + /* A8 : WWAN_RF_DISABLE_ODL */ + PAD_NC(GPP_A8, NONE), + /* F12 : WWAN_RST_L */ + PAD_NC_LOCK(GPP_F12, NONE, LOCK_CONFIG), + /* F13 : PLTRST_WWAN# */ + PAD_NC(GPP_F13, NONE), + /* H23 : WWAN_EN */ + PAD_NC(GPP_H23, NONE), +}; + void fw_config_gpio_padbased_override(struct pad_config *padbased_table) { if (fw_config_probe(FW_CONFIG(TOUCHSCREEN, TOUCHSCREEN_NONE))) { @@ -44,18 +55,25 @@ gpio_padbased_override(padbased_table, touchscreen_disable_pads, ARRAY_SIZE(touchscreen_disable_pads)); } + if (fw_config_probe(FW_CONFIG(DB_Cellular, Cellular_ABSENT))) { + printk(BIOS_INFO, "Disable Cellular GPIO pins.\n"); + gpio_padbased_override(padbased_table, lte_disable_pads, + ARRAY_SIZE(lte_disable_pads)); + } } void variant_init(void) { - /* - * RW350R power on seuqence: - * De-assert WWAN_EN -> 20ms -> de-assert WWAN_RST -> 30ms -> - * de-assert WWAN_PERST - */ - gpio_configure_pads(rw350r_en_pad, ARRAY_SIZE(rw350r_en_pad)); - mdelay(RW350R_RST_DELAY_MS); - gpio_configure_pads(rw350r_rst_pad, ARRAY_SIZE(rw350r_rst_pad)); - mdelay(RW350R_PERST_DELAY_MS); - gpio_configure_pads(rw350r_perst_pad, ARRAY_SIZE(rw350r_perst_pad)); + if (fw_config_probe(FW_CONFIG(DB_Cellular, Cellular_RW350R))) { + /* + * RW350R power on seuqence: + * De-assert WWAN_EN -> 20ms -> de-assert WWAN_RST -> 30ms -> + * de-assert WWAN_PERST + */ + gpio_configure_pads(rw350r_en_pad, ARRAY_SIZE(rw350r_en_pad)); + mdelay(RW350R_RST_DELAY_MS); + gpio_configure_pads(rw350r_rst_pad, ARRAY_SIZE(rw350r_rst_pad)); + mdelay(RW350R_PERST_DELAY_MS); + gpio_configure_pads(rw350r_perst_pad, ARRAY_SIZE(rw350r_perst_pad)); + } } -- To view, visit https://review.coreboot.org/c/coreboot/+/86255?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: Ib664f543c6012b44a0a604d0943416519d92a057 Gerrit-Change-Number: 86255 Gerrit-PatchSet: 1 Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com>

1 0

[L] Change in coreboot[main]: [wip]crossgcc: upgrade binutils version to 2.44
by Elyes Haouas (Code Review) 03 Feb '25

03 Feb '25

Attention is currently required from: Martin L Roth. Hello Martin L Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86254?usp=email to look at the new patch set (#2). Change subject: [wip]crossgcc: upgrade binutils version to 2.44 ...................................................................... [wip]crossgcc: upgrade binutils version to 2.44 Change-Id: Ic78053f55c59de7af16fca0265d0d23fadfb20f6 Signed-off-by: Elyes Haouas <ehaouas(a)noos.fr> --- M util/crossgcc/buildgcc D util/crossgcc/patches/binutils-2.43.1_no-makeinfo.patch R util/crossgcc/patches/binutils-2.44_as-ipxe.patch A util/crossgcc/patches/binutils-2.44_no-makeinfo.patch D util/crossgcc/sum/binutils-2.43.1.tar.xz.cksum A util/crossgcc/sum/binutils-2.44.tar.xz.cksum 6 files changed, 219 insertions(+), 243 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/86254/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86254?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: Ic78053f55c59de7af16fca0265d0d23fadfb20f6 Gerrit-Change-Number: 86254 Gerrit-PatchSet: 2 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com>

1 0

[M] Change in coreboot[main]: mb/amd/birman_plus/ec: Rectify ECRAM register bits
by Ana Carolina Cabral (Code Review) 03 Feb '25

03 Feb '25

Ana Carolina Cabral has posted comments on this change by Ana Carolina Cabral. ( https://review.coreboot.org/c/coreboot/+/86084?usp=email ) Change subject: mb/amd/birman_plus/ec: Rectify ECRAM register bits ...................................................................... Patch Set 15: (1 comment) File src/mainboard/amd/birman_plus/ec.c: https://review.coreboot.org/c/coreboot/+/86084/comment/b605a8c8_a9630580?us… : PS13, Line 36: _N > no, this should be EC7_WL_RADIO_DIS Done -- To view, visit https://review.coreboot.org/c/coreboot/+/86084?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: I1a13d99a55a4aa02a5cb0e67ffa4ed555f91a471 Gerrit-Change-Number: 86084 Gerrit-PatchSet: 15 Gerrit-Owner: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Mon, 03 Feb 2025 14:03:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Ana Carolina Cabral <ana.cpmelo95(a)gmail.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[M] Change in coreboot[main]: drivers/amd/opensil/memmap.c: Factor out common memmap code to driver
by Felix Held (Code Review) 03 Feb '25

03 Feb '25

Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Nick Kochlowski, Varshit Pandya. Felix Held has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/85634?usp=email ) Change subject: drivers/amd/opensil/memmap.c: Factor out common memmap code to driver ...................................................................... Patch Set 19: (1 comment) File src/vendorcode/amd/opensil/genoa_poc/memmap.c: https://review.coreboot.org/c/coreboot/+/85634/comment/be44d3dd_3e43ae93?us… : PS16, Line 37: for (i = 0; i < ARRAY_SIZE(types); i++) : if (enum_type == types[i].type) : break; : if (i == ARRAY_SIZE(types)) : return "Unknown type"; : return types[i].string; > i'm fine with also doing that in the same patch please mention that in the commit message though -- To view, visit https://review.coreboot.org/c/coreboot/+/85634?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: I80b9bdd7fd633c7b12d695ced5d4b9b518570d80 Gerrit-Change-Number: 85634 Gerrit-PatchSet: 19 Gerrit-Owner: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Mon, 03 Feb 2025 13:55:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nick Kochlowski <nickkochlowski(a)gmail.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[M] Change in coreboot[main]: drivers/amd/opensil/memmap.c: Factor out common memmap code to driver
by Felix Held (Code Review) 03 Feb '25

03 Feb '25

Attention is currently required from: Fred Reitberger, Jason Glenesk, Martin L Roth, Matt DeVillier, Nick Kochlowski, Varshit Pandya. Felix Held has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/85634?usp=email ) Change subject: drivers/amd/opensil/memmap.c: Factor out common memmap code to driver ...................................................................... Patch Set 19: (2 comments) File src/vendorcode/amd/opensil/genoa_poc/memmap.c: https://review.coreboot.org/c/coreboot/+/85634/comment/379f72ad_3f8657f5?us… : PS16, Line 37: for (i = 0; i < ARRAY_SIZE(types); i++) : if (enum_type == types[i].type) : break; : if (i == ARRAY_SIZE(types)) : return "Unknown type"; : return types[i].string; > Agreed, it's much easier to read this way. I added this change to the new patchset. […] i'm fine with also doing that in the same patch File src/vendorcode/amd/opensil/genoa_poc/memmap.c: https://review.coreboot.org/c/coreboot/+/85634/comment/311a8f8c_d00a1443?us… : PS19, Line 16: i'd add a check to make sure that the size of the type field is what we expect: _Static_assert(sizeof(uint32_t) == sizeof(((MEMORY_HOLE_DESCRIPTOR){0}).Type), "Unexpected size of MEMORY_HOLE_TYPES in the MEMORY_HOLE_DESCRIPTOR struct which doesn't match the code in drivers/amd/opensil/memmap.c"); if that doesn't work the next best thing would be _Static_assert(sizeof(uint32_t) == sizeof(MEMORY_HOLE_TYPES), "Unexpected size of MEMORY_HOLE_TYPES which doesn't match the code in drivers/amd/opensil/memmap.c"); This check needs to be in this file, since only this file has the definitions of MEMORY_HOLE_DESCRIPTOR and MEMORY_HOLE_TYPES -- To view, visit https://review.coreboot.org/c/coreboot/+/85634?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: I80b9bdd7fd633c7b12d695ced5d4b9b518570d80 Gerrit-Change-Number: 85634 Gerrit-PatchSet: 19 Gerrit-Owner: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Mon, 03 Feb 2025 13:55:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nick Kochlowski <nickkochlowski(a)gmail.com> Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>

1 0

[XS] Change in coreboot[main]: soc/mediatek/mt8196: Add TF-A static library path
by Yu-Ping Wu (Code Review) 03 Feb '25

03 Feb '25

Attention is currently required from: Gavin Liu, Hung-Te Lin. Yu-Ping Wu has posted comments on this change by Gavin Liu. ( https://review.coreboot.org/c/coreboot/+/86252?usp=email ) Change subject: soc/mediatek/mt8196: Add TF-A static library path ...................................................................... Patch Set 2: (1 comment) File src/soc/mediatek/mt8196/Makefile.mk: https://review.coreboot.org/c/coreboot/+/86252/comment/8520f132_1632afcb?us… : PS1, Line 96: libmtk.a > We only need `lib` prefix. We can rename it to `libbl31.a` or `libtf-a. […] I like libbl31, as it's more consistent with BL31_MAKEARGS. libmtk is a too general name. Regarding the tf-a patch, the chromiumos ebuild can rename the file, if you don't want to modify the patch. -- To view, visit https://review.coreboot.org/c/coreboot/+/86252?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: I858686ede3730fb70f71565ca3593e7eb4c460d2 Gerrit-Change-Number: 86252 Gerrit-PatchSet: 2 Gerrit-Owner: Gavin Liu <gavin.liu(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Gavin Liu <gavin.liu(a)mediatek.corp-partner.google.com> Gerrit-Comment-Date: Mon, 03 Feb 2025 13:42:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Gavin Liu <gavin.liu(a)mediatek.corp-partner.google.com> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2025