coreboot-gerrit January 2025

coreboot-gerrit@coreboot.org

1 participants
3269 discussions

[M] Change in coreboot[main]: mb/google/var/trulo: Remove touchscreen definitions
by Kapil Porwal (Code Review) 08 Jan '25

08 Jan '25

Kapil Porwal has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85893?usp=email ) Change subject: mb/google/var/trulo: Remove touchscreen definitions ...................................................................... mb/google/var/trulo: Remove touchscreen definitions Remove touchscreen definitions for Trulo since it doesn't have any. Additionally, disable unused I2C controller 5. BUG=none TEST=Build trulo firmware image Change-Id: If9d00ebf6165449d96f9337f313ee83262507ab4 Signed-off-by: Kapil Porwal <kapilporwal(a)google.com> --- M src/mainboard/google/brya/variants/trulo/gpio.c M src/mainboard/google/brya/variants/trulo/overridetree.cb 2 files changed, 9 insertions(+), 43 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/93/85893/1 diff --git a/src/mainboard/google/brya/variants/trulo/gpio.c b/src/mainboard/google/brya/variants/trulo/gpio.c index 8f0ad94..c813154 100644 --- a/src/mainboard/google/brya/variants/trulo/gpio.c +++ b/src/mainboard/google/brya/variants/trulo/gpio.c @@ -44,8 +44,8 @@ PAD_NC(GPP_A19, NONE), /* A20 : NC */ PAD_NC(GPP_A20, NONE), - /* A21 : DDPC_CTRLCLK ==> TCHSCR_REPORT_EN */ - PAD_CFG_GPO(GPP_A21, 0, DEEP), + /* A21 : NC */ + PAD_NC(GPP_A21, NONE), /* A22 : GPP_A22 ==> TPM_PIRQ# */ PAD_CFG_GPI(GPP_A22, NONE, DEEP), /* A23 : GPP_A23 ==> NC */ @@ -147,10 +147,10 @@ PAD_CFG_NF(GPP_D13, NONE, DEEP, NF1), /* D14 : UART0_ISH_TX_DBG_RX */ PAD_CFG_NF(GPP_D14, NONE, DEEP, NF1), - /* D15 : GPP_D15 ==> SOC_TS_I2C_RST# */ - PAD_CFG_GPO_LOCK(GPP_D15, 1, LOCK_CONFIG), - /* D16 : ISH_UART0_CTS# ==> SOC_TS_I2C_INT# */ - PAD_CFG_GPI_LOCK(GPP_D16, NONE, LOCK_CONFIG), + /* D15 : NC */ + PAD_NC(GPP_D15, NONE), + /* D16 : NC */ + PAD_NC(GPP_D16, NONE), /* D17 : NC */ PAD_NC_LOCK(GPP_D17, NONE, LOCK_CONFIG), /* D18 : NC */ @@ -192,8 +192,8 @@ PAD_NC(GPP_E15, NONE), /* E16 : NC */ PAD_NC(GPP_E16, NONE), - /* E17 : GPP_E17 ==> SOC_TS_PWR_EN */ - PAD_CFG_GPO_LOCK(GPP_E17, 1, LOCK_CONFIG), + /* E17 : NC */ + PAD_NC(GPP_E17, NONE), /* E18 : NC */ PAD_NC(GPP_E18, NONE), /* E19 : DDP1_CTRLDATA ==> GPP_E19_STRAP */ @@ -432,11 +432,6 @@ /* Fill romstage gpio configuration */ static const struct pad_config romstage_gpio_table[] = { - /* Enable touchscreen, hold in reset */ - /* E17 : GPP_E17 ==> SOC_TS_PWR_EN */ - PAD_CFG_GPO(GPP_E17, 1, DEEP), - /* D15 : GPP_D15 ==> SOC_TS_I2C_RST# */ - PAD_CFG_GPO(GPP_D15, 0, DEEP), }; const struct pad_config *variant_gpio_table(size_t *num) diff --git a/src/mainboard/google/brya/variants/trulo/overridetree.cb b/src/mainboard/google/brya/variants/trulo/overridetree.cb index d135db0..5374666 100644 --- a/src/mainboard/google/brya/variants/trulo/overridetree.cb +++ b/src/mainboard/google/brya/variants/trulo/overridetree.cb @@ -122,7 +122,7 @@ [PchSerialIoIndexI2C2] = PchSerialIoDisabled, [PchSerialIoIndexI2C3] = PchSerialIoDisabled, [PchSerialIoIndexI2C4] = PchSerialIoDisabled, - [PchSerialIoIndexI2C5] = PchSerialIoPci, + [PchSerialIoIndexI2C5] = PchSerialIoDisabled, }" register "serial_io_gspi_mode" = "{ @@ -148,7 +148,6 @@ #| | required to set up a BAR | #| | for TPM communication | #| I2C1 | Trackpad | - #| I2C5 | Touchscreen | #+-------------------+---------------------------+ register "common_soc_config" = "{ .i2c[0] = { @@ -170,15 +169,6 @@ .sda_hold = 7, } }, - .i2c[5] = { - .speed = I2C_SPEED_FAST, - .speed_config[0] = { - .speed = I2C_SPEED_FAST, - .scl_lcnt = 158, - .scl_hcnt = 79, - .sda_hold = 7, - } - }, }" device domain 0 on @@ -470,25 +460,6 @@ device i2c 15 on end end end #I2C1 - device ref i2c5 on - chip drivers/i2c/hid - register "generic.hid" = ""ELAN9004"" - register "generic.desc" = ""ELAN Touchscreen"" - register "generic.irq" = "ACPI_IRQ_LEVEL_LOW(GPP_D16_IRQ)" - register "generic.detect" = "1" - register "generic.reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D15)" - register "generic.reset_delay_ms" = "20" - register "generic.reset_off_delay_ms" = "2" - register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_E17)" - register "generic.enable_delay_ms" = "1" - register "generic.stop_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A21)" - register "generic.stop_delay_ms" = "150" - register "generic.stop_off_delay_ms" = "2" - register "generic.has_power_resource" = "1" - register "hid_desc_reg_offset" = "0x01" - device i2c 10 on end - end - end #I2C5 device ref heci1 on end device ref emmc on probe STORAGE STORAGE_UNKNOWN -- To view, visit https://review.coreboot.org/c/coreboot/+/85893?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: If9d00ebf6165449d96f9337f313ee83262507ab4 Gerrit-Change-Number: 85893 Gerrit-PatchSet: 1 Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com>

