coreboot-gerrit January 2025

coreboot-gerrit@coreboot.org

1 participants
3269 discussions

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add DDP driver
by Nancy Lin (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Hung-Te Lin, Jarried Lin, Paul Menzel, Yidi Lin, Yu-Ping Wu. Nancy Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85950?usp=email ) Change subject: soc/mediatek/mt8196: Add DDP driver ...................................................................... Patch Set 3: (1 comment) File src/soc/mediatek/mt8196/ddp.c: https://review.coreboot.org/c/coreboot/+/85950/comment/682832b0_0d219235?us… : PS3, Line 312: printk(BIOS_WARNING, "%s: w/h: %d/%d exceed hw limit\n", __func__, width, height); > Also print the hardware limit? ok, I will add it -- To view, visit https://review.coreboot.org/c/coreboot/+/85950?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: I006911e83d940c1eec7135a6a0c36fbfa2aad466 Gerrit-Change-Number: 85950 Gerrit-PatchSet: 3 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nancy Lin <nancy.lin(a)mediatek.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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Wed, 15 Jan 2025 12:41:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[S] Change in coreboot[main]: mb/lenovo/x131e: Remove old USB configurations
by Patrick Rudolph (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Alexander Couzens, Keith Hui. Patrick Rudolph has posted comments on this change by Keith Hui. ( https://review.coreboot.org/c/coreboot/+/85944?usp=email ) Change subject: mb/lenovo/x131e: Remove old USB configurations ...................................................................... Patch Set 2: (1 comment) File src/mainboard/lenovo/x131e/devicetree.cb: https://review.coreboot.org/c/coreboot/+/85944/comment/974921f0_471f9003?us… : PS2, Line 45: 0x00000c03 > And it matches usb_port_config, unconnected P2 also being assigned to OC0#. […] Please update according to schematics. I doubt that most mainboard have sane values for xhci_switchable_ports and superspeed_capable_ports. -- To view, visit https://review.coreboot.org/c/coreboot/+/85944?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: I6bdc9a188b2baa2207aaccb46821b58f97ff7da6 Gerrit-Change-Number: 85944 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Comment-Date: Wed, 15 Jan 2025 12:07:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Keith Hui <buurin(a)gmail.com>

1 0

[M] Change in coreboot[main]: drivers/option: Add CBFS file based option backend
by Subrata Banik (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Julius Werner, Nicholas Chin. Subrata Banik has posted comments on this change by Nicholas Chin. ( https://review.coreboot.org/c/coreboot/+/85905?usp=email ) Change subject: drivers/option: Add CBFS file based option backend ...................................................................... Patch Set 6: (1 comment) File src/drivers/option/Makefile.mk: https://review.coreboot.org/c/coreboot/+/85905/comment/922a920c_e5e9b8a0?us… : PS6, Line 6: smm-$(CONFIG_USE_CBFS_FILE_OPTION_BACKEND) += cbfs_file_option.c as we don't compile vboot library in smm mode, I'm getting compilation error due to undefined reference to `vb2_digest_size` and others ``` HOSTCC cbfstool/bpdt_1_7.o HOSTCC cbfstool/subpart_hdr_1.o /build/rex/tmp/portage/sys-boot/coreboot-9999/work/coreboot-sdk/bin/i386-elf-ld.bfd: /build/rex/tmp/portage/sys-boot/coreboot-9999/work/build/rex0/smm/commonlib/bsd/cbfs_private.o: in function `cbfs_file_hash': /build/rex/tmp/portage/sys-boot/coreboot-9999/work/coreboot-9999/src/commonlib/bsd/cbfs_private.c:207: undefined reference to `vb2_digest_size' HOSTCC cbfstool/subpart_hdr_2.o HOSTCC cbfstool/subpart_entry_1.o /build/rex/tmp/portage/sys-boot/coreboot-9999/work/coreboot-sdk/bin/i386-elf-ld.bfd: /build/rex/tmp/portage/sys-boot/coreboot-9999/work/build/rex0/smm/lib/cbfs.o: in function `vboot_hwcrypto_allowed': /build/rex/tmp/portage/sys-boot/coreboot-9999/work/coreboot-9999/src/security/vboot/misc.h:98: undefined reference to `vboot_get_context' /build/rex/tmp/portage/sys-boot/coreboot-9999/work/coreboot-sdk/bin/i386-elf-ld.bfd: /build/rex/tmp/portage/sys-boot/coreboot-9999/work/coreboot-9999/src/security/vboot/misc.h:98: undefined reference to `vb2api_hwcrypto_allowed' /build/rex/tmp/portage/sys-boot/coreboot-9999/work/coreboot-sdk/bin/i386-elf-ld.bfd: /build/rex/tmp/portage/sys-boot/coreboot-9999/work/build/rex0/smm/lib/cbfs.o: in function `cbfs_file_hash_mismatch': /build/rex/tmp/portage/sys-boot/coreboot-9999/work/coreboot-9999/src/lib/cbfs.c:177: undefined reference to `vb2_hash_verify' /build/rex/tmp/portage/sys-boot/coreboot-9999/work/coreboot-sdk/bin/i386-elf-ld.bfd: /build/rex/tmp/portage/sys-boot/coreboot-9999/work/coreboot-9999/src/lib/cbfs.c:182: undefined reference to `vboot_get_context' /build/rex/tmp/portage/sys-boot/coreboot-9999/work/coreboot-sdk/bin/i386-elf-ld.bfd: /build/rex/tmp/portage/sys-boot/coreboot-9999/work/coreboot-9999/src/lib/cbfs.c:182: undefined reference to `vboot_fail_and_reboot' OBJCOPY bootblock.raw.bin HOSTCC cbfstool/ifittool (link) HOSTCC cbfstool/cse_fpt (link) ``` I believe we don't want to link `FWLIB = ${BUILD}/vboot_fw.a` in smm? @jwerner@chromium.org thoughts ? -- To view, visit https://review.coreboot.org/c/coreboot/+/85905?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: Ifc0439ee42f13f49ae54d4855d1d9333c39b01f5 Gerrit-Change-Number: 85905 Gerrit-PatchSet: 6 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Wed, 15 Jan 2025 12:06:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in coreboot[main]: mb/lenovo/x131e: Remove old USB configurations
by Keith Hui (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Alexander Couzens, Patrick Rudolph. Keith Hui has posted comments on this change by Keith Hui. ( https://review.coreboot.org/c/coreboot/+/85944?usp=email ) Change subject: mb/lenovo/x131e: Remove old USB configurations ...................................................................... Patch Set 2: (1 comment) File src/mainboard/lenovo/x131e/devicetree.cb: https://review.coreboot.org/c/coreboot/+/85944/comment/3f504e4b_8d5c1259?us… : PS2, Line 45: 0x00000c03 > according to schematics P0 and P1 are the only USB3.0 ports and both connected to OC0#. […] And it matches usb_port_config, unconnected P2 also being assigned to OC0#. So to reconcile, there are two ways: 1. Leave the patch as is and let XOCM be programmed as 0x00000007. 2. Change P2 to have no OC and let XOCM be programmed as 0x00000003. OC1# appears completely unused according to this usb_port_config. If that is also what your schematics say, this will be fine. What would you like done? -- To view, visit https://review.coreboot.org/c/coreboot/+/85944?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: I6bdc9a188b2baa2207aaccb46821b58f97ff7da6 Gerrit-Change-Number: 85944 Gerrit-PatchSet: 2 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 15 Jan 2025 12:03:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com>

1 0

[S] Change in coreboot[main]: mb/google/rauru: Configure firmware display for eDP panel
by Paul Menzel (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Hung-Te Lin, Jarried Lin, Nancy Lin, Yu-Ping Wu. Paul Menzel has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85952?usp=email ) Change subject: mb/google/rauru: Configure firmware display for eDP panel ...................................................................... Patch Set 4: (2 comments) File src/mainboard/google/rauru/mainboard.c: https://review.coreboot.org/c/coreboot/+/85952/comment/692583b9_d4b9f9c7?us… : PS4, Line 104: mt6373_enable_vcn33_3(true); Would `false` disable? I’d prefer an enable and disable function without an argument. https://review.coreboot.org/c/coreboot/+/85952/comment/c7bd9cc9_4c0313f9?us… : PS4, Line 104: mt6373_enable_vcn33_3(true); : mt6373_set_vcn33_3_voltage(3300000); This API looks strange. Why does the set function not run enable, when needed? -- To view, visit https://review.coreboot.org/c/coreboot/+/85952?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: Ic928b2478c41ccd03223fd2b73d9e81d303a2036 Gerrit-Change-Number: 85952 Gerrit-PatchSet: 4 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nancy Lin <nancy.lin(a)mediatek.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: Nancy Lin <nancy.lin(a)mediatek.corp-partner.google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Wed, 15 Jan 2025 11:23:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in coreboot[main]: soc/amd/glinda/cpu: Update cache info
by Paul Menzel (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Jérémy Compostella, Naresh Solanki. Paul Menzel has posted comments on this change by Naresh Solanki. ( https://review.coreboot.org/c/coreboot/+/85640?usp=email ) Change subject: soc/amd/glinda/cpu: Update cache info ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/85640/comment/531e8144_7979486a?us… : PS3, Line 8: > Previously Cache size of only BSP was reported. […] Please add that to the commit message. -- To view, visit https://review.coreboot.org/c/coreboot/+/85640?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: I46947e8ac62c903036a81642e03201e353c3dac6 Gerrit-Change-Number: 85640 Gerrit-PatchSet: 3 Gerrit-Owner: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Naresh Solanki <naresh.solanki(a)9elements.com> Gerrit-Comment-Date: Wed, 15 Jan 2025 11:21:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Naresh Solanki <naresh.solanki(a)9elements.com>

1 0

[L] Change in coreboot[main]: soc/mediatek/mt8196: Add DDP driver
by Paul Menzel (Code Review) 15 Jan '25

15 Jan '25

Attention is currently required from: Hung-Te Lin, Jarried Lin, Nancy Lin, Yidi Lin, Yu-Ping Wu. Paul Menzel has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85950?usp=email ) Change subject: soc/mediatek/mt8196: Add DDP driver ...................................................................... Patch Set 3: (1 comment) File src/soc/mediatek/mt8196/ddp.c: https://review.coreboot.org/c/coreboot/+/85950/comment/3c96b482_1fb51cd5?us… : PS3, Line 312: printk(BIOS_WARNING, "%s: w/h: %d/%d exceed hw limit\n", __func__, width, height); Also print the hardware limit? -- To view, visit https://review.coreboot.org/c/coreboot/+/85950?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: I006911e83d940c1eec7135a6a0c36fbfa2aad466 Gerrit-Change-Number: 85950 Gerrit-PatchSet: 3 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nancy Lin <nancy.lin(a)mediatek.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: Nancy Lin <nancy.lin(a)mediatek.corp-partner.google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Wed, 15 Jan 2025 11:20:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in coreboot[main]: soc/mediatek/mt8196: Initialize PMIF for SD Card
by Yidi Lin (Code Review) 15 Jan '25

15 Jan '25

Yidi Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85969?usp=email ) ( 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: soc/mediatek/mt8196: Initialize PMIF for SD Card ...................................................................... soc/mediatek/mt8196: Initialize PMIF for SD Card mt6373_init_pmif_arb() needs to be initialized for SD card to control the regulator. TEST=emrege-rauru coreboot TEST=The assertion is gone on Rauru during normal boot. Change-Id: I7e3265bb62a6c78d44e2c756be9a020a49a03056 Signed-off-by: Yidi Lin <yidilin(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85969 Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/mediatek/mt8196/msdc.c 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Yu-Ping Wu: Looks good to me, approved diff --git a/src/soc/mediatek/mt8196/msdc.c b/src/soc/mediatek/mt8196/msdc.c index f787374..c24bc42 100644 --- a/src/soc/mediatek/mt8196/msdc.c +++ b/src/soc/mediatek/mt8196/msdc.c @@ -7,6 +7,7 @@ #include <gpio.h> #include <soc/msdc.h> +#include <soc/mt6373.h> #include <soc/regulator.h> static const struct pad_func sdcard_pins[] = { @@ -29,6 +30,7 @@ } /* enable SD card power */ + mt6373_init_pmif_arb(); mainboard_enable_regulator(MTK_REGULATOR_VMCH, true); mainboard_enable_regulator(MTK_REGULATOR_VMC, true); mainboard_set_regulator_voltage(MTK_REGULATOR_VMCH, 3000000); -- To view, visit https://review.coreboot.org/c/coreboot/+/85969?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: I7e3265bb62a6c78d44e2c756be9a020a49a03056 Gerrit-Change-Number: 85969 Gerrit-PatchSet: 5 Gerrit-Owner: Yidi Lin <yidilin(a)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>

1 0

[S] Change in coreboot[main]: soc/mediatek: Rename is_pmif_init_done to check_init_done
by Yidi Lin (Code Review) 15 Jan '25

15 Jan '25

Yidi Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85968?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: soc/mediatek: Rename is_pmif_init_done to check_init_done ...................................................................... soc/mediatek: Rename is_pmif_init_done to check_init_done TEST=emerge-geralt coreboot && emerge-rauru coreboot Change-Id: Ib4b9a7969f5af6e001c5b491ec09a43e1289a6ae Signed-off-by: Yidi Lin <yidilin(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85968 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> --- M src/soc/mediatek/common/include/soc/pmif_common.h M src/soc/mediatek/common/mt6315.c M src/soc/mediatek/common/mt6316.c M src/soc/mediatek/common/mt6359p.c M src/soc/mediatek/common/mt6363.c M src/soc/mediatek/common/mt6373.c M src/soc/mediatek/common/mt6685.c M src/soc/mediatek/common/pmif_init.c M src/soc/mediatek/common/pmif_spmi.c M src/soc/mediatek/mt8196/pmif_init.c M src/soc/mediatek/mt8196/pmif_spmi.c 11 files changed, 17 insertions(+), 17 deletions(-) Approvals: build bot (Jenkins): Verified Yu-Ping Wu: Looks good to me, approved diff --git a/src/soc/mediatek/common/include/soc/pmif_common.h b/src/soc/mediatek/common/include/soc/pmif_common.h index 949b34f..b9cee91 100644 --- a/src/soc/mediatek/common/include/soc/pmif_common.h +++ b/src/soc/mediatek/common/include/soc/pmif_common.h @@ -34,7 +34,7 @@ void (*write16)(struct pmif *arb, u32 slvid, u32 reg, u16 data); u32 (*read_field)(struct pmif *arb, u32 slvid, u32 reg, u32 mask, u32 shift); void (*write_field)(struct pmif *arb, u32 slvid, u32 reg, u32 val, u32 mask, u32 shift); - int (*is_pmif_init_done)(struct pmif *arb); + int (*check_init_done)(struct pmif *arb); }; enum { diff --git a/src/soc/mediatek/common/mt6315.c b/src/soc/mediatek/common/mt6315.c index 7862c59..35567d1 100644 --- a/src/soc/mediatek/common/mt6315.c +++ b/src/soc/mediatek/common/mt6315.c @@ -91,7 +91,7 @@ pmif_arb = get_pmif_controller(PMIF_SPMI, 0); assert(pmif_arb); - if (pmif_arb->is_pmif_init_done(pmif_arb)) + if (pmif_arb->check_init_done(pmif_arb)) die("ERROR - Failed to initialize pmif spmi"); } diff --git a/src/soc/mediatek/common/mt6316.c b/src/soc/mediatek/common/mt6316.c index b7242fd..89c8c83 100644 --- a/src/soc/mediatek/common/mt6316.c +++ b/src/soc/mediatek/common/mt6316.c @@ -177,7 +177,7 @@ pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_0); assert(pmif_arb); - if (pmif_arb->is_pmif_init_done(pmif_arb)) + if (pmif_arb->check_init_done(pmif_arb)) die("ERROR - Failed to initialize pmif spmi"); for (int i = 0; i < ARRAY_SIZE(mt6316_slave_id); i++) diff --git a/src/soc/mediatek/common/mt6359p.c b/src/soc/mediatek/common/mt6359p.c index fc7c677..530f4d1 100644 --- a/src/soc/mediatek/common/mt6359p.c +++ b/src/soc/mediatek/common/mt6359p.c @@ -337,7 +337,7 @@ pmif_arb = get_pmif_controller(PMIF_SPI, 0); assert(pmif_arb); - if (pmif_arb->is_pmif_init_done(pmif_arb)) + if (pmif_arb->check_init_done(pmif_arb)) die("ERROR - Failed to initialize pmif spi"); } diff --git a/src/soc/mediatek/common/mt6363.c b/src/soc/mediatek/common/mt6363.c index ba96163..220a7ce 100644 --- a/src/soc/mediatek/common/mt6363.c +++ b/src/soc/mediatek/common/mt6363.c @@ -267,7 +267,7 @@ pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_1); assert(pmif_arb); - if (pmif_arb->is_pmif_init_done(pmif_arb)) + if (pmif_arb->check_init_done(pmif_arb)) die("ERROR - Failed to initialize pmif spi"); printk(BIOS_INFO, "[%s][MT6363]CHIP ID = 0x%x\n", __func__, diff --git a/src/soc/mediatek/common/mt6373.c b/src/soc/mediatek/common/mt6373.c index 8dca0a0..315f19b 100644 --- a/src/soc/mediatek/common/mt6373.c +++ b/src/soc/mediatek/common/mt6373.c @@ -247,7 +247,7 @@ pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_1); assert(pmif_arb); - if (pmif_arb->is_pmif_init_done(pmif_arb)) + if (pmif_arb->check_init_done(pmif_arb)) die("%s: initialization failed", __func__); printk(BIOS_INFO, "[%s][MT6373]CHIP ID = 0x%x\n", diff --git a/src/soc/mediatek/common/mt6685.c b/src/soc/mediatek/common/mt6685.c index 81ee6c6..7d17556 100644 --- a/src/soc/mediatek/common/mt6685.c +++ b/src/soc/mediatek/common/mt6685.c @@ -86,7 +86,7 @@ pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_1); assert(pmif_arb); - if (pmif_arb->is_pmif_init_done(pmif_arb)) + if (pmif_arb->check_init_done(pmif_arb)) die("ERROR - Failed to initialize pmif spi"); printk(BIOS_INFO, "[%s]CHIP ID = %#x\n", __func__, mt6685_read_field(0xb, 0xFF, 0)); diff --git a/src/soc/mediatek/common/pmif_init.c b/src/soc/mediatek/common/pmif_init.c index 73b22cf..1425081 100644 --- a/src/soc/mediatek/common/pmif_init.c +++ b/src/soc/mediatek/common/pmif_init.c @@ -17,7 +17,7 @@ .read = pmif_spmi_read, .write_field = pmif_spmi_write_field, .read_field = pmif_spmi_read_field, - .is_pmif_init_done = pmif_check_init_done, + .check_init_done = pmif_check_init_done, }, }; @@ -32,7 +32,7 @@ .read = pmif_spi_read, .write_field = pmif_spi_write_field, .read_field = pmif_spi_read_field, - .is_pmif_init_done = pmif_check_init_done, + .check_init_done = pmif_check_init_done, }, }; diff --git a/src/soc/mediatek/common/pmif_spmi.c b/src/soc/mediatek/common/pmif_spmi.c index 7263ff2..0de4df0 100644 --- a/src/soc/mediatek/common/pmif_spmi.c +++ b/src/soc/mediatek/common/pmif_spmi.c @@ -152,12 +152,12 @@ int pmif_spmi_init(struct pmif *arb) { - if (arb->is_pmif_init_done(arb) != 0) { + if (arb->check_init_done(arb) != 0) { pmif_spmi_force_normal_mode(arb); pmif_spmi_enable_swinf(arb); pmif_spmi_enable_cmdIssue(arb, true); pmif_spmi_enable(arb); - if (arb->is_pmif_init_done(arb)) + if (arb->check_init_done(arb)) return -E_NODEV; } diff --git a/src/soc/mediatek/mt8196/pmif_init.c b/src/soc/mediatek/mt8196/pmif_init.c index 0baf720..a6151bb 100644 --- a/src/soc/mediatek/mt8196/pmif_init.c +++ b/src/soc/mediatek/mt8196/pmif_init.c @@ -24,7 +24,7 @@ .read16 = pmif_spmi_read16, .write_field = pmif_spmi_write_field, .read_field = pmif_spmi_read_field, - .is_pmif_init_done = pmif_check_init_done, + .check_init_done = pmif_check_init_done, }, { .mtk_pmif = (struct mtk_pmif_regs *)PMIF_SPMI_BASE, .ch = (struct chan_regs *)PMIF_SPMI_AP_CHAN, @@ -36,7 +36,7 @@ .read16 = pmif_spmi_read16, .write_field = pmif_spmi_write_field, .read_field = pmif_spmi_read_field, - .is_pmif_init_done = pmif_check_init_done, + .check_init_done = pmif_check_init_done, }, { .mtk_pmif = (struct mtk_pmif_regs *)PMIF_SPMI_P_BASE, .ch = (struct chan_regs *)PMIF_SPMI_AP_CHAN_P, @@ -48,7 +48,7 @@ .read16 = pmif_spmi_read16, .write_field = pmif_spmi_write_field, .read_field = pmif_spmi_read_field, - .is_pmif_init_done = pmif_check_init_done, + .check_init_done = pmif_check_init_done, }, }; @@ -63,7 +63,7 @@ .read = pmif_spi_read, .write_field = pmif_spi_write_field, .read_field = pmif_spi_read_field, - .is_pmif_init_done = pmif_check_init_done, + .check_init_done = pmif_check_init_done, }, }; diff --git a/src/soc/mediatek/mt8196/pmif_spmi.c b/src/soc/mediatek/mt8196/pmif_spmi.c index e646816..c4a984f 100644 --- a/src/soc/mediatek/mt8196/pmif_spmi.c +++ b/src/soc/mediatek/mt8196/pmif_spmi.c @@ -337,12 +337,12 @@ int pmif_spmi_init(struct pmif *arb) { - if (arb->is_pmif_init_done(arb) != 0) { + if (arb->check_init_done(arb) != 0) { pmif_spmi_force_normal_mode(arb); pmif_spmi_enable_swinf(arb); pmif_spmi_enable_cmd_issue(arb, true); pmif_spmi_enable(arb); - if (arb->is_pmif_init_done(arb)) + if (arb->check_init_done(arb)) return -E_NODEV; } -- To view, visit https://review.coreboot.org/c/coreboot/+/85968?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: Ib4b9a7969f5af6e001c5b491ec09a43e1289a6ae Gerrit-Change-Number: 85968 Gerrit-PatchSet: 4 Gerrit-Owner: Yidi Lin <yidilin(a)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>

1 0

[M] Change in coreboot[main]: soc/mediatek: Skip duplicate pmif_arb->is_pmif_init_done() call
by Yidi Lin (Code Review) 15 Jan '25

15 Jan '25

Yidi Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85967?usp=email ) Change subject: soc/mediatek: Skip duplicate pmif_arb->is_pmif_init_done() call ...................................................................... soc/mediatek: Skip duplicate pmif_arb->is_pmif_init_done() call Return to the caller immediately if pmif_arb has been initiailized. In this way, we can skip unnecessary check and reduce the access to the PMIF register. TEST=emerge-geralt coreboot && emerge-rauru coreboot Change-Id: Id1d11f8b238855edb393d77151159792e7716d22 Signed-off-by: Yidi Lin <yidilin(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85967 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Yu-Ping Wu <yupingso(a)google.com> --- M src/soc/mediatek/common/mt6315.c M src/soc/mediatek/common/mt6316.c M src/soc/mediatek/common/mt6359p.c M src/soc/mediatek/common/mt6363.c M src/soc/mediatek/common/mt6373.c M src/soc/mediatek/common/mt6685.c 6 files changed, 31 insertions(+), 26 deletions(-) Approvals: Yu-Ping Wu: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/soc/mediatek/common/mt6315.c b/src/soc/mediatek/common/mt6315.c index aebbd4c..7862c59 100644 --- a/src/soc/mediatek/common/mt6315.c +++ b/src/soc/mediatek/common/mt6315.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <assert.h> #include <console/console.h> #include <delay.h> #include <soc/mt6315.h> @@ -84,11 +85,11 @@ static void init_pmif_arb(void) { - if (!pmif_arb) { - pmif_arb = get_pmif_controller(PMIF_SPMI, 0); - if (!pmif_arb) - die("ERROR: No spmi device"); - } + if (pmif_arb) + return; + + pmif_arb = get_pmif_controller(PMIF_SPMI, 0); + assert(pmif_arb); if (pmif_arb->is_pmif_init_done(pmif_arb)) die("ERROR - Failed to initialize pmif spmi"); diff --git a/src/soc/mediatek/common/mt6316.c b/src/soc/mediatek/common/mt6316.c index 797ae3e..b7242fd 100644 --- a/src/soc/mediatek/common/mt6316.c +++ b/src/soc/mediatek/common/mt6316.c @@ -171,10 +171,11 @@ static void init_pmif_arb(void) { - if (!pmif_arb) { - pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_0); - assert(pmif_arb); - } + if (pmif_arb) + return; + + pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_0); + assert(pmif_arb); if (pmif_arb->is_pmif_init_done(pmif_arb)) die("ERROR - Failed to initialize pmif spmi"); diff --git a/src/soc/mediatek/common/mt6359p.c b/src/soc/mediatek/common/mt6359p.c index e766479..fc7c677 100644 --- a/src/soc/mediatek/common/mt6359p.c +++ b/src/soc/mediatek/common/mt6359p.c @@ -331,11 +331,11 @@ void mt6359p_init_pmif_arb(void) { - if (!pmif_arb) { - pmif_arb = get_pmif_controller(PMIF_SPI, 0); - if (!pmif_arb) - die("ERROR: No spi device"); - } + if (pmif_arb) + return; + + pmif_arb = get_pmif_controller(PMIF_SPI, 0); + assert(pmif_arb); if (pmif_arb->is_pmif_init_done(pmif_arb)) die("ERROR - Failed to initialize pmif spi"); diff --git a/src/soc/mediatek/common/mt6363.c b/src/soc/mediatek/common/mt6363.c index a618b6f..ba96163 100644 --- a/src/soc/mediatek/common/mt6363.c +++ b/src/soc/mediatek/common/mt6363.c @@ -261,10 +261,11 @@ void mt6363_init_pmif_arb(void) { - if (!pmif_arb) { - pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_1); - assert(pmif_arb); - } + if (pmif_arb) + return; + + pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_1); + assert(pmif_arb); if (pmif_arb->is_pmif_init_done(pmif_arb)) die("ERROR - Failed to initialize pmif spi"); diff --git a/src/soc/mediatek/common/mt6373.c b/src/soc/mediatek/common/mt6373.c index 1d5e1e2..8dca0a0 100644 --- a/src/soc/mediatek/common/mt6373.c +++ b/src/soc/mediatek/common/mt6373.c @@ -241,10 +241,11 @@ void mt6373_init_pmif_arb(void) { - if (!pmif_arb) { - pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_1); - assert(pmif_arb); - } + if (pmif_arb) + return; + + pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_1); + assert(pmif_arb); if (pmif_arb->is_pmif_init_done(pmif_arb)) die("%s: initialization failed", __func__); diff --git a/src/soc/mediatek/common/mt6685.c b/src/soc/mediatek/common/mt6685.c index fa434f4..81ee6c6 100644 --- a/src/soc/mediatek/common/mt6685.c +++ b/src/soc/mediatek/common/mt6685.c @@ -80,10 +80,11 @@ void mt6685_init_pmif_arb(void) { - if (!pmif_arb) { - pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_1); - assert(pmif_arb); - } + if (pmif_arb) + return; + + pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_1); + assert(pmif_arb); if (pmif_arb->is_pmif_init_done(pmif_arb)) die("ERROR - Failed to initialize pmif spi"); -- To view, visit https://review.coreboot.org/c/coreboot/+/85967?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: Id1d11f8b238855edb393d77151159792e7716d22 Gerrit-Change-Number: 85967 Gerrit-PatchSet: 4 Gerrit-Owner: Yidi Lin <yidilin(a)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>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2025