coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[XS] Change in coreboot[main]: soc/intel/adl: Guard TWL SoC missing UPDs for build integrity
by Subrata Banik (Code Review) 21 Mar '24

21 Mar '24

Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81376?usp=email ) Change subject: soc/intel/adl: Guard TWL SoC missing UPDs for build integrity ...................................................................... soc/intel/adl: Guard TWL SoC missing UPDs for build integrity Adds config-based guards for Usb4CmMode and CnviWifiCore UPDs, specific to Twin Lake SoCs (SOC_INTEL_TWINLAKE). Prevents compilation errors due to missing UPD definitions. BUG=b:330654700 TEST=Able to build google/tivviks. Change-Id: I6e0a9a7536df6295e23bf06003539e56bb98a311 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/soc/intel/alderlake/fsp_params.c 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/81376/1 diff --git a/src/soc/intel/alderlake/fsp_params.c b/src/soc/intel/alderlake/fsp_params.c index 4ddf1e4..33ebee3 100644 --- a/src/soc/intel/alderlake/fsp_params.c +++ b/src/soc/intel/alderlake/fsp_params.c @@ -683,7 +683,9 @@ s_cfg->UsbTcPortEn |= BIT(i); } +#if !CONFIG(SOC_INTEL_TWINLAKE) s_cfg->Usb4CmMode = CONFIG(SOFTWARE_CONNECTION_MANAGER); +#endif } static void fill_fsps_chipset_lockdown_params(FSP_S_CONFIG *s_cfg, @@ -831,7 +833,9 @@ const struct soc_intel_alderlake_config *config) { /* CNVi */ +#if !CONFIG(SOC_INTEL_TWINLAKE) s_cfg->CnviWifiCore = is_devfn_enabled(PCH_DEVFN_CNVI_WIFI); +#endif s_cfg->CnviMode = is_devfn_enabled(PCH_DEVFN_CNVI_WIFI); s_cfg->CnviBtCore = config->cnvi_bt_core; s_cfg->CnviBtAudioOffload = config->cnvi_bt_audio_offload; -- To view, visit https://review.coreboot.org/c/coreboot/+/81376?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6e0a9a7536df6295e23bf06003539e56bb98a311 Gerrit-Change-Number: 81376 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: newchange

1 0

