coreboot-gerrit December 2024

coreboot-gerrit@coreboot.org

1 participants
3018 discussions

[M] Change in coreboot[main]: mb/trulo/var/uldrenite: Configure USB ports and mapping
by Subrata Banik (Code Review) 18 Dec '24

18 Dec '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85582?usp=email ) Change subject: mb/trulo/var/uldrenite: Configure USB ports and mapping ...................................................................... mb/trulo/var/uldrenite: Configure USB ports and mapping Based on the schematic_20241203, configure USB ports and their mappings. BUG=b:380789023 TEST=emerge-nissa coreboot Change-Id: I8fa3bf958faaf17b35e33fe93e2a11e0d2eb48d6 Signed-off-by: John Su <john_su(a)compal.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85582 Reviewed-by: Subrata Banik <subratabanik(a)google.com> Reviewed-by: Eric Lai <ericllai(a)google.com> Reviewed-by: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/google/brya/Kconfig M src/mainboard/google/brya/variants/uldrenite/overridetree.cb 2 files changed, 93 insertions(+), 0 deletions(-) Approvals: Subrata Banik: Looks good to me, approved Dtrain Hsu: Looks good to me, approved Kapil Porwal: Looks good to me, approved build bot (Jenkins): Verified Eric Lai: Looks good to me, approved diff --git a/src/mainboard/google/brya/Kconfig b/src/mainboard/google/brya/Kconfig index bd8ee2b..d894f7f 100644 --- a/src/mainboard/google/brya/Kconfig +++ b/src/mainboard/google/brya/Kconfig @@ -612,6 +612,7 @@ config BOARD_GOOGLE_ULDRENITE select BOARD_GOOGLE_BASEBOARD_TRULO + select SOC_INTEL_TCSS_USE_PDC_PMC_USBC_MUX_CONFIGURATION config BOARD_GOOGLE_VELL select BOARD_GOOGLE_BASEBOARD_BRYA diff --git a/src/mainboard/google/brya/variants/uldrenite/overridetree.cb b/src/mainboard/google/brya/variants/uldrenite/overridetree.cb index 3483ecf..d225e19 100644 --- a/src/mainboard/google/brya/variants/uldrenite/overridetree.cb +++ b/src/mainboard/google/brya/variants/uldrenite/overridetree.cb @@ -58,6 +58,23 @@ # [6:0] Rx Strobe Delay DLL 2(HS400 Mode), each 125ps, range: 0 - 39. register "common_soc_config.emmc_dll.emmc_rx_strobe_cntl" = "0x01515" + register "usb2_ports[0]" = "USB2_PORT_TYPE_C(OC_SKIP)" # USB2_C0 + register "usb2_ports[1]" = "USB2_PORT_TYPE_C(OC_SKIP)" # USB2_C1 + register "usb2_ports[2]" = "USB2_PORT_MID(OC_SKIP)" # USB2_A0 + register "usb2_ports[4]" = "USB2_PORT_MID(OC_SKIP)" # WWAN + register "usb2_ports[5]" = "USB2_PORT_MID(OC_SKIP)" # User Facing Camera + register "usb2_ports[7]" = "USB2_PORT_MID(OC_SKIP)" # World Facing Camera + + register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC_SKIP)" # USB3/2 Type A port A0 + + register "tcss_ports[0]" = "TCSS_PORT_DEFAULT(OC_SKIP)" + register "tcss_ports[1]" = "TCSS_PORT_DEFAULT(OC_SKIP)" + + # Bit 0 - C0 has no redriver, so enable SBU muxing in the SoC. + # Bit 2 - C1 has a redriver which does SBU muxing. + # Bit 1,3 - AUX lines are not swapped on the motherboard for either C0 or C1. + register "tcss_aux_ori" = "0" + # Configure external V1P05/Vnn/VnnSx Rails register "ext_fivr_settings" = "{ .configure_ext_fivr = 1, @@ -193,6 +210,81 @@ device generic 0 on end end end # DPTF + device ref tcss_xhci on + chip drivers/usb/acpi + device ref tcss_root_hub on + chip drivers/usb/acpi + register "desc" = ""USB3 Type-C Port C0 (MLB)"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + register "use_custom_pld" = "true" + register "custom_pld" = "ACPI_PLD_TYPE_C(LEFT, LEFT, ACPI_PLD_GROUP(1, 1))" + device ref tcss_usb3_port1 on end + end + chip drivers/usb/acpi + register "desc" = ""USB3 Type-C Port C1 (MLB)"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + register "use_custom_pld" = "true" + register "custom_pld" = "ACPI_PLD_TYPE_C(LEFT, RIGHT, ACPI_PLD_GROUP(2, 1))" + device ref tcss_usb3_port2 on end + end + end + end + end + device ref xhci on + chip drivers/usb/acpi + device ref xhci_root_hub on + chip drivers/usb/acpi + register "desc" = ""USB2 Type-C Port C0 (MLB)"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + register "use_custom_pld" = "true" + register "custom_pld" = "ACPI_PLD_TYPE_C(LEFT, LEFT, ACPI_PLD_GROUP(1, 1))" + device ref usb2_port1 on end + end + chip drivers/usb/acpi + register "desc" = ""USB2 Type-C Port C1 (MLB)"" + register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" + register "use_custom_pld" = "true" + register "custom_pld" = "ACPI_PLD_TYPE_C(LEFT, LEFT, ACPI_PLD_GROUP(2, 1))" + device ref usb2_port2 on end + end + chip drivers/usb/acpi + register "desc" = ""USB2 Type-A Port A0 (MLB)"" + register "type" = "UPC_TYPE_A" + register "use_custom_pld" = "true" + register "custom_pld" = "ACPI_PLD_TYPE_A(LEFT, RIGHT, ACPI_PLD_GROUP(3, 1))" + device ref usb2_port3 on end + end + chip drivers/usb/acpi + register "desc" = ""USB2 WWAN"" + register "type" = "UPC_TYPE_INTERNAL" + device ref usb2_port5 on end + end + chip drivers/usb/acpi + register "desc" = ""USB2 User Facing Camera"" + register "type" = "UPC_TYPE_INTERNAL" + device ref usb2_port6 on end + end + chip drivers/usb/acpi + register "desc" = ""USB2 World Facing Camera"" + register "type" = "UPC_TYPE_INTERNAL" + device ref usb2_port8 on end + end + chip drivers/usb/acpi + register "desc" = ""USB2 Bluetooth"" + register "type" = "UPC_TYPE_INTERNAL" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A13)" + device ref usb2_port10 on end + end + chip drivers/usb/acpi + register "desc" = ""USB3 Type-A Port A0 (MLB)"" + register "type" = "UPC_TYPE_USB3_A" + register "use_custom_pld" = "true" + register "custom_pld" = "ACPI_PLD_TYPE_A(LEFT, RIGHT, ACPI_PLD_GROUP(3, 1))" + device ref usb3_port1 on end + end + end + end + end device ref i2c0 on chip drivers/i2c/tpm register "hid" = ""GOOG0005"" -- To view, visit https://review.coreboot.org/c/coreboot/+/85582?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: I8fa3bf958faaf17b35e33fe93e2a11e0d2eb48d6 Gerrit-Change-Number: 85582 Gerrit-PatchSet: 6 Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.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: 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-CC: Frank Wu <frank_wu(a)compal.corp-partner.google.com> Gerrit-CC: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[M] Change in coreboot[main]: mb/trulo/var/uldrenite: Configure serial_io and I2C
by Subrata Banik (Code Review) 18 Dec '24

