coreboot-gerrit September 2024

coreboot-gerrit@coreboot.org

1 participants
3342 discussions

[S] Change in coreboot[main]: mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186
by Yu-Ping Wu (Code Review) 19 Sep '24

19 Sep '24

Attention is currently required from: Hung-Te Lin, Knox Chiou, Paul Menzel, Xinxiong Xu, Yang Wu, Yidi Lin. Yu-Ping Wu has posted comments on this change by Yang Wu. ( https://review.coreboot.org/c/coreboot/+/84342?usp=email ) Change subject: mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186 ...................................................................... Patch Set 8: (3 comments) File src/mainboard/google/corsola/boardid.c: https://review.coreboot.org/c/coreboot/+/84342/comment/291a24a0_7686c458?us… : PS6, Line 118: if (cached_sku_code == CROS_SKU_UNPROVISIONED || : cached_sku_code == CROS_SKU_UNKNOWN) { : printk(BIOS_WARNING, "SKU code from EC: 0x%x\n", cached_sku_code); : : if (CONFIG(BOARD_GOOGLE_STARYU_COMMON)) { : if (get_cpu_id() == MTK_CPU_ID_MT8186T) { : /* Reserve last 4 bits to report PANEL_ID */ : cached_sku_code = 0x7FFFFEF0UL | panel_id(); : } else { : cached_sku_code = 0x7FFFFFF0UL | panel_id(); : } : } else if (get_cpu_id() == MTK_CPU_ID_MT8186T && : !CONFIG(BOARD_GOOGLE_VOLTORB) && : !CONFIG(BOARD_GOOGLE_SQUIRTLE)) { : /* : * Distinguish MT8186T from MT8186 to select different device trees : * in the payload. Voltorb/Squirtle are exception here, because right now : * they use 0x7FFFFFFF in the DTS compatible string. We'll need to change : * that to 0x7FFFFEFF in kernel before the exception can be dropped. : */ : cached_sku_code = CROS_SKU_UNPROVISIONED_MT8186T; : } : } > or […] That also works. Given that we've decided to drop the exception for voltorb and squirtle here, I think PS8 looks clean enough. File src/mainboard/google/corsola/boardid.c: https://review.coreboot.org/c/coreboot/+/84342/comment/fde96ee4_42af43b9?us… : PS8, Line 119: cached_sku_code Align with `cached_sku_code` above. https://review.coreboot.org/c/coreboot/+/84342/comment/09ce8001_61a55284?us… : PS8, Line 125: if (CONFIG(BOARD_GOOGLE_STARYU_COMMON)) { Keep the `/* Reserve last 4 bits to report PANEL_ID */` comment. -- To view, visit https://review.coreboot.org/c/coreboot/+/84342?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: I91306d3abd508e104851916882fb36a4fd302036 Gerrit-Change-Number: 84342 Gerrit-PatchSet: 8 Gerrit-Owner: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Knox Chiou <knoxchiou(a)google.com> Gerrit-Reviewer: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Knox Chiou <knoxchiou(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Attention: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Thu, 19 Sep 2024 03:06:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[XS] Change in coreboot[main]: efi: Set EFIAPI to 32-bit ABI for FSP1_1
by Subrata Banik (Code Review) 19 Sep '24

19 Sep '24

Attention is currently required from: Jérémy Compostella. Subrata Banik has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/84402?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: efi: Set EFIAPI to 32-bit ABI for FSP1_1 ...................................................................... Patch Set 5: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/84402/comment/e69c9109_6a721c66?us… : PS4, Line 16: is not supported for this target is this intention to drop the TEST tag now (with latest commit ?) -- To view, visit https://review.coreboot.org/c/coreboot/+/84402?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: I914f73ff06bfb801fc319b45b23d7ce4cb7a6d5d Gerrit-Change-Number: 84402 Gerrit-PatchSet: 5 Gerrit-Owner: 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> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Comment-Date: Thu, 19 Sep 2024 03:02:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186
by Hung-Te Lin (Code Review) 19 Sep '24

19 Sep '24

Attention is currently required from: Knox Chiou, Paul Menzel, Xinxiong Xu, Yang Wu, Yidi Lin, Yu-Ping Wu. Hung-Te Lin has posted comments on this change by Yang Wu. ( https://review.coreboot.org/c/coreboot/+/84342?usp=email ) Change subject: mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186 ...................................................................... Patch Set 8: (1 comment) File src/mainboard/google/corsola/boardid.c: https://review.coreboot.org/c/coreboot/+/84342/comment/b472376d_6939dffe?us… : PS6, Line 118: if (cached_sku_code == CROS_SKU_UNPROVISIONED || : cached_sku_code == CROS_SKU_UNKNOWN) { : printk(BIOS_WARNING, "SKU code from EC: 0x%x\n", cached_sku_code); : : if (CONFIG(BOARD_GOOGLE_STARYU_COMMON)) { : if (get_cpu_id() == MTK_CPU_ID_MT8186T) { : /* Reserve last 4 bits to report PANEL_ID */ : cached_sku_code = 0x7FFFFEF0UL | panel_id(); : } else { : cached_sku_code = 0x7FFFFFF0UL | panel_id(); : } : } else if (get_cpu_id() == MTK_CPU_ID_MT8186T && : !CONFIG(BOARD_GOOGLE_VOLTORB) && : !CONFIG(BOARD_GOOGLE_SQUIRTLE)) { : /* : * Distinguish MT8186T from MT8186 to select different device trees : * in the payload. Voltorb/Squirtle are exception here, because right now : * they use 0x7FFFFFFF in the DTS compatible string. We'll need to change : * that to 0x7FFFFEFF in kernel before the exception can be dropped. : */ : cached_sku_code = CROS_SKU_UNPROVISIONED_MT8186T; : } : } > or […] ```` cached_sku_code = (CROS_SKU_UNPROVISIONED & ~0x10F) | soc_type << 16 | panel << 0; ```` Or we can use the SETBITFIELD APIs to modify the sku_code to make it easier to read. -- To view, visit https://review.coreboot.org/c/coreboot/+/84342?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: I91306d3abd508e104851916882fb36a4fd302036 Gerrit-Change-Number: 84342 Gerrit-PatchSet: 8 Gerrit-Owner: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Knox Chiou <knoxchiou(a)google.com> Gerrit-Reviewer: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Knox Chiou <knoxchiou(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Attention: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Thu, 19 Sep 2024 03:01:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[S] Change in coreboot[main]: mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186
by Hung-Te Lin (Code Review) 19 Sep '24

19 Sep '24

Attention is currently required from: Knox Chiou, Paul Menzel, Xinxiong Xu, Yidi Lin, Yu-Ping Wu. Hung-Te Lin has posted comments on this change by Yang Wu. ( https://review.coreboot.org/c/coreboot/+/84342?usp=email ) Change subject: mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186 ...................................................................... Patch Set 6: (1 comment) File src/mainboard/google/corsola/boardid.c: https://review.coreboot.org/c/coreboot/+/84342/comment/6df06e59_f922eef2?us… : PS6, Line 118: if (cached_sku_code == CROS_SKU_UNPROVISIONED || : cached_sku_code == CROS_SKU_UNKNOWN) { : printk(BIOS_WARNING, "SKU code from EC: 0x%x\n", cached_sku_code); : : if (CONFIG(BOARD_GOOGLE_STARYU_COMMON)) { : if (get_cpu_id() == MTK_CPU_ID_MT8186T) { : /* Reserve last 4 bits to report PANEL_ID */ : cached_sku_code = 0x7FFFFEF0UL | panel_id(); : } else { : cached_sku_code = 0x7FFFFFF0UL | panel_id(); : } : } else if (get_cpu_id() == MTK_CPU_ID_MT8186T && : !CONFIG(BOARD_GOOGLE_VOLTORB) && : !CONFIG(BOARD_GOOGLE_SQUIRTLE)) { : /* : * Distinguish MT8186T from MT8186 to select different device trees : * in the payload. Voltorb/Squirtle are exception here, because right now : * they use 0x7FFFFFFF in the DTS compatible string. We'll need to change : * that to 0x7FFFFEFF in kernel before the exception can be dropped. : */ : cached_sku_code = CROS_SKU_UNPROVISIONED_MT8186T; : } : } > What about […] or ```` /* Only Staryu has MIPI panels. */ uint32_t panel = CONFIG(BOARD_GOOGLE_STARYU_COMMON) ? panel_id() : 0xF; /* Check if we are running standard 8186 instead of 8186T. */ uint32_t soc_type = get_cpu_id() == MTK_CPU_ID_MT8186T ? 0 : 1; /* Voltorb and Squirtle doesn't have the new 8186T settings in the DTS. * TODO(xxx): b/XXXXX: Add 8186T compatible DTS to Voltorb and Squirtle * so we can remove this special rule. */ if (CONFIG(BOARD_GOOGLE_VOLTORB) || CONFIG(BOARD_GOOGLE_SQUIRTLE)) soc_type = 1; cached_sku_code =0x7FFFFEF0 | soc_type << 16 | panel << 0; ```` -- To view, visit https://review.coreboot.org/c/coreboot/+/84342?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: I91306d3abd508e104851916882fb36a4fd302036 Gerrit-Change-Number: 84342 Gerrit-PatchSet: 6 Gerrit-Owner: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Knox Chiou <knoxchiou(a)google.com> Gerrit-Reviewer: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Knox Chiou <knoxchiou(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Thu, 19 Sep 2024 02:57:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[XS] Change in coreboot[main]: xcompile: Disable unterminated-string-initialization warning
by Elyes Haouas (Code Review) 19 Sep '24

19 Sep '24

Attention is currently required from: Martin L Roth. Elyes Haouas has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/84417?usp=email ) Change subject: xcompile: Disable unterminated-string-initialization warning ...................................................................... Patch Set 1: Code-Review-1 -- To view, visit https://review.coreboot.org/c/coreboot/+/84417?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: Icca87e333daf240b8dbc23c71863500294a37e74 Gerrit-Change-Number: 84417 Gerrit-PatchSet: 1 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Comment-Date: Thu, 19 Sep 2024 02:39:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186
by Yang Wu (Code Review) 19 Sep '24

19 Sep '24

Attention is currently required from: Hung-Te Lin, Knox Chiou, Paul Menzel, Xinxiong Xu, Yidi Lin, Yu-Ping Wu. Yang Wu has posted comments on this change by Yang Wu. ( https://review.coreboot.org/c/coreboot/+/84342?usp=email ) Change subject: mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186 ...................................................................... Patch Set 8: (7 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/84342/comment/40ce9867_e535cfb5?us… : PS6, Line 9: default > unprovisioned Done https://review.coreboot.org/c/coreboot/+/84342/comment/25a2cc2b_0d5c7ae2?us… : PS6, Line 9: SKUD > SKU Done https://review.coreboot.org/c/coreboot/+/84342/comment/4728b7d4_5a3acc5a?us… : PS6, Line 10: load > use Done https://review.coreboot.org/c/coreboot/+/84342/comment/28981f4d_a84df6dd?us… : PS6, Line 10: dts > DTS Done https://review.coreboot.org/c/coreboot/+/84342/comment/3778678a_4f343c06?us… : PS6, Line 11: 0x7fffffff > unprovisioned Done File src/mainboard/google/corsola/boardid.c: https://review.coreboot.org/c/coreboot/+/84342/comment/fa2bb022_4727c066?us… : PS6, Line 131: BOARD_GOOGLE_SQUIRTLE > > Although Squirlte has only one DTS, since 0x7FFFFEFF is not configured in config. […] Done https://review.coreboot.org/c/coreboot/+/84342/comment/8f9377e6_95a8a576?us… : PS6, Line 118: if (cached_sku_code == CROS_SKU_UNPROVISIONED || : cached_sku_code == CROS_SKU_UNKNOWN) { : printk(BIOS_WARNING, "SKU code from EC: 0x%x\n", cached_sku_code); : : if (CONFIG(BOARD_GOOGLE_STARYU_COMMON)) { : if (get_cpu_id() == MTK_CPU_ID_MT8186T) { : /* Reserve last 4 bits to report PANEL_ID */ : cached_sku_code = 0x7FFFFEF0UL | panel_id(); : } else { : cached_sku_code = 0x7FFFFFF0UL | panel_id(); : } : } else if (get_cpu_id() == MTK_CPU_ID_MT8186T && : !CONFIG(BOARD_GOOGLE_VOLTORB) && : !CONFIG(BOARD_GOOGLE_SQUIRTLE)) { : /* : * Distinguish MT8186T from MT8186 to select different device trees : * in the payload. Voltorb/Squirtle are exception here, because right now : * they use 0x7FFFFFFF in the DTS compatible string. We'll need to change : * that to 0x7FFFFEFF in kernel before the exception can be dropped. : */ : cached_sku_code = CROS_SKU_UNPROVISIONED_MT8186T; : } : } > What about […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/84342?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: I91306d3abd508e104851916882fb36a4fd302036 Gerrit-Change-Number: 84342 Gerrit-PatchSet: 8 Gerrit-Owner: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Knox Chiou <knoxchiou(a)google.com> Gerrit-Reviewer: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Knox Chiou <knoxchiou(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Thu, 19 Sep 2024 02:39:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Knox Chiou <knoxchiou(a)google.com> Comment-In-Reply-To: Xinxiong Xu <xuxinxiong(a)huaqin.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

[S] Change in coreboot[main]: mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186
by Yang Wu (Code Review) 19 Sep '24

19 Sep '24

Attention is currently required from: Hung-Te Lin, Knox Chiou, Paul Menzel, Xinxiong Xu, Yang Wu. Hello Hung-Te Lin, Knox Chiou, Xinxiong Xu, Yidi Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84342?usp=email to look at the new patch set (#8). Change subject: mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186 ...................................................................... mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186 For MT8186, factory pre-flashed 0x7fffffff as unprovisioned SKU ID and kernel can use the corresponding DTS file. To make MT8186T functional on unprovisioned devices, change the SKU ID to 0x7ffffeff, so that the correct dts file will be selected by the payload. BUG=b:365730137 TEST=1. Pre-flashed 0x7fffffff and boot OS. 2. Check OS boot normally by 0x7ffffeff. BRANCH=corsola Change-Id: I91306d3abd508e104851916882fb36a4fd302036 Signed-off-by: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> --- M src/mainboard/google/corsola/boardid.c 1 file changed, 13 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/84342/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/84342?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: I91306d3abd508e104851916882fb36a4fd302036 Gerrit-Change-Number: 84342 Gerrit-PatchSet: 8 Gerrit-Owner: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Knox Chiou <knoxchiou(a)google.com> Gerrit-Reviewer: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Knox Chiou <knoxchiou(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Attention: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com>

1 0

[S] Change in coreboot[main]: mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186
by Yang Wu (Code Review) 19 Sep '24

19 Sep '24

Attention is currently required from: Hung-Te Lin, Knox Chiou, Paul Menzel, Xinxiong Xu, Yang Wu. Hello Hung-Te Lin, Knox Chiou, Xinxiong Xu, Yidi Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84342?usp=email to look at the new patch set (#7). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186 ...................................................................... mb/google/corsola: Distinguish MT8186T's SKU ID from MT8186 For MT8186, factory pre-flashed 0x7fffffff as unprovisioned SKUD ID and kernel can use the corresponding DTS file. To make MT8186T functional on unprovisioned devices, change the SKU ID to 0x7ffffeff, so that the correct dts file will be selected by the payload. BUG=b:365730137 TEST=1. Pre-flashed 0x7fffffff and boot OS. 2. Check OS boot normally by 0x7ffffeff. BRANCH=corsola Change-Id: I91306d3abd508e104851916882fb36a4fd302036 Signed-off-by: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> --- M src/mainboard/google/corsola/boardid.c 1 file changed, 13 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/84342/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/84342?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: I91306d3abd508e104851916882fb36a4fd302036 Gerrit-Change-Number: 84342 Gerrit-PatchSet: 7 Gerrit-Owner: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Knox Chiou <knoxchiou(a)google.com> Gerrit-Reviewer: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Knox Chiou <knoxchiou(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Xinxiong Xu <xuxinxiong(a)huaqin.corp-partner.google.com> Gerrit-Attention: Yang Wu <wuyang5(a)huaqin.corp-partner.google.com>

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Simplify FSP global reset definition
by Jérémy Compostella (Code Review) 19 Sep '24

19 Sep '24

Attention is currently required from: Andrey Petrov, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Jérémy Compostella, Kapil Porwal, Lean Sheng Tan, Nick Vaccaro, Paul Menzel, Pranava Y N, Rishika Raj, Ronak Kanabar, Sean Rhodes, Subrata Banik, Tarun, Werner Zeh. Hello Andrey Petrov, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Lean Sheng Tan, Nick Vaccaro, Pranava Y N, Rishika Raj, Ronak Kanabar, Sean Rhodes, Subrata Banik, Tarun, Werner Zeh, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/84356?usp=email to look at the new patch set (#11). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: drivers/intel/fsp2_0: Simplify FSP global reset definition ...................................................................... drivers/intel/fsp2_0: Simplify FSP global reset definition According to all FSP 2.x specifications, the OEM status code have the highest bit clear and the next to highest bit set. Therefore, the FSP reset status varies with the FSP architecture. This commits reduces the global reset Kconfig to a single one specifying the suffix X of `FSP_STATUS_RESET_REQUIRED_X'. `fsp_reset.c' constructs the name of the constant by macro expansion and concatenation to determine which `FSP_STATUS_RESET_REQUIRED_X' constant to map to the global reset request. Compared to the previous implementation: - It is scalable (the number of Kconfig should not evolve with FSP specification addition new OEM status code). - It does not introduce any duplicate and prevent wrong values C header files and Kconfig. - It use the FSP (or other) header files coreboot is compiled against for `FSP_STATUS_RESET_REQUIRED_X' constants - It works with both 32-bit and 64-bit FSP binaries Since all the platforms defining SOC_INTEL_COMMON_FSP_RESET (tigerlake, alderlake, jasperlake, meteorlake, pantherlake, elkhartlake, skylake, cannonlake and apollolake) pick a global reset value, this commit default the most common: 3. As a result of using the appropriate constants and because FSP 2.x status code varies with the FSP architecture, this commit includes some necessary changes to a few function prototypes manipulating FSP status codes. BUG=b:348678529 TEST=FSP-s Global reset request is handled properly on pantherlake fatcat Change-Id: I914f73ff06bfb801fc319b45b23d7ce4cb7a6d5f Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/drivers/intel/fsp2_0/Kconfig M src/soc/intel/alderlake/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/common/fsp_reset.c M src/soc/intel/common/reset.h M src/soc/intel/elkhartlake/Kconfig M src/soc/intel/jasperlake/Kconfig M src/soc/intel/meteorlake/Kconfig M src/soc/intel/meteorlake/chip.c M src/soc/intel/pantherlake/Kconfig M src/soc/intel/pantherlake/chip.c M src/soc/intel/skylake/Kconfig M src/soc/intel/tigerlake/Kconfig 14 files changed, 32 insertions(+), 62 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/84356/11 -- To view, visit https://review.coreboot.org/c/coreboot/+/84356?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: I914f73ff06bfb801fc319b45b23d7ce4cb7a6d5f Gerrit-Change-Number: 84356 Gerrit-PatchSet: 11 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.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: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.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: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com>

1 0

[M] Change in coreboot[main]: drivers/intel/fsp2_0: Simplify FSP global reset definition
by Jérémy Compostella (Code Review) 19 Sep '24

19 Sep '24

Attention is currently required from: Andrey Petrov, Dinesh Gehlot, Eran Mitrani, Jakub Czapiga, Jayvik Desai, Kapil Porwal, Lean Sheng Tan, Nick Vaccaro, Paul Menzel, Pranava Y N, Rishika Raj, Ronak Kanabar, Sean Rhodes, Subrata Banik, Tarun, Werner Zeh. Jérémy Compostella has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/84356?usp=email ) Change subject: drivers/intel/fsp2_0: Simplify FSP global reset definition ...................................................................... Patch Set 9: (4 comments) File src/drivers/intel/fsp2_0/Kconfig: https://review.coreboot.org/c/coreboot/+/84356/comment/3245629b_dddab1f7?us… : PS6, Line 347: 3 > It is in the way different compared to the previous implementation as before the default was 0xfffff […] As documented in the commit message, all the platforms making use of this feature are defining a Global reset status value. If a new platform without global reset support is added one day, we can always add a mechanism for it. If you insist that it is mandatory to have one right away even though there are no use-cases, I can design something tomorrow. File src/soc/intel/apollolake/Kconfig: https://review.coreboot.org/c/coreboot/+/84356/comment/3620c71d_f5f9ee18?us… : PS6, Line 400: default 5 I am not a FSP designer but it seems to be following a simple logic. The type is `EFI_STATUS`. `EFI_STATUS` error codes are encoded as `(MAX_BIT | (a))`. For example, `EFI_INVALID_PARAMETER` being `0x80000002` or `0x8000000000000002` on 32-bit or 64-bit respectively. A similar logic is applied to the OEM Status Code: `(MAX_BIT >> 1 | (a))`. This is actually documented in FSP 2.0 specification section **11.2.2 OEM Status Code** (cf. quote below) so technically this is not new. > The range of status code that have the highest bit clear and the next to highest bit set > are reserved for use by OEMs. I am having a hard time understanding why cleaning it up now is getting so much push back. Is there an actual use-case I am breaking that I don't see ? The change I am suggesting (see latest update) is about having a simple Kconfig with the suffix X of `FSP_STATUS_RESET_REQUIRED_X`). `fsp_reset.c` constructs the name of the constant by macro expansion and concatenation (simplified: `FSP_STATUS_RESET_REQUIRED ## FSP_STATUS_GLOBAL_RESET_SUFFIX`) and uses it. File src/soc/intel/apollolake/Kconfig: https://review.coreboot.org/c/coreboot/+/84356/comment/b8644aa1_30364965?us… : PS8, Line 399: INDEX > I would rather call it "REQUEST" or "TYPE" instead of INDEDX. […] I went with `SUFFIX` as it is what it is about having the suffix of `FSP_STATUS_RESET_REQUIRED_`. What do you think ? File src/soc/intel/common/fsp_reset.c: https://review.coreboot.org/c/coreboot/+/84356/comment/7e1f6e19_8e4cd73b?us… : PS8, Line 9: #define FSP_STATUS_GLOBAL_RESET \ : (FSP_STATUS_RESET_REQUIRED_COLD + CONFIG_FSP_STATUS_GLOBAL_RESET_INDEX - 1) > > This is again very intel specific behavior. […] There is no "benefits". I am not a FSP designer but it seems to be following a simple logic. The type is `EFI_STATUS`. `EFI_STATUS` error codes are encoded as `(MAX_BIT | (a))`. For example, `EFI_INVALID_PARAMETER` being `0x80000002` or `0x8000000000000002` on 32-bit or 64-bit respectively. A similar logic is applied to the OEM Status Code: `(MAX_BIT >> 1 | (a))`. But since the FSPs < 2.4 do not support 64-bits mode the full logic was not needed and the final values were provided. -- To view, visit https://review.coreboot.org/c/coreboot/+/84356?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: I914f73ff06bfb801fc319b45b23d7ce4cb7a6d5f Gerrit-Change-Number: 84356 Gerrit-PatchSet: 9 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.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: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.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: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Rishika Raj <rishikaraj(a)google.com> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Thu, 19 Sep 2024 02:05:33 +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> Comment-In-Reply-To: Werner Zeh <werner.zeh(a)siemens.com>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2024