1 0

[M] Change in coreboot[main]: soc/intel/xeon_sp: Use \_SB.POSC on all platforms
by Shuo Liu (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu. Shuo Liu has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85559?usp=email ) Change subject: soc/intel/xeon_sp: Use \_SB.POSC on all platforms ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85559?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: I2f25ffbde9b83d286c568202fcffb75ffb07286c Gerrit-Change-Number: 85559 Gerrit-PatchSet: 8 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> 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 <tanleansheng(a)outlook.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> 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: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Wed, 08 Jan 2025 11:44:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: soc/intel/xeon_sp: Guard function prototypes
by Shuo Liu (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu. Shuo Liu has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85595?usp=email ) Change subject: soc/intel/xeon_sp: Guard function prototypes ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85595?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: Id6c361155c914f168577833279b4b7cc317b2eec Gerrit-Change-Number: 85595 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> 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 <tanleansheng(a)outlook.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> 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: Lean Sheng Tan <tanleansheng(a)outlook.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Wed, 08 Jan 2025 11:44:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: mb/google/nissa/var/rull: Match VBT with SSFC
by Rui Zhou (Code Review) 08 Jan '25

08 Jan '25

Rui Zhou has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/85875?usp=email ) Change subject: mb/google/nissa/var/rull: Match VBT with SSFC ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/85875?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: abandon Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I413179af0a1346b7d21f17d728d6846c30707978 Gerrit-Change-Number: 85875 Gerrit-PatchSet: 2 Gerrit-Owner: Rui Zhou <zhourui(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Kun Liu <liukun11(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[M] Change in coreboot[main]: mb/google/nissa/var/rull: Match VBT with SSFC
by Rui Zhou (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Dinesh Gehlot, Eric Lai, Jayvik Desai, Kapil Porwal, Kun Liu, Nick Vaccaro, Subrata Banik. Rui Zhou has posted comments on this change by Rui Zhou. ( https://review.coreboot.org/c/coreboot/+/85875?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: mb/google/nissa/var/rull: Match VBT with SSFC ...................................................................... Patch Set 2: (1 comment) File src/mainboard/google/brya/variants/rull/variant.c: https://review.coreboot.org/c/coreboot/+/85875/comment/507755a0_165dc230?us… : PS2, Line 47: : static int get_ssfc(uint32_t *val) : { : static uint32_t known_value; : static enum { : SSFC_NOT_READ, : SSFC_AVAILABLE, : } ssfc_state = SSFC_NOT_READ; : : if (ssfc_state == SSFC_AVAILABLE) { : *val = known_value; : return 0; : } : : /* : * If SSFC field is not in the CBI then the value of SSFC will be 0 for : * further processing later since 0 of each bits group means default : * component in a variant. For more detail, please refer to cbi_ssfc.h. : */ : if (google_chromeec_cbi_get_ssfc(&known_value) != 0) { : printk(BIOS_DEBUG, "SSFC not set in CBI\n"); : return -1; : } : : ssfc_state = SSFC_AVAILABLE; : *val = known_value; : printk(BIOS_INFO, "SSFC 0x%x.\n", known_value); : return 0; : } > But here we are just calling it once. Thank you for your suggestions. When we talked with customers, they felt that the after-sales LCDs replacement service wasn't friendly. We will temporarily abandon the ssfc compatible vbt solution. If necessary, I will revise it according to your suggestions. thank you, I will set it to "abandon" later. -- To view, visit https://review.coreboot.org/c/coreboot/+/85875?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: I413179af0a1346b7d21f17d728d6846c30707978 Gerrit-Change-Number: 85875 Gerrit-PatchSet: 2 Gerrit-Owner: Rui Zhou <zhourui(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Kun Liu <liukun11(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Kun Liu <liukun11(a)huaqin.corp-partner.google.com> Gerrit-Comment-Date: Wed, 08 Jan 2025 11:41:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Rui Zhou <zhourui(a)huaqin.corp-partner.google.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Kapil Porwal <kapilporwal(a)google.com>

1 0

[M] Change in coreboot[main]: mb/google/nissa/var/rull: Match VBT with SSFC
by Subrata Banik (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Dinesh Gehlot, Eric Lai, Jayvik Desai, Kun Liu, Nick Vaccaro, Rui Zhou. Subrata Banik has posted comments on this change by Rui Zhou. ( https://review.coreboot.org/c/coreboot/+/85875?usp=email ) Change subject: mb/google/nissa/var/rull: Match VBT with SSFC ...................................................................... Patch Set 2: (1 comment) File src/mainboard/google/brya/variants/rull/variant.c: https://review.coreboot.org/c/coreboot/+/85875/comment/c6fc5470_25032065?us… : PS2, Line 47: : static int get_ssfc(uint32_t *val) : { : static uint32_t known_value; : static enum { : SSFC_NOT_READ, : SSFC_AVAILABLE, : } ssfc_state = SSFC_NOT_READ; : : if (ssfc_state == SSFC_AVAILABLE) { : *val = known_value; : return 0; : } : : /* : * If SSFC field is not in the CBI then the value of SSFC will be 0 for : * further processing later since 0 of each bits group means default : * component in a variant. For more detail, please refer to cbi_ssfc.h. : */ : if (google_chromeec_cbi_get_ssfc(&known_value) != 0) { : printk(BIOS_DEBUG, "SSFC not set in CBI\n"); : return -1; : } : : ssfc_state = SSFC_AVAILABLE; : *val = known_value; : printk(BIOS_INFO, "SSFC 0x%x.\n", known_value); : return 0; : } > But here we are just calling it once. that would be many other reason to read SSFC in AP FW scope (i mean for boot flow) marking it resolve to allow ODM team to land this CL -- To view, visit https://review.coreboot.org/c/coreboot/+/85875?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: I413179af0a1346b7d21f17d728d6846c30707978 Gerrit-Change-Number: 85875 Gerrit-PatchSet: 2 Gerrit-Owner: Rui Zhou <zhourui(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Kun Liu <liukun11(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Rui Zhou <zhourui(a)huaqin.corp-partner.google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Kun Liu <liukun11(a)huaqin.corp-partner.google.com> Gerrit-Comment-Date: Wed, 08 Jan 2025 11:40:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Rui Zhou <zhourui(a)huaqin.corp-partner.google.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Kapil Porwal <kapilporwal(a)google.com>

1 0

[M] Change in coreboot[main]: mb/google/nissa/var/rull: Match VBT with SSFC
by Kapil Porwal (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Dinesh Gehlot, Eric Lai, Jayvik Desai, Kun Liu, Nick Vaccaro, Rui Zhou. Kapil Porwal has posted comments on this change by Rui Zhou. ( https://review.coreboot.org/c/coreboot/+/85875?usp=email ) Change subject: mb/google/nissa/var/rull: Match VBT with SSFC ...................................................................... Patch Set 2: (1 comment) File src/mainboard/google/brya/variants/rull/variant.c: https://review.coreboot.org/c/coreboot/+/85875/comment/cded07b7_3e5bed14?us… : PS2, Line 47: : static int get_ssfc(uint32_t *val) : { : static uint32_t known_value; : static enum { : SSFC_NOT_READ, : SSFC_AVAILABLE, : } ssfc_state = SSFC_NOT_READ; : : if (ssfc_state == SSFC_AVAILABLE) { : *val = known_value; : return 0; : } : : /* : * If SSFC field is not in the CBI then the value of SSFC will be 0 for : * further processing later since 0 of each bits group means default : * component in a variant. For more detail, please refer to cbi_ssfc.h. : */ : if (google_chromeec_cbi_get_ssfc(&known_value) != 0) { : printk(BIOS_DEBUG, "SSFC not set in CBI\n"); : return -1; : } : : ssfc_state = SSFC_AVAILABLE; : *val = known_value; : printk(BIOS_INFO, "SSFC 0x%x.\n", known_value); : return 0; : } > > Looking at the use case of the new function, I am wondering why we are not using `google_chromeec_ […] But here we are just calling it once. -- To view, visit https://review.coreboot.org/c/coreboot/+/85875?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: I413179af0a1346b7d21f17d728d6846c30707978 Gerrit-Change-Number: 85875 Gerrit-PatchSet: 2 Gerrit-Owner: Rui Zhou <zhourui(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Jayvik Desai <jayvik(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Kun Liu <liukun11(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jayvik Desai <jayvik(a)google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Rui Zhou <zhourui(a)huaqin.corp-partner.google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Kun Liu <liukun11(a)huaqin.corp-partner.google.com> Gerrit-Comment-Date: Wed, 08 Jan 2025 11:36:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Rui Zhou <zhourui(a)huaqin.corp-partner.google.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Kapil Porwal <kapilporwal(a)google.com>

1 0

[XS] Change in coreboot[main]: mb/google/rauru: Disable modem power
by Jarried Lin (Code Review) 08 Jan '25

08 Jan '25

1 0

[M] Change in coreboot[main]: soc/mediatek/mt8196: Add modem power driver to disable unused power
by Jarried Lin (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Hung-Te Lin, Yidi Lin, Yu-Ping Wu. Jarried Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85651?usp=email ) Change subject: soc/mediatek/mt8196: Add modem power driver to disable unused power ...................................................................... Patch Set 5: (9 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/85651/comment/dfb27896_476f2a8d?us… : PS3, Line 7: disable modem power driver > Any update? Done File src/soc/mediatek/mt8196/include/soc/md_power_ctrl.h: https://review.coreboot.org/c/coreboot/+/85651/comment/c29c93cb_c9ebf8b2?us… : PS3, Line 6: /* : * md_power_down() - Power down modem : */ > `/* Power down modem. […] Done https://review.coreboot.org/c/coreboot/+/85651/comment/8043f4df_e8f08882?us… : PS3, Line 9: md > Also rename the file to modem_power_ctrl.*. Done File src/soc/mediatek/mt8196/md_power_ctrl.c: https://review.coreboot.org/c/coreboot/+/85651/comment/facdcba4_0e259a48?us… : PS3, Line 28: mtk_apifrbus_ao_io_reg_bus > As this is used only in this file, can we have a shorter name? Done https://review.coreboot.org/c/coreboot/+/85651/comment/01d08985_c307ff7b?us… : PS3, Line 31: struct mtk_spm_regs { : u32 reserved0[896]; : u32 md1_pwr_con; : u32 reserved1[20]; : u32 adsp_top_pwr_con; : u32 adsp_infra_pwr_con; : u32 adsp_ao_pwr_con; : u32 reserved2[11]; : u32 spu_hwrot_pwr_con; : u32 reserved3[27]; : u32 md_buck_iso_con; : u32 reserved4[5]; : u32 pwr_status; : u32 pwr_status_2nd; : }; : : static struct mtk_spm_regs *const mtk_spm = (void *)SPM_BASE; > This is already defined in spm.h. Please rebase onto CB:85599. Done https://review.coreboot.org/c/coreboot/+/85651/comment/b12f1abe_d214c83f?us… : PS3, Line 50: OTHER_SRAM_CKISO = BIT(0), : PWR_RST_B = BIT(0), > Are these both for the `md1_pwr_con` register? Done https://review.coreboot.org/c/coreboot/+/85651/comment/7a665c32_7f6b5014?us… : PS3, Line 52: OTHER_SRAM_ISOINT_B = BIT(1), : PWR_ISO = BIT(1), > Same here. […] Done https://review.coreboot.org/c/coreboot/+/85651/comment/9b643eb7_5947c4cc?us… : PS3, Line 90: struct pmif *pmif_arb = get_pmif_controller(PMIF_SPMI, SPMI_MASTER_1); : : /* Vsram_vosel = 0x24d, read Vsram value */ : reg = VSRAM_VOSEL; : pmif_arb->read(pmif_arb, SPMI_SLAVE_4, reg, &val); : : /* Vmodem_vosel = 0x24e, write Vmodem 0.75V */ : reg = VMODEM_VOSEL; : pmif_arb->write(pmif_arb, SPMI_SLAVE_4, reg, SPMI_SLAVE_4_750MV); : pmif_arb->read(pmif_arb, SPMI_SLAVE_4, reg, &val); > One minor side effect is that `pmif_check_init_done` will be called multiple times. […] Done https://review.coreboot.org/c/coreboot/+/85651/comment/6a03dc15_6d1a7497?us… : PS3, Line 124: : while ((read32(&mtk_apifrbus_ao_io_reg_bus->sleep_protect_rdy_sta_1) & : MD1_PROT_STEP0_0_ACK_MASK) != MD1_PROT_STEP0_0_ACK_MASK) { : printk(BIOS_INFO, "First check: Does not get MD1_PROT_STEP0_0_ACK_MASK\n"); : retryCount++; : if (retryCount > MAX_RETRY_COUNT) { : printk(BIOS_INFO, "First check: check failed\n"); : break; : } : } > use `retry` API or implement a `wait_for_state_ready` function Done -- To view, visit https://review.coreboot.org/c/coreboot/+/85651?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: Ia8808e3500727753e4537017b46ac8ab39d59468 Gerrit-Change-Number: 85651 Gerrit-PatchSet: 5 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.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: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Wed, 08 Jan 2025 11:26:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Comment-In-Reply-To: Yidi Lin <yidilin(a)google.com>

1 0

[XS] Change in coreboot[main]: mb/google/rauru: Disable modem power
by Jarried Lin (Code Review) 08 Jan '25

08 Jan '25

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin. Hello Hung-Te Lin, Yidi Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/85663?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: mb/google/rauru: Disable modem power ...................................................................... mb/google/rauru: Disable modem power BUG=b:315894234 TEST=Build pass. The actual measurement of the average power over 20 seconds decreased from 6.755W to 6.716W. Change-Id: I71bda7055afc902525501ddf7074f9b2c5550d4a Signed-off-by: Xavier Chang <xavier.chang(a)mediatek.corp-partner.google.com> --- M src/mainboard/google/rauru/romstage.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/85663/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85663?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: I71bda7055afc902525501ddf7074f9b2c5550d4a Gerrit-Change-Number: 85663 Gerrit-PatchSet: 2 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.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: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2025