18 Dec '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85581?usp=email ) Change subject: mb/trulo/var/uldrenite: Configure serial_io and I2C ...................................................................... mb/trulo/var/uldrenite: Configure serial_io and I2C 1. Configure serial_io and I2C 2. Add I2C devices BUG=b:380789023 TEST=emerge-nissa coreboot Change-Id: I3c1b6028c7e4392a2142851d5fd0199954e2272d Signed-off-by: John Su <john_su(a)compal.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85581 Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> Reviewed-by: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/google/brya/Kconfig M src/mainboard/google/brya/variants/uldrenite/overridetree.cb 2 files changed, 189 insertions(+), 0 deletions(-) Approvals: Dtrain Hsu: Looks good to me, approved Kapil Porwal: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/mainboard/google/brya/Kconfig b/src/mainboard/google/brya/Kconfig index 8f5c8dc..bd8ee2b 100644 --- a/src/mainboard/google/brya/Kconfig +++ b/src/mainboard/google/brya/Kconfig @@ -764,6 +764,7 @@ default 0x0 if BOARD_GOOGLE_TIVVIKS default 0x0 if BOARD_GOOGLE_TRULO default 0x0 if BOARD_GOOGLE_ULDREN + default 0x0 if BOARD_GOOGLE_ULDRENITE default 0x1 if BOARD_GOOGLE_VELL default 0x1 if BOARD_GOOGLE_VOLMAR default 0x0 if BOARD_GOOGLE_XIVU @@ -841,6 +842,7 @@ default 13 if BOARD_GOOGLE_TIVVIKS default 17 if BOARD_GOOGLE_TRULO default 13 if BOARD_GOOGLE_ULDREN + default 17 if BOARD_GOOGLE_ULDRENITE default 13 if BOARD_GOOGLE_VELL default 13 if BOARD_GOOGLE_VOLMAR default 13 if BOARD_GOOGLE_XIVU diff --git a/src/mainboard/google/brya/variants/uldrenite/overridetree.cb b/src/mainboard/google/brya/variants/uldrenite/overridetree.cb index 9eaa0ac..3483ecf 100644 --- a/src/mainboard/google/brya/variants/uldrenite/overridetree.cb +++ b/src/mainboard/google/brya/variants/uldrenite/overridetree.cb @@ -73,6 +73,67 @@ .vnn_icc_max_ma = 500, }" + register "serial_io_i2c_mode" = "{ + [PchSerialIoIndexI2C0] = PchSerialIoPci, + [PchSerialIoIndexI2C1] = PchSerialIoPci, + [PchSerialIoIndexI2C2] = PchSerialIoDisabled, + [PchSerialIoIndexI2C3] = PchSerialIoDisabled, + [PchSerialIoIndexI2C4] = PchSerialIoDisabled, + [PchSerialIoIndexI2C5] = PchSerialIoPci, + }" + + register "serial_io_gspi_mode" = "{ + [PchSerialIoIndexGSPI0] = PchSerialIoDisabled, + [PchSerialIoIndexGSPI1] = PchSerialIoDisabled, + }" + + register "serial_io_uart_mode" = "{ + [PchSerialIoIndexUART0] = PchSerialIoPci, + [PchSerialIoIndexUART1] = PchSerialIoDisabled, + [PchSerialIoIndexUART2] = PchSerialIoDisabled, + }" + + # Intel Common SoC Config + #+-------------------+---------------------------+ + #| Field | Value | + #+-------------------+---------------------------+ + #| I2C0 | TPM. Early init is | + #| | required to set up a BAR | + #| | for TPM communication | + #| I2C1 | Trackpad | + #| I2C5 | Touchscreen | + #+-------------------+---------------------------+ + register "common_soc_config" = "{ + .i2c[0] = { + .early_init = 1, + .speed = I2C_SPEED_FAST_PLUS, + .speed_config[0] = { + .speed = I2C_SPEED_FAST_PLUS, + .scl_lcnt = 55, + .scl_hcnt = 30, + .sda_hold = 7, + } + }, + .i2c[1] = { + .speed = I2C_SPEED_FAST, + .speed_config[0] = { + .speed = I2C_SPEED_FAST, + .scl_lcnt = 158, + .scl_hcnt = 79, + .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, + } + }, + }" + register "power_limits_config[ADL_N_041_6W_CORE]" = "{ .tdp_pl1_override = 10, .tdp_pl2_override = 25, @@ -132,6 +193,132 @@ device generic 0 on end end end # DPTF + device ref i2c0 on + chip drivers/i2c/tpm + register "hid" = ""GOOG0005"" + register "irq" = "ACPI_IRQ_EDGE_LOW(GPP_A17_IRQ)" + device i2c 50 on end + end + end # I2C0 + device ref i2c1 on + chip drivers/i2c/generic + register "hid" = ""ELAN0000"" + register "desc" = ""ELAN Touchpad"" + register "irq" = "ACPI_IRQ_WAKE_LEVEL_LOW(GPP_B3_IRQ)" + register "wake" = "GPE0_DW1_03" + register "detect" = "1" + device i2c 15 on end + end + chip drivers/i2c/hid + register "generic.hid" = ""PNP0C50"" + register "generic.desc" = ""Cirque Touchpad"" + register "generic.irq" = "ACPI_IRQ_WAKE_LEVEL_LOW(GPP_B3_IRQ)" + register "generic.detect" = "1" + register "generic.wake" = "GPE0_DW1_03" + register "hid_desc_reg_offset" = "0x20" + device i2c 2c on end + end + end # I2C1 + device ref i2c5 on + chip drivers/i2c/generic + register "hid" = ""ELAN0001"" + register "desc" = ""ELAN Touchscreen"" + register "irq" = "ACPI_IRQ_LEVEL_LOW(GPP_D16_IRQ)" + register "detect" = "1" + register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_D15)" + register "reset_delay_ms" = "20" + register "reset_off_delay_ms" = "2" + register "stop_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A21)" + register "stop_delay_ms" = "150" + register "stop_off_delay_ms" = "2" + register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_E17)" + register "enable_delay_ms" = "1" + register "has_power_resource" = "1" + device i2c 10 on end + end # EKTH5012 + chip drivers/i2c/hid + register "generic.hid" = ""ELAN900C"" + 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" = "6" + register "generic.reset_off_delay_ms" = "1" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_E17)" + register "generic.enable_delay_ms" = "6" + register "generic.stop_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A21)" + register "generic.stop_delay_ms" = "150" + register "generic.has_power_resource" = "1" + register "hid_desc_reg_offset" = "0x01" + device i2c 16 on end + end # EKTH7D18 + chip drivers/i2c/hid + register "generic.hid" = ""GXTP7996"" + register "generic.desc" = ""Goodix 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" = "100" + register "generic.stop_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A21)" + register "generic.stop_delay_ms" = "200" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_E17)" + register "generic.enable_delay_ms" = "10" + register "generic.has_power_resource" = "1" + register "hid_desc_reg_offset" = "0x01" + device i2c 5d on end + end # GXTP7996 + chip drivers/i2c/hid + register "generic.hid" = ""WDHT0002"" + register "generic.desc" = ""WDT 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.stop_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A21)" + register "generic.stop_delay_ms" = "130" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_E17)" + register "generic.enable_delay_ms" = "1" + register "generic.has_power_resource" = "1" + register "hid_desc_reg_offset" = "0x20" + device i2c 2c on end + end # WDT8752A + chip drivers/i2c/hid + register "generic.hid" = ""WDHT2601"" + register "generic.desc" = ""WDT 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.stop_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A21)" + register "generic.stop_delay_ms" = "130" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_E17)" + register "generic.enable_delay_ms" = "1" + register "generic.has_power_resource" = "1" + register "hid_desc_reg_offset" = "0x20" + device i2c 3c on end + end # WDT8790A + chip drivers/i2c/hid + register "generic.hid" = ""GTCH7502"" + register "generic.desc" = ""G2TOUCH 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" = "100" + register "generic.stop_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_A21)" + register "generic.stop_delay_ms" = "30" + register "generic.enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_E17)" + register "generic.enable_delay_ms" = "30" + register "generic.has_power_resource" = "1" + register "hid_desc_reg_offset" = "0x01" + device i2c 40 on end + end # GTCH7502 + end # I2C5 device ref emmc on end + device ref uart0 on end + device ref pch_espi on + chip ec/google/chromeec + device pnp 0c09.0 on end + end + end end end -- To view, visit https://review.coreboot.org/c/coreboot/+/85581?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: I3c1b6028c7e4392a2142851d5fd0199954e2272d Gerrit-Change-Number: 85581 Gerrit-PatchSet: 12 Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.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: 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-CC: Frank Wu <frank_wu(a)compal.corp-partner.google.com> Gerrit-CC: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[M] Change in coreboot[main]: mb/trulo/var/uldrenite: Enable eMMC and DLL tuning parameters
by Subrata Banik (Code Review) 18 Dec '24