[L] Change in coreboot[main]: soc/intel/xeon_sp: Add support of _OSC ASL generation
by Shuo Liu (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu. Shuo Liu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/81375?usp=email ) Change subject: soc/intel/xeon_sp: Add support of _OSC ASL generation ...................................................................... soc/intel/xeon_sp: Add support of _OSC ASL generation Change-Id: I711ce5350d718e47feb2912555108801ad7f918d Signed-off-by: Shuo Liu <shuo.liu(a)intel.com> --- M src/soc/intel/xeon_sp/acpi.c M src/soc/intel/xeon_sp/gnr/soc_acpi.c M src/soc/intel/xeon_sp/include/soc/acpi.h 3 files changed, 340 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/81375/1 diff --git a/src/soc/intel/xeon_sp/acpi.c b/src/soc/intel/xeon_sp/acpi.c index 9ee8d6a..2a37070 100644 --- a/src/soc/intel/xeon_sp/acpi.c +++ b/src/soc/intel/xeon_sp/acpi.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ +#include <acpi/acpigen.h> #include <assert.h> #include <intelblocks/acpi.h> #include <soc/chip_common.h> @@ -157,3 +158,319 @@ return NULL; } + +#define PCI_HOST_BRIDGE_OSC_UUID "33db4d5b-1ff7-401c-9657-7441c03dd766" +#define CXL_HOST_BRIDGE_OSC_UUID "68f2d50b-c469-4d8a-bd3d-941a103fd3fc" + +#define OSC_RET_FAILURE 0x02 +#define OSC_RET_UNRECOGNIZED_UUID 0x04 +#define OSC_RET_UNRECOGNIZED_REV 0x08 +#define OSC_RET_CAPABILITIES_MASKED 0x10 + +#define OSC_QUERY_SUPPORT_SET 0x01 + +#define PCIE_NATIVE_HOTPLUG_CONTROL 0x01 +#define SHPC_NATIVE_HOTPLUG_CONTROL 0x02 +#define PCIE_PME_CONTROL 0x04 +#define PCIE_AER_CONTROL 0x08 +#define PCIE_CAP_STRUCTURE_CONTROL 0x10 +#define PCIE_LTR_CONTROL 0x20 + +#define ASL_BRANCH_HIT 0x01 +#define ASL_BRANCH_MISS 0x00 + +static void soc_acpigen_OSC_handle_pcie_request(const struct device *domain); +static void soc_acpigen_OSC_handle_cxl_request(const struct device *domain); + +/* + * soc_acpigen_write_OSC + * + * Reference: + * 6.2.11 in https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/06_Device_Configuration/Devic… + * + * _OSC ASL Arguments: (4) + * Arg0 - A Buffer containing a UUID + * Arg1 - An Integer containing a Revision ID of the buffer format + * Arg2 - An Integer containing a count of entries in Arg3 + * Arg3 - A Buffer containing a list of DWORD capabilities + * + * _OSC ASL Return Value: + * A Buffer containing a list of capabilities + * + * Local Variables Assignment: + * Local0 - Temp assigned + * Local1 - Temp assigned + * Local2 - Temp assigned + * Local3 - Temp assigned + * Local4 - Temp assigned + * Local5 - Not used + * Local6 - Not used + * Local7 - Backs up the input value of Arg3 + * + * Field Definitions: + * Name - With, Source, Offset, Description + * -------------------------------- + * QSUP - DWord, Local7, 0x00, Query support + * RETE - DWord, Arg3, 0x00, Returned errors + * SUPP - Dword, Arg3, 0x04, PCIe Features that OS supported + * CTRL - Dword, Arg3, 0x08, PCIe Features that firmware grant control to OS + * OTRL - Dword, Local7, 0x08, PCIe Features that OS requests for control + * SUPC - Dword, Arg3, 0x0C, CXL Features that OS supported + * CTRC - Dword, Arg3, 0x10, CXL Features that firmware grant control to OS + * OTRC - Dword, Local7, 0x10, CXL Features that OS requests for control + */ +void soc_acpigen_write_OSC(const struct device *domain) +{ + /* + * Method (_OSC, 4, NotSerialized) + * { + */ + acpigen_write_method("_OSC", 4); + + /* + * Local7 = Arg3 + * CreateDwordField (Local7, 0x00, QSUP) + * CreateDWordField (Arg3, 0x00, RETE) + * RETE = 0x0 + */ + acpigen_write_store_ops(ARG3_OP, LOCAL7_OP); + acpigen_write_create_dword_field(LOCAL7_OP, 0x00, "QSUP"); + acpigen_write_create_dword_field(ARG3_OP, 0x00, "RETE"); + acpigen_write_store_int_to_namestr(0x0, "RETE"); + + if (is_cxl_domain(domain)) { + /* + * If ((Arg0 != ToUUID (PCI_HOST_BRIDGE_OSC_UUID)) && + * (Arg0 != ToUUID (CXL_HOST_BRIDGE_OSC_UUID))) + */ + acpigen_write_if(); + acpigen_emit_byte(LAND_OP); + acpigen_emit_byte(LNOT_OP); + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_byte(ARG0_OP); + acpigen_write_uuid(PCI_HOST_BRIDGE_OSC_UUID); + acpigen_emit_byte(LNOT_OP); + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_byte(ARG0_OP); + acpigen_write_uuid(CXL_HOST_BRIDGE_OSC_UUID); + } else { + /* + * If (Arg0 != ToUUID (PCI_HOST_BRIDGE_OSC_UUID)) + */ + acpigen_write_if(); + acpigen_emit_byte(LNOT_OP); + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_byte(ARG0_OP); + acpigen_write_uuid(PCI_HOST_BRIDGE_OSC_UUID); + } + + /* + * { + * RETE |= OSC_RET_UNRECOGNIZED_UUID + * Return (Arg3) + * } + */ + acpigen_write_store(); + acpigen_write_integer(ASL_BRANCH_HIT); + acpigen_emit_byte(LOCAL0_OP); + + acpigen_write_else(); + + acpigen_write_store(); + acpigen_write_integer(ASL_BRANCH_MISS); + acpigen_emit_byte(LOCAL0_OP); + + acpigen_write_if_end(); + + acpigen_write_if_lequal_op_int(LOCAL0_OP, ASL_BRANCH_HIT); + + acpigen_write_to_integer_from_namestring("RETE", LOCAL1_OP); + acpigen_write_store_int_to_op(OSC_RET_UNRECOGNIZED_UUID, LOCAL2_OP); + acpigen_write_or(LOCAL1_OP, LOCAL2_OP, LOCAL1_OP); + acpigen_write_store_op_to_namestr(LOCAL1_OP, "RETE"); + acpigen_write_return_op(ARG3_OP); + + acpigen_write_if_end(); + + if (!is_cxl_domain(domain)) { + /* + * // Handle PCIe _OSC request + */ + soc_acpigen_OSC_handle_pcie_request(domain); + + } else { + /* + * If (Arg0 == ToUUID (PCI_HOST_BRIDGE_OSC_UUID)) + * { + * // Handle PCIe _OSC request + * } + */ + acpigen_write_if(); + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_byte(ARG0_OP); + acpigen_write_uuid(PCI_HOST_BRIDGE_OSC_UUID); + + soc_acpigen_OSC_handle_pcie_request(domain); + + acpigen_write_if_end(); + /* + * If (Arg0 == ToUUID (CXL_HOST_BRIDGE_OSC_UUID)) + * { + * // Handle CXL _OSC request + * } + */ + acpigen_write_if(); + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_byte(ARG0_OP); + acpigen_write_uuid(CXL_HOST_BRIDGE_OSC_UUID); + + soc_acpigen_OSC_handle_pcie_request(domain); + soc_acpigen_OSC_handle_cxl_request(domain); + + acpigen_write_if_end(); + } + + /* + * Return (Arg3) + */ + acpigen_write_return_op(ARG3_OP); + + /* + * } + */ + acpigen_pop_len(); +} + +void soc_acpigen_OSC_handle_pcie_request(const struct device *domain) +{ + unsigned long osc_pci_host_bridge_features = get_supported_pci_host_bridge_features(domain); + + /* + * If (Arg2 < 2)) + * { + * RETE |= OSC_RET_FAILURE + * Return (Arg3) + * } + */ + acpigen_write_if(); + acpigen_emit_byte(LLESS_OP); + acpigen_emit_byte(ARG2_OP); + acpigen_write_integer(0x2); + + acpigen_write_to_integer_from_namestring("RETE", LOCAL1_OP); + acpigen_write_store_int_to_op(OSC_RET_FAILURE, LOCAL2_OP); + acpigen_write_or(LOCAL1_OP, LOCAL2_OP, LOCAL1_OP); + acpigen_write_store_op_to_namestr(LOCAL1_OP, "RETE"); + acpigen_write_return_op(ARG3_OP); + + acpigen_write_if_end(); + + /* + * CreateDWordField (Arg3, 0x04, SUPP) + * CreateDWordField (Arg3, 0x08, CTRL) + * CreateDWordField (Local7, 0x08, OTRL) + */ + acpigen_write_create_dword_field(ARG3_OP, 0x04, "SUPP"); + acpigen_write_create_dword_field(ARG3_OP, 0x08, "CTRL"); + acpigen_write_create_dword_field(LOCAL7_OP, 0x08, "OTRL"); + + /* + * // Grant PCIe feature controls to OS + * CTRL &= osc_pci_host_bridge_features + */ + acpigen_write_to_integer_from_namestring("CTRL", LOCAL1_OP); + acpigen_write_store_int_to_op(osc_pci_host_bridge_features, LOCAL2_OP); + acpigen_write_and(LOCAL1_OP, LOCAL2_OP, LOCAL1_OP); + acpigen_write_store_op_to_namestr(LOCAL1_OP, "CTRL"); + + /* + * If (CTRL != OTRL) + * { + * RETE |= OSC_RET_CAPABILITIES_MASKED + * } + */ + acpigen_write_to_integer_from_namestring("CTRL", LOCAL1_OP); + acpigen_write_to_integer_from_namestring("OTRL", LOCAL2_OP); + acpigen_write_if(); + acpigen_emit_byte(LNOT_OP); + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_byte(LOCAL1_OP); + acpigen_emit_byte(LOCAL2_OP); + acpigen_write_to_integer_from_namestring("RETE", LOCAL3_OP); + acpigen_write_store_int_to_op(OSC_RET_CAPABILITIES_MASKED, LOCAL4_OP); + acpigen_write_or(LOCAL3_OP, LOCAL4_OP, LOCAL3_OP); + acpigen_write_store_op_to_namestr(LOCAL3_OP, "RETE"); + acpigen_write_if_end(); +}; + +void soc_acpigen_OSC_handle_cxl_request(const struct device *domain) +{ + unsigned long osc_cxl_host_bridge_features = get_supported_cxl_host_bridge_features(domain); + + /* + * If (Arg2 < 4)) + * { + * RETE |= OSC_RET_FAILURE + * Return (Arg3) + * } + */ + acpigen_write_if(); + acpigen_emit_byte(LLESS_OP); + acpigen_emit_byte(ARG2_OP); + acpigen_write_integer(0x4); + + acpigen_write_to_integer_from_namestring("RETE", LOCAL1_OP); + acpigen_write_store_int_to_op(OSC_RET_FAILURE, LOCAL2_OP); + acpigen_write_or(LOCAL1_OP, LOCAL2_OP, LOCAL1_OP); + acpigen_write_store_op_to_namestr(LOCAL1_OP, "RETE"); + acpigen_write_return_op(ARG3_OP); + + acpigen_write_if_end(); + + /* + * CreateDWordField (Arg3, 0x0C, SUPC) + * CreateDWordField (Arg3, 0x10, CTRC) + * CreateDWordField (Local7, 0x10, OTRC) + */ + acpigen_write_create_dword_field(ARG3_OP, 0x0C, "SUPC"); + acpigen_write_create_dword_field(ARG3_OP, 0x10, "CTRC"); + acpigen_write_create_dword_field(LOCAL7_OP, 0x10, "OTRC"); + + /* + * // Grant CXL feature controls to OS + * CTRC &= osc_cxl_host_bridge_features + */ + acpigen_write_to_integer_from_namestring("CTRC", LOCAL1_OP); + acpigen_write_store_int_to_op(osc_cxl_host_bridge_features, LOCAL2_OP); + acpigen_write_and(LOCAL1_OP, LOCAL2_OP, LOCAL1_OP); + acpigen_write_store_op_to_namestr(LOCAL1_OP, "CTRC"); + + /* + * If (CTRC != OTRC) + * { + * RETE |= OSC_RET_CAPABILITIES_MASKED + * } + */ + acpigen_write_to_integer_from_namestring("CTRC", LOCAL1_OP); + acpigen_write_to_integer_from_namestring("OTRC", LOCAL2_OP); + acpigen_write_if(); + acpigen_emit_byte(LNOT_OP); + acpigen_emit_byte(LEQUAL_OP); + acpigen_emit_byte(LOCAL1_OP); + acpigen_emit_byte(LOCAL2_OP); + acpigen_write_to_integer_from_namestring("RETE", LOCAL3_OP); + acpigen_write_store_int_to_op(OSC_RET_CAPABILITIES_MASKED, LOCAL4_OP); + acpigen_write_or(LOCAL3_OP, LOCAL4_OP, LOCAL3_OP); + acpigen_write_store_op_to_namestr(LOCAL3_OP, "RETE"); + acpigen_write_if_end(); +}; + +__weak unsigned long get_supported_pci_host_bridge_features(const struct device *domain) +{ + return 0; +} + +__weak unsigned long get_supported_cxl_host_bridge_features(const struct device *domain) +{ + return 0; +} diff --git a/src/soc/intel/xeon_sp/gnr/soc_acpi.c b/src/soc/intel/xeon_sp/gnr/soc_acpi.c index ab4328a..fc83f09 100644 --- a/src/soc/intel/xeon_sp/gnr/soc_acpi.c +++ b/src/soc/intel/xeon_sp/gnr/soc_acpi.c @@ -95,6 +95,15 @@ acpigen_write_string(name); /* + * Method (_OSC, 4, NotSerialized) + * { + */ + soc_acpigen_write_OSC(domain); + /* + * } + */ + + /* * } //Device * } //Scope */ @@ -103,3 +112,13 @@ return; } + +unsigned long get_supported_pci_host_bridge_features(const struct device *domain) +{ + return 0; +} + +unsigned long get_supported_cxl_host_bridge_features(const struct device *domain) +{ + return 0; +} diff --git a/src/soc/intel/xeon_sp/include/soc/acpi.h b/src/soc/intel/xeon_sp/include/soc/acpi.h index e374544..367dc25 100644 --- a/src/soc/intel/xeon_sp/include/soc/acpi.h +++ b/src/soc/intel/xeon_sp/include/soc/acpi.h @@ -25,4 +25,8 @@ void iio_domain_set_acpi_name(struct device *dev, const char *prefix); +void soc_acpigen_write_OSC(const struct device *domain); +unsigned long get_supported_pci_host_bridge_features(const struct device *domain); +unsigned long get_supported_cxl_host_bridge_features(const struct device *domain); + #endif /* _SOC_ACPI_H_ */ -- To view, visit https://review.coreboot.org/c/coreboot/+/81375?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I711ce5350d718e47feb2912555108801ad7f918d Gerrit-Change-Number: 81375 Gerrit-PatchSet: 1 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> 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 <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> 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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: soc/intel/xeon_sp/gnr: Add soc_pci_domain_fill_ssdt
by Shuo Liu (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Arthur Heymans, Christian Walter, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, Tim Chu. Hello Jincheng Li, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/81374?usp=email to review the following change. Change subject: soc/intel/xeon_sp/gnr: Add soc_pci_domain_fill_ssdt ...................................................................... soc/intel/xeon_sp/gnr: Add soc_pci_domain_fill_ssdt Domain device objects are created with HID/CID/UID. Change-Id: I28bfdf74d8044235f79f67d832860d8b4306670c Signed-off-by: Shuo Liu <shuo.liu(a)intel.com> Signed-off-by: Jincheng Li <jincheng.li(a)intel.com> --- M src/soc/intel/xeon_sp/gnr/soc_acpi.c 1 file changed, 36 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/81374/1 diff --git a/src/soc/intel/xeon_sp/gnr/soc_acpi.c b/src/soc/intel/xeon_sp/gnr/soc_acpi.c index 9d7c2ab..ab4328a 100644 --- a/src/soc/intel/xeon_sp/gnr/soc_acpi.c +++ b/src/soc/intel/xeon_sp/gnr/soc_acpi.c @@ -1,5 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <acpi/acpigen.h> +#include <acpi/acpigen_pci.h> #include <assert.h> #include <intelblocks/acpi.h> #include <intelblocks/pcr.h> @@ -67,3 +69,37 @@ { return current; } + +void soc_pci_domain_fill_ssdt(const struct device *domain) +{ + const char *name = acpi_device_name(domain); + printk(BIOS_DEBUG, "%s: Creating device %s\n", __func__, name); + + /* + * Scope (\_SB) { + * Device (name) { + */ + acpigen_write_scope("\\_SB"); + acpigen_write_device(name); + + /* + * Name (_HID, EisaId ("PNP0A08")) + * Name (_CID, EisaId ("PNP0A03")) + * Name (_UID, name) + */ + acpigen_write_name("_HID"); + acpigen_emit_eisaid("PNP0A08"); + acpigen_write_name("_CID"); + acpigen_emit_eisaid("PNP0A03"); + acpigen_write_name("_UID"); + acpigen_write_string(name); + + /* + * } //Device + * } //Scope + */ + acpigen_pop_len(); + acpigen_pop_len(); + + return; +} -- To view, visit https://review.coreboot.org/c/coreboot/+/81374?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I28bfdf74d8044235f79f67d832860d8b4306670c Gerrit-Change-Number: 81374 Gerrit-PatchSet: 1 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.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 <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> 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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: acpi: Add soc_pci_domain_fill_ssdt
by Shuo Liu (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Jincheng Li. Hello Jincheng Li, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/81373?usp=email to review the following change. Change subject: acpi: Add soc_pci_domain_fill_ssdt ...................................................................... acpi: Add soc_pci_domain_fill_ssdt soc_pci_domain_fill_ssdt does SoC layer domain SSDT generation, e.g. device object creation and some SoC specific methods. SoC specific generation is placed ahead of generic content generation, so that the device object could be created before being referenced in the generic contents. A default null weak implementation is provided. For platforms with static domain SoC SSDT generation, just use the default weak implementation. For platforms with dyanmic domain SoC SSDT generation, the default method should be overridden. Change-Id: I893eb64c776e78f46737072b475acde5e32a796a Signed-off-by: Shuo Liu <shuo.liu(a)intel.com> Signed-off-by: Jincheng Li <jincheng.li(a)intel.com> --- M src/acpi/acpigen_pci_root_resource_producer.c M src/include/acpi/acpigen_pci.h 2 files changed, 7 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/81373/1 diff --git a/src/acpi/acpigen_pci_root_resource_producer.c b/src/acpi/acpigen_pci_root_resource_producer.c index 78e1a89..5746f9d 100644 --- a/src/acpi/acpigen_pci_root_resource_producer.c +++ b/src/acpi/acpigen_pci_root_resource_producer.c @@ -46,6 +46,10 @@ void pci_domain_fill_ssdt(const struct device *domain) { + /* SoC specific settings, device object creation could be placed here */ + soc_pci_domain_fill_ssdt(domain); + + /* Generic settings */ const char *acpi_scope = acpi_device_path(domain); printk(BIOS_DEBUG, "%s ACPI scope: '%s'\n", __func__, acpi_scope); acpigen_write_scope(acpi_device_path(domain)); @@ -104,3 +108,5 @@ /* Scope */ acpigen_pop_len(); } + +__weak void soc_pci_domain_fill_ssdt(const struct device *domain) {}; diff --git a/src/include/acpi/acpigen_pci.h b/src/include/acpi/acpigen_pci.h index 69216ec..78945bc 100644 --- a/src/include/acpi/acpigen_pci.h +++ b/src/include/acpi/acpigen_pci.h @@ -15,5 +15,6 @@ const char *source_path, unsigned int index); void pci_domain_fill_ssdt(const struct device *domain); +void soc_pci_domain_fill_ssdt(const struct device *domain); #endif /* ACPIGEN_PCI_H */ -- To view, visit https://review.coreboot.org/c/coreboot/+/81373?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I893eb64c776e78f46737072b475acde5e32a796a Gerrit-Change-Number: 81373 Gerrit-PatchSet: 1 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-MessageType: newchange

1 0

[XL] Change in coreboot[main]: soc/intel/xeon_sp: Add GraniteRapids initial codes
by Shuo Liu (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, TangYiwei, Tim Chu. Hello Arthur Heymans, Chen, Gang C, Christian Walter, David Hendricks, Jincheng Li, Johnny Lin, Jonathan Zhang, Lean Sheng Tan, Patrick Rudolph, TangYiwei, Tim Chu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81316?usp=email to look at the new patch set (#20). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: soc/intel/xeon_sp: Add GraniteRapids initial codes ...................................................................... soc/intel/xeon_sp: Add GraniteRapids initial codes coreboot GNR (GraniteRapids) is a FSP2.4 based, no-PCH, single IO-APIC Xeon-SP platform. The same set of codes is also used for SRF (SierraForest) SoC. This patch initially setups the code set. All register definitions are forked from SPR (SapphireRapids) and EBG (Emmisburg PCH)'s codes are reused. Change-Id: I3084e1b5abf25d8d9504bebeaed2a15b916ed56b Signed-off-by: Shuo Liu <shuo.liu(a)intel.com> Signed-off-by: Gang Chen <gang.c.chen(a)intel.com> Signed-off-by: Jincheng Li <jincheng.li(a)intel.com> --- M src/soc/intel/xeon_sp/Makefile.mk M src/soc/intel/xeon_sp/acpi.c M src/soc/intel/xeon_sp/chip_gen1.c A src/soc/intel/xeon_sp/chip_gen6.c A src/soc/intel/xeon_sp/gnr/Kconfig A src/soc/intel/xeon_sp/gnr/Makefile.inc A src/soc/intel/xeon_sp/gnr/acpi/gpe.asl A src/soc/intel/xeon_sp/gnr/acpi/platform.asl A src/soc/intel/xeon_sp/gnr/chip.c A src/soc/intel/xeon_sp/gnr/chip.h A src/soc/intel/xeon_sp/gnr/cpu.c A src/soc/intel/xeon_sp/gnr/include/soc/cpu.h A src/soc/intel/xeon_sp/gnr/include/soc/pci_devs.h A src/soc/intel/xeon_sp/gnr/include/soc/soc_msr.h A src/soc/intel/xeon_sp/gnr/include/soc/soc_util.h A src/soc/intel/xeon_sp/gnr/include/soc/vpd.h A src/soc/intel/xeon_sp/gnr/ramstage.c A src/soc/intel/xeon_sp/gnr/romstage.c A src/soc/intel/xeon_sp/gnr/soc_acpi.c A src/soc/intel/xeon_sp/gnr/soc_util.c M src/soc/intel/xeon_sp/include/soc/chip_common.h M src/soc/intel/xeon_sp/include/soc/fsp_upd.h M src/soc/intel/xeon_sp/lockdown.c M src/soc/intel/xeon_sp/uncore.c 24 files changed, 1,365 insertions(+), 62 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/81316/20 -- To view, visit https://review.coreboot.org/c/coreboot/+/81316?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I3084e1b5abf25d8d9504bebeaed2a15b916ed56b Gerrit-Change-Number: 81316 Gerrit-PatchSet: 20 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.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 <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: TangYiwei 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: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: mb/google/nissa/var/anraggar: Update touchscreen enable pin to GPP_C0
by Qinghong Zeng (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Paul Menzel, Subrata Banik. Qinghong Zeng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81265?usp=email ) Change subject: mb/google/nissa/var/anraggar: Update touchscreen enable pin to GPP_C0 ...................................................................... Patch Set 5: (5 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/81265/comment/afedde26_e6936fba : PS4, Line 7: Assign GPP_C0 as TouchScreen enable pin > Maybe: […] Done https://review.coreboot.org/c/coreboot/+/81265/comment/a18f33ae_66514d67 : PS4, Line 7: TouchScreen > touchscreen Done https://review.coreboot.org/c/coreboot/+/81265/comment/cc7c3136_d080ae17 : PS4, Line 9: effectively : reduce the power consumption > Please add the measurements. Done https://review.coreboot.org/c/coreboot/+/81265/comment/b8a5672f_1dcdfd04 : PS4, Line 10: Suspend > suspend (S0ix?) Yes https://review.coreboot.org/c/coreboot/+/81265/comment/f71c2cff_0d1066a1 : PS4, Line 11: > What is the source of this change? Schematics? Why was GPP_C6 used before? Yes, the schematic has been changed. GPP_C6 was used before for code integrity. GPP_C6 has no actual VDD enable control function. -- To view, visit https://review.coreboot.org/c/coreboot/+/81265?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia06209aa8303be4fc0669c5d6e5d7a06e8e9ab99 Gerrit-Change-Number: 81265 Gerrit-PatchSet: 5 Gerrit-Owner: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(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: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> 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-Comment-Date: Thu, 21 Mar 2024 06:11:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: amdfwtool: Use macro to get the table relative address
by Bao Zheng (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Felix Held, Fred Reitberger, Zheng Bao. Hello Felix Held, Fred Reitberger, Zheng Bao, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/78281?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: amdfwtool: Use macro to get the table relative address ...................................................................... amdfwtool: Use macro to get the table relative address TEST=Identical binary test on all AMD SOC platform Change-Id: Iece4ba65e0476543a8d472168d93801714330dde Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> --- M util/amdfwtool/amdfwtool.c 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/78281/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/78281?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iece4ba65e0476543a8d472168d93801714330dde Gerrit-Change-Number: 78281 Gerrit-PatchSet: 8 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Zheng Bao Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: mb/google/nissa/var/anraggar: Update touchscreen enable pin to GPP_C0
by Qinghong Zeng (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Nick Vaccaro, Qinghong Zeng, Subrata Banik. Hello Dinesh Gehlot, Eric Lai, Kapil Porwal, Nick Vaccaro, Subrata Banik, Weimin Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/81265?usp=email to look at the new patch set (#5). Change subject: mb/google/nissa/var/anraggar: Update touchscreen enable pin to GPP_C0 ...................................................................... mb/google/nissa/var/anraggar: Update touchscreen enable pin to GPP_C0 Assign GPP_C0 and enable only the touchscreen. Before modification, GPP_C0 supplies power to the touchscreen and sensor at the same time. Now the hardware circuit has been modified, GPP_C0 supplies power to the touchscreen alone. After the software is synchronously modified, when the device enters suspend(S0ix), GPP_C0 will not enable VDD, which can reduce the standby power consumption of the touchscreen when it is suspended(S0ix), which is about 2.1mW. BUG=b:304920262 TEST= touchscreen function workable Change-Id: Ia06209aa8303be4fc0669c5d6e5d7a06e8e9ab99 Signed-off-by: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com> --- M src/mainboard/google/brya/variants/anraggar/overridetree.cb 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/81265/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/81265?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia06209aa8303be4fc0669c5d6e5d7a06e8e9ab99 Gerrit-Change-Number: 81265 Gerrit-PatchSet: 5 Gerrit-Owner: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(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: Weimin Wu <wuweimin(a)huaqin.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> 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: Qinghong Zeng <zengqinghong(a)huaqin.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

[L] Change in coreboot[main]: mb/hp: Add Pro 3500 series (Sandy/Ivy Bridge)
by Nicholas Chin (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Joel Linn, Matt DeVillier. Nicholas Chin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81368?usp=email ) Change subject: mb/hp: Add Pro 3500 series (Sandy/Ivy Bridge) ...................................................................... Patch Set 2: (32 comments) Patchset: PS2: Welcome to coreboot! Thanks for the port. We hope to see many more from you. I left a bunch of comments, some of which are probably a bit nitpicky, so don't feel like you need to fix them if you don't want to. A bunch of things are also just the result of autoport not really being maintained and omitting things that are now required on all files, like license identifiers. File Documentation/mainboard/hp/pro_3500_series.md: PS2: Please reflow all lines to 72 columns (though I do see many documentation files that use 80 columns) PS2: This file needs to be added to `Documentation/mainboard/index.html` so that it will actually show up in the generated html https://review.coreboot.org/c/coreboot/+/81368/comment/b3996ab9_dca32968 : PS2, Line 6: Remove trailing space https://review.coreboot.org/c/coreboot/+/81368/comment/86903322_3d84105c : PS2, Line 12: With disabled ME, the SuperIO will not get CPU temperatures via PECI and therefore the automatic fan control will not increase the fan speed. Reflow to 72 characters https://review.coreboot.org/c/coreboot/+/81368/comment/2fe6385e_37f42ff5 : PS2, Line 87: ITE8779e Probably should be either `IT8779E` or `ITE IT8779E` File Documentation/mainboard/hp/pro_3500_series_flash.jpg: PS2: Also, would you mind running the image through an image optimizer website (such as https://jpeg-optimizer.com/)? Images sizes do add up in the repo, and usually there isn't a need for the maximum possible clarity. See https://review.coreboot.org/c/coreboot/+/67818/comment/e9e39fa9_a5451496/ PS2: The red is a bit difficult to read here. By the way, the pinout of this header is already documented on https://doc.coreboot.org/tutorial/flashing_firmware/ext_standalone.html#hp-… (source file `Documentation/tutorial/flashing_firmware/ext_standalone.md`), though this image is still useful for helping locate the flash. File Documentation/mainboard/hp/pro_3500_series_jumper.jpg: PS2: Same here, probably could be reduced in size File src/mainboard/hp/pro_3500_series/Kconfig: PS2: Needs an SPDX license identifier https://review.coreboot.org/c/coreboot/+/81368/comment/584e7498_6a6979d1 : PS2, Line 5: select BOARD_ROMSIZE_KB_8192 : select HAVE_ACPI_RESUME : select HAVE_ACPI_TABLES : select INTEL_INT15 : select MAINBOARD_HAS_LIBGFXINIT : select INTEL_GMA_HAVE_VBT : select MAINBOARD_USES_IFD_GBE_REGION : select NORTHBRIDGE_INTEL_SANDYBRIDGE : select SERIRQ_CONTINUOUS_MODE : select SOUTHBRIDGE_INTEL_BD82X6X : select SUPERIO_ITE_IT8772F : select USE_NATIVE_RAMINIT Please sort these alphabetically https://review.coreboot.org/c/coreboot/+/81368/comment/c5d0110f_62fef45e : PS2, Line 22: string Remove to avoid redefining types, as it is already declared in other shared Kconfig files https://review.coreboot.org/c/coreboot/+/81368/comment/a8dd24a1_3e89bcfb : PS2, Line 26: string Remove https://review.coreboot.org/c/coreboot/+/81368/comment/dadb879d_47271d5a : PS2, Line 30: string Remove https://review.coreboot.org/c/coreboot/+/81368/comment/bd2e9fb7_5be23227 : PS2, Line 34: int Remove https://review.coreboot.org/c/coreboot/+/81368/comment/1cf80794_e10c447f : PS2, Line 38: int Remove File src/mainboard/hp/pro_3500_series/Kconfig.name: PS2: Needs an SPDX license identifier File src/mainboard/hp/pro_3500_series/Makefile.mk: PS2: Needs an SPDX license identifier File src/mainboard/hp/pro_3500_series/acpi/ec.asl: PS2: Should contain ``` /* SPDX-License-Identifier: CC-PDDC */ /* Please update the license if adding licensable material. */ ``` Refer to commit cf4722d317 File src/mainboard/hp/pro_3500_series/devicetree.cb: PS2: Needs an SPDX license identifier https://review.coreboot.org/c/coreboot/+/81368/comment/0a17bcc0_0bbc96b3 : PS2, Line 22: register "gen2_dec" = "0x00000000" : register "gen3_dec" = "0x00000000" : register "gen4_dec" = "0x00000000" Can be removed https://review.coreboot.org/c/coreboot/+/81368/comment/f7504312_f3fbbb71 : PS2, Line 35: # Management Engine Interface 1 The device ref aliases make some of these comments redundant, so they should be removed https://review.coreboot.org/c/coreboot/+/81368/comment/034ddbec_0bff7ebd : PS2, Line 36: subsystemid 0x103c 0x2abf All of these use the same subsystemid, so you can add the line `subsystemid 0x103c 0x2abf inherit` right after the `device domain 0x0 on` line, and all subsequent devices will inherit it. Then all the individual subsystemid lines can be dropped. See `mb/hp/elitebook_820_g1/devicetree.cb` for an example of this. https://review.coreboot.org/c/coreboot/+/81368/comment/321aaddf_632f0a74 : PS2, Line 38: device ref mei2 off # Management Engine Interface 2 : end Typically the `end` is moved to the same line as the `device`, after the on/off, if there isn't anything nested within it to make the devicetree cleaner. https://review.coreboot.org/c/coreboot/+/81368/comment/9e67b35f_cf5a45ba : PS2, Line 150: device ref host_bridge on # Host bridge Host bridge : subsystemid 0x103c 0x2abf : end : device ref peg10 on # PEG : subsystemid 0x103c 0x2abf : end : device ref igd on # iGPU : subsystemid 0x103c 0x2abf : end I guess this just comes down to style and maybe organization, but typically these devices are placed before the southbridge chip entry, right after `device domain 0x0 on`. Just a quirk of how autoport operates. File src/mainboard/hp/pro_3500_series/dsdt.asl: https://review.coreboot.org/c/coreboot/+/81368/comment/9a8fb56c_eb7e7f1e : PS2, Line 3: /* SPDX-License-Identifier: GPL-2.0-only */ The license identifier should be the first line File src/mainboard/hp/pro_3500_series/early_init.c: https://review.coreboot.org/c/coreboot/+/81368/comment/13cc2732_3d30a3f6 : PS2, Line 2: Nit: Remove extra blank line https://review.coreboot.org/c/coreboot/+/81368/comment/edcb8c2a_7a4b7d81 : PS2, Line 33: 0x1400 Could be replaced with defines from `southbridge/intel/bd82x6x/pch.h` (line 140 ish) to more clearly show what's being enabled. https://review.coreboot.org/c/coreboot/+/81368/comment/74aeddc8_04fb8a13 : PS2, Line 33: PCI_DEV(0, 0x1f, 0) Could be replaced with `PCH_LPC_DEV` from `southbridge/intel/bd82x6x/pch.h` https://review.coreboot.org/c/coreboot/+/81368/comment/6f9cb3f5_2f391040 : PS2, Line 34: pci_write_config16(PCI_DEV(0, 0x1f, 0), 0x80, 0x0000); : Could be removed, all zeros is the default File src/superio/ite/common/early_serial.c: PS2: I would split the changes to src/superio/ite/common into a separate patch File src/superio/ite/common/ite.h: PS2: Same as early_serial.c, split into a separate patch -- To view, visit https://review.coreboot.org/c/coreboot/+/81368?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Idf793fe915096cf2553572964faec5c7f8526b9a Gerrit-Change-Number: 81368 Gerrit-PatchSet: 2 Gerrit-Owner: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Joel Linn <jl_coreboot(a)conductive.de> Gerrit-Comment-Date: Thu, 21 Mar 2024 05:56:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: mb/google/brya: Create a tivviks variant
by Eric Lai (Code Review) 21 Mar '24

21 Mar '24

Attention is currently required from: Kapil Porwal, Nick Vaccaro, V Sowmya. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81262?usp=email ) Change subject: mb/google/brya: Create a tivviks variant ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81262?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia833a1dad45e13cd271506ade364b116c5880982 Gerrit-Change-Number: 81262 Gerrit-PatchSet: 3 Gerrit-Owner: V Sowmya <v.sowmya(a)intel.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(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-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: V Sowmya <v.sowmya(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Thu, 21 Mar 2024 05:53:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024