18 Dec '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85579?usp=email ) Change subject: mb/trulo/var/uldrenite: Enable eMMC and DLL tuning parameters ...................................................................... mb/trulo/var/uldrenite: Enable eMMC and DLL tuning parameters Enable eMMC and add DLL tuning parameters. BUG=b:380789023 TEST=emerge-nissa coreboot Change-Id: I5f0fdb31ce4eaad5537df36c4165c404239a7bd4 Signed-off-by: John Su <john_su(a)compal.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85579 Reviewed-by: Eric Lai <ericllai(a)google.com> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> --- M src/mainboard/google/brya/variants/uldrenite/overridetree.cb 1 file changed, 50 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Kapil Porwal: Looks good to me, approved Dtrain Hsu: Looks good to me, approved Eric Lai: Looks good to me, approved diff --git a/src/mainboard/google/brya/variants/uldrenite/overridetree.cb b/src/mainboard/google/brya/variants/uldrenite/overridetree.cb index 2a2425e..9eaa0ac 100644 --- a/src/mainboard/google/brya/variants/uldrenite/overridetree.cb +++ b/src/mainboard/google/brya/variants/uldrenite/overridetree.cb @@ -9,6 +9,55 @@ register "tcc_offset" = "5" # TCC of 100 + # eMMC HS400 + register "emmc_enable_hs400_mode" = "true" + + #eMMC DLL tuning parameters + # EMMC Tx CMD Delay + # Refer to EDS-Vol2-42.3.7. + # [14:8] steps of delay for DDR mode, each 125ps, range: 0 - 39. + # [6:0] steps of delay for SDR mode, each 125ps, range: 0 - 39. + register "common_soc_config.emmc_dll.emmc_tx_cmd_cntl" = "0x505" + + # EMMC TX DATA Delay 1 + # Refer to EDS-Vol2-42.3.8. + # [14:8] steps of delay for HS400, each 125ps, range: 0 - 78. + # [6:0] steps of delay for SDR104/HS200, each 125ps, range: 0 - 79. + register "common_soc_config.emmc_dll.emmc_tx_data_cntl1" = "0x909" + + # EMMC TX DATA Delay 2 + # Refer to EDS-Vol2-42.3.9. + # [30:24] steps of delay for SDR50, each 125ps, range: 0 - 79. + # [22:16] steps of delay for DDR50, each 125ps, range: 0 - 78. + # [14:8] steps of delay for SDR25/HS50, each 125ps, range: 0 -79. + # [6:0] steps of delay for SDR12, each 125ps. Range: 0 - 79. + register "common_soc_config.emmc_dll.emmc_tx_data_cntl2" = "0x1C2A2828" + + # EMMC RX CMD/DATA Delay 1 + # Refer to EDS-Vol2-42.3.10. + # [30:24] steps of delay for SDR50, each 125ps, range: 0 - 119. + # [22:16] steps of delay for DDR50, each 125ps, range: 0 - 78. + # [14:8] steps of delay for SDR25/HS50, each 125ps, range: 0 - 119. + # [6:0] steps of delay for SDR12, each 125ps, range: 0 - 119. + register "common_soc_config.emmc_dll.emmc_rx_cmd_data_cntl1" = "0x1C1B4F1B" + + # EMMC RX CMD/DATA Delay 2 + # Refer to EDS-Vol2-42.3.12. + # [17:16] stands for Rx Clock before Output Buffer, + # 00: Rx clock after output buffer, + # 01: Rx clock before output buffer, + # 10: Automatic selection based on working mode. + # 11: Reserved + # [14:8] steps of delay for Auto Tuning Mode, each 125ps, range: 0 - 39. + # [6:0] steps of delay for HS200, each 125ps, range: 0 - 79. + register "common_soc_config.emmc_dll.emmc_rx_cmd_data_cntl2" = "0x1004E" + + # EMMC Rx Strobe Delay + # Refer to EDS-Vol2-42.3.11. + # [14:8] Rx Strobe Delay DLL 1(HS400 Mode), each 125ps, range: 0 - 39. + # [6:0] Rx Strobe Delay DLL 2(HS400 Mode), each 125ps, range: 0 - 39. + register "common_soc_config.emmc_dll.emmc_rx_strobe_cntl" = "0x01515" + # Configure external V1P05/Vnn/VnnSx Rails register "ext_fivr_settings" = "{ .configure_ext_fivr = 1, @@ -83,5 +132,6 @@ device generic 0 on end end end # DPTF + device ref emmc on end end end -- To view, visit https://review.coreboot.org/c/coreboot/+/85579?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: I5f0fdb31ce4eaad5537df36c4165c404239a7bd4 Gerrit-Change-Number: 85579 Gerrit-PatchSet: 5 Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.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: 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-CC: Frank Wu <frank_wu(a)compal.corp-partner.google.com> Gerrit-CC: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com>

1 0

[M] Change in coreboot[main]: mb/trulo/var/uldrenite: Enable DPTF, S0ix and configure FIVR setting
by Subrata Banik (Code Review) 18 Dec '24

18 Dec '24

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85578?usp=email ) Change subject: mb/trulo/var/uldrenite: Enable DPTF, S0ix and configure FIVR setting ...................................................................... mb/trulo/var/uldrenite: Enable DPTF, S0ix and configure FIVR setting 1. Enable DPTF 2. Enable S0ix 3. Configure external fivr setting BUG=b:380789023 TEST=emerge-nissa coreboot Change-Id: Id7777b7560b40c1427df9645d991240c027e58e4 Signed-off-by: John Su <john_su(a)compal.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85578 Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> Reviewed-by: Eric Lai <ericllai(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> --- M src/mainboard/google/brya/variants/uldrenite/overridetree.cb 1 file changed, 83 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Eric Lai: Looks good to me, approved Dtrain Hsu: Looks good to me, approved Kapil Porwal: Looks good to me, approved diff --git a/src/mainboard/google/brya/variants/uldrenite/overridetree.cb b/src/mainboard/google/brya/variants/uldrenite/overridetree.cb index a5e2217..2a2425e 100644 --- a/src/mainboard/google/brya/variants/uldrenite/overridetree.cb +++ b/src/mainboard/google/brya/variants/uldrenite/overridetree.cb @@ -1,4 +1,87 @@ chip soc/intel/alderlake + register "sagv" = "SaGv_Enabled" + + # S0ix enable + register "s0ix_enable" = "true" + + # DPTF enable + register "dptf_enable" = "1" + + register "tcc_offset" = "5" # TCC of 100 + + # Configure external V1P05/Vnn/VnnSx Rails + register "ext_fivr_settings" = "{ + .configure_ext_fivr = 1, + .v1p05_enable_bitmap = FIVR_ENABLE_ALL_SX & ~FIVR_ENABLE_S0, + .vnn_enable_bitmap = FIVR_ENABLE_ALL_SX, + .vnn_sx_enable_bitmap = FIVR_ENABLE_ALL_SX, + .v1p05_supported_voltage_bitmap = FIVR_VOLTAGE_NORMAL, + .vnn_supported_voltage_bitmap = FIVR_VOLTAGE_MIN_ACTIVE, + .v1p05_voltage_mv = 1050, + .vnn_voltage_mv = 780, + .vnn_sx_voltage_mv = 1050, + .v1p05_icc_max_ma = 500, + .vnn_icc_max_ma = 500, + }" + + register "power_limits_config[ADL_N_041_6W_CORE]" = "{ + .tdp_pl1_override = 10, + .tdp_pl2_override = 25, + .tdp_pl4 = 78, + }" + device domain 0 on + device ref igpu on end + device ref dtt on + chip drivers/intel/dptf + ## sensor information + register "options.tsr[0].desc" = ""DDR"" + register "options.tsr[1].desc" = ""charger"" + register "options.tsr[2].desc" = ""ambient"" + + ## Passive Policy + register "policies.passive" = "{ + [0] = DPTF_PASSIVE(CPU, CPU, 95, 5000), + [1] = DPTF_PASSIVE(CPU, TEMP_SENSOR_0, 90, 5000), + [2] = DPTF_PASSIVE(CPU, TEMP_SENSOR_1, 90, 5000), + [3] = DPTF_PASSIVE(CPU, TEMP_SENSOR_2, 90, 5000), + }" + + ## Critical Policy + register "policies.critical" = "{ + [0] = DPTF_CRITICAL(CPU, 105, SHUTDOWN), + [1] = DPTF_CRITICAL(TEMP_SENSOR_0, 98, SHUTDOWN), + [2] = DPTF_CRITICAL(TEMP_SENSOR_1, 98, SHUTDOWN), + [3] = DPTF_CRITICAL(TEMP_SENSOR_2, 98, SHUTDOWN), + }" + + register "controls.power_limits" = "{ + .pl1 = { + .min_power = 6000, + .max_power = 10000, + .time_window_min = 28 * MSECS_PER_SEC, + .time_window_max = 28 * MSECS_PER_SEC, + .granularity = 500 + }, + .pl2 = { + .min_power = 25000, + .max_power = 25000, + .time_window_min = 32 * MSECS_PER_SEC, + .time_window_max = 32 * MSECS_PER_SEC, + .granularity = 500 + } + }" + + ## Charger Performance Control (Control, mA) + register "controls.charger_perf" = "{ + [0] = { 255, 1700 }, + [1] = { 24, 1500 }, + [2] = { 16, 1000 }, + [3] = { 8, 500 } + }" + + device generic 0 on end + end + end # DPTF end end -- To view, visit https://review.coreboot.org/c/coreboot/+/85578?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: Id7777b7560b40c1427df9645d991240c027e58e4 Gerrit-Change-Number: 85578 Gerrit-PatchSet: 6 Gerrit-Owner: John Su <john_su(a)compal.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.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: 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-CC: Frank Wu <frank_wu(a)compal.corp-partner.google.com> Gerrit-CC: Jamie Chen <jamie_chen(a)compal.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[S] Change in coreboot[main]: soc/mediatek/mt8196: Add pwrsel driver
by Jarried Lin (Code Review) 18 Dec '24

18 Dec '24

Attention is currently required from: Hung-Te Lin, Paul Menzel, Yidi Lin, Yu-Ping Wu. Jarried Lin has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85613?usp=email ) Change subject: soc/mediatek/mt8196: Add pwrsel driver ...................................................................... Patch Set 3: (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/85613/comment/996deb66_c1f5deaa?us… : PS1, Line 11: > What values (profiles?) can be written by the firmware? Sorry, I don't quite understand what you mean. This patch is just to initialize the PWRSEL hardware. Is it related to how mcupm operates? Thank you. https://review.coreboot.org/c/coreboot/+/85613/comment/5f4a2650_523f2330?us… : PS1, Line 14: PWR_SEL = 0x0 > Aren’t two log messages added in the code? please check 85620 File src/soc/mediatek/mt8196/mtk_pwrsel.c: https://review.coreboot.org/c/coreboot/+/85613/comment/667a48a4_00941632?us… : PS1, Line 22: printk(BIOS_INFO, "PWR_SEL = %#x\n", > It’d be great to have a comment documenting the possible values. Done https://review.coreboot.org/c/coreboot/+/85613/comment/cf2ba30d_b0750fe0?us… : PS1, Line 22: printk(BIOS_INFO, "PWR_SEL = %#x\n", : read32p(MCUSYS_BASE + OFFSET_PWRSEL)); : printk(BIOS_INFO, "PWRSEL_CONFIG = %#x\n", : read32p(MFG_VCORE_AO_RPC_PWRSEL_CONFIG)); > For an info level messages it’d be great to make this message user readable/understandable. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/85613?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: Ib1b8588810fdad5c675dee865627337269b57d18 Gerrit-Change-Number: 85613 Gerrit-PatchSet: 3 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-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: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Wed, 18 Dec 2024 04:41:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[XS] Change in coreboot[main]: mb/google/rauru: Add pwrsel init in romstage
by Jarried Lin (Code Review) 18 Dec '24

18 Dec '24

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin, Yu-Ping Wu. 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/+/85620?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+2 by Yu-Ping Wu, Verified+1 by build bot (Jenkins) The change is no longer submittable: Code-Review and Verified are unsatisfied now. Change subject: mb/google/rauru: Add pwrsel init in romstage ...................................................................... mb/google/rauru: Add pwrsel init in romstage BUG=b:317009620 TEST=build pass, reg set ok, log show: PWR_SEL = 0x0 PWRSEL_CONFIG = 0x7fff Change-Id: I37c0fb905f99491ca99f04bc5bfa6abfb1c01059 Signed-off-by: Jarried Lin <jarried.lin(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/20/85620/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85620?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: I37c0fb905f99491ca99f04bc5bfa6abfb1c01059 Gerrit-Change-Number: 85620 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: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com>

1 0

[S] Change in coreboot[main]: soc/mediatek/mt8196: Add pwrsel driver
by Jarried Lin (Code Review) 18 Dec '24

18 Dec '24

Attention is currently required from: Hung-Te Lin, Jarried Lin, Yidi Lin, Yu-Ping Wu. 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/+/85613?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+1 by Yu-Ping Wu, Verified+1 by build bot (Jenkins) Change subject: soc/mediatek/mt8196: Add pwrsel driver ...................................................................... soc/mediatek/mt8196: Add pwrsel driver The MediaTek pwrsel (Power Select) is mainly used to reduce power consumption, controlled by mcupm. BUG=b:317009620 TEST=Build pass Change-Id: Ib1b8588810fdad5c675dee865627337269b57d18 Signed-off-by: Jarried Lin <jarried.lin(a)mediatek.corp-partner.google.com> --- M src/soc/mediatek/mt8196/Makefile.mk M src/soc/mediatek/mt8196/include/soc/addressmap.h A src/soc/mediatek/mt8196/include/soc/mtk_pwrsel.h A src/soc/mediatek/mt8196/mtk_pwrsel.c 4 files changed, 47 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/85613/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/85613?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: Ib1b8588810fdad5c675dee865627337269b57d18 Gerrit-Change-Number: 85613 Gerrit-PatchSet: 3 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com>

1 0

[M] Change in coreboot[main]: soc/mediatek/mt8196: Add unmask eint event for bootblock
by Jarried Lin (Code Review) 18 Dec '24

18 Dec '24

Jarried Lin has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/84025?usp=email ) Change subject: soc/mediatek/mt8196: Add unmask eint event for bootblock ...................................................................... Abandoned -- To view, visit https://review.coreboot.org/c/coreboot/+/84025?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: I4bf3820a89172186b8f51591f8760787affbb7a3 Gerrit-Change-Number: 84025 Gerrit-PatchSet: 14 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Chhao Chang <ot_chhao.chang(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>

1 0

[M] Change in coreboot[main]: soc/mediatek/mt8196: Add unmask eint event for bootblock
by Yu-Ping Wu (Code Review) 18 Dec '24

18 Dec '24

Attention is currently required from: Chhao Chang, Hung-Te Lin, Jarried Lin. Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/84025?usp=email ) Change subject: soc/mediatek/mt8196: Add unmask eint event for bootblock ...................................................................... Patch Set 14: (1 comment) File src/soc/mediatek/common/eint_event.c: https://review.coreboot.org/c/coreboot/+/84025/comment/bec73ea1_738305fc?us… : PS14, Line 10: 0x880 > Do you mean we don't need this patch anymore? If so, please abandon this patch. Please abandon. -- To view, visit https://review.coreboot.org/c/coreboot/+/84025?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: I4bf3820a89172186b8f51591f8760787affbb7a3 Gerrit-Change-Number: 84025 Gerrit-PatchSet: 14 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Chhao Chang <ot_chhao.chang(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: Chhao Chang <ot_chhao.chang(a)mediatek.corp-partner.google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Wed, 18 Dec 2024 04:04:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Chhao Chang <ot_chhao.chang(a)mediatek.corp-partner.google.com> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com>

1 0

[XL] Change in coreboot[main]: soc/mediatek/mt8196: Add PMIC MT6316 driver
by Yu-Ping Wu (Code Review) 18 Dec '24

18 Dec '24

Attention is currently required from: Hope Wang, Hung-Te Lin, Jarried Lin. Yu-Ping Wu has posted comments on this change by Jarried Lin. ( https://review.coreboot.org/c/coreboot/+/85630?usp=email ) Change subject: soc/mediatek/mt8196: Add PMIC MT6316 driver ...................................................................... Patch Set 3: (18 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/85630/comment/88e1dada_3b5ed226?us… : PS3, Line 9: Add MT6316 driver in SoC folder Missing `.` File src/soc/mediatek/common/include/soc/mt6316.h: https://review.coreboot.org/c/coreboot/+/85630/comment/cc2d7e76_14b0ee50?us… : PS3, Line 21: max MAX File src/soc/mediatek/common/mt6316.c: https://review.coreboot.org/c/coreboot/+/85630/comment/1bcffe99_f1b87596?us… : PS2, Line 14: assert(pmif_arb); : pmif_arb->read(pmif_arb, slvid, reg, data); > Please follow other PMICs' read/write semantic. […] Is `data` supposed to be `u8`? File src/soc/mediatek/common/mt6316.c: https://review.coreboot.org/c/coreboot/+/85630/comment/791b7ec1_2bb15449?us… : PS3, Line 12: enum spmi_slave Please consistently use either `u32` or `enum spmi_slave` in all of the functions. https://review.coreboot.org/c/coreboot/+/85630/comment/a3fbe0f9_d9cc559d?us… : PS3, Line 77: ( remove https://review.coreboot.org/c/coreboot/+/85630/comment/ffecfd88_046dfebb?us… : PS3, Line 78: ( remove https://review.coreboot.org/c/coreboot/+/85630/comment/389f3945_ec5b3030?us… : PS3, Line 107: ( remove https://review.coreboot.org/c/coreboot/+/85630/comment/f7e196ed_2c728ae4?us… : PS3, Line 108: ( remove https://review.coreboot.org/c/coreboot/+/85630/comment/d96b4eab_1c4c5a43?us… : PS3, Line 111: u8 Is this supposed to be a boolean? https://review.coreboot.org/c/coreboot/+/85630/comment/b6fd6828_7717eb83?us… : PS3, Line 160: con9 What does con9 mean? https://review.coreboot.org/c/coreboot/+/85630/comment/b0fc3387_5105dbc5?us… : PS3, Line 164: 0x222 > Is the mask correct ? Doesn't `pmif_arb->write` send one byte per call ? 0x222 is `reg`, not the mask. Please define a macro for it. https://review.coreboot.org/c/coreboot/+/85630/comment/6d7d712b_a9215b5c?us… : PS3, Line 169: reg_val_6 > We expect to read one byte back, right ? +1. Please change the type of `data` to u8 for both `mt6316_read` and `mt6316_write`. https://review.coreboot.org/c/coreboot/+/85630/comment/04050582_2b815af3?us… : PS3, Line 181: /* for N*/ remove. What does that mean BTW? https://review.coreboot.org/c/coreboot/+/85630/comment/6ff087d9_8f8354d4?us… : PS3, Line 202: mt6316_wdt_enable(SPMI_SLAVE_6); : mt6316_wdt_enable(SPMI_SLAVE_7); : mt6316_wdt_enable(SPMI_SLAVE_8); : mt6316_wdt_enable(SPMI_SLAVE_15); : Define a global static const array for `[SPMI_SLAVE_6, SPMI_SLAVE_7, SPMI_SLAVE_8, SPMI_SLAVE_15]` and then use for loops in `mt6316_set_all_test_con9`, `mt6316_init` and here. Also, can we pass `SPMI_SLAVE_*` as an argument to `mt6316_init_setting(enum spmi_slave slvid)`? File src/soc/mediatek/mt8196/mt6316.c: https://review.coreboot.org/c/coreboot/+/85630/comment/85b3d2fb_993c7155?us… : PS3, Line 6: MeidaTek MediaTek https://review.coreboot.org/c/coreboot/+/85630/comment/e34309a7_eacd9494?us… : PS3, Line 885: for (int i = 0; i < ARRAY_SIZE(init_setting_s8); i++) Can we make the order consistent: 6, 7, 8, 15 (instead of 8, 6, 7, 15)? https://review.coreboot.org/c/coreboot/+/85630/comment/0979c2ec_ff63cb55?us… : PS3, Line 886: mt6316_write_field(SPMI_SLAVE_8, : init_setting_s8[i].addr, init_setting_s8[i].val, : init_setting_s8[i].mask, init_setting_s8[i].shift); Extract it to ``` static inline void write_field(u32 slvid, const struct mt6316_setting *setting) { mt6316_write_field(slvid, settings->addr, ...); } ``` https://review.coreboot.org/c/coreboot/+/85630/comment/90b70e89_2a3e5c48?us… : PS3, Line 889: printk(BIOS_INFO, "%s S8 done\n", __func__); Do we really need these logs? `mt6316_write_field` won't fail, so these logs will always be printed. -- To view, visit https://review.coreboot.org/c/coreboot/+/85630?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: I39e91d64e77cff03281845dfbea565e6ddf748f6 Gerrit-Change-Number: 85630 Gerrit-PatchSet: 3 Gerrit-Owner: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Reviewer: Hope Wang <hope.wang(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: Hope Wang <hope.wang(a)mediatek.corp-partner.google.com> Gerrit-Attention: Jarried Lin <jarried.lin(a)mediatek.com> Gerrit-Comment-Date: Wed, 18 Dec 2024 04:04:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: 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 December 2024