coreboot-gerrit April 2023

coreboot-gerrit@coreboot.org

1 participants
5761 discussions

[M] Change in coreboot[master]: soc/intel/xeon_sp/acpi: Fix _OSC method
by Jonathan Zhang (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Patrick Rudolph, Johnny Lin, Ziang Wang, Christian Walter, Arthur Heymans, Tim Chu. Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74231 ) Change subject: soc/intel/xeon_sp/acpi: Fix _OSC method ...................................................................... Patch Set 2: Code-Review+1 (1 comment) File src/soc/intel/xeon_sp/spr/acpi/cxl_resource.asl: https://review.coreboot.org/c/coreboot/+/74231/comment/ee57bdaf_6ce3ce67 PS2, Line 56: If (Arg0 == ToUUID ("68f2d50b-c469-4d8a-bd3d-941a103fd3fc")) nit: would be good to add back the comment that this GUID is for CXL, as defined in ACPI spec. -- To view, visit https://review.coreboot.org/c/coreboot/+/74231 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ide598e386c30ced24e4f96c37f2b4a609ac33441 Gerrit-Change-Number: 74231 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Ziang Wang <ziang.wang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Ziang Wang <ziang.wang(a)intel.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Wed, 05 Apr 2023 13:45:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/msi/ms7d25/vboot-rwab.fmd: Add 32KiB HSPHY cache region
by Michał Żygowski (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Paul Menzel, Michał Kopeć. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68988 ) Change subject: mb/msi/ms7d25/vboot-rwab.fmd: Add 32KiB HSPHY cache region ...................................................................... Patch Set 8: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/68988/comment/587d6bd0_d2a012eb PS6, Line 7: mainboard > mb Done https://review.coreboot.org/c/coreboot/+/68988/comment/fc5031d9_51d3de03 PS6, Line 7: Add HSPHY cache region > Maybe: Add 32 KB HSPHY cache region Done https://review.coreboot.org/c/coreboot/+/68988/comment/f0ae9a46_2948286c PS6, Line 8: > Why is that region needed? It is explained in the previous patch in relation chain. E.g. ME disabling. -- To view, visit https://review.coreboot.org/c/coreboot/+/68988 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4793fc9457f58e914ef3e18cce1294f230462bd Gerrit-Change-Number: 68988 Gerrit-PatchSet: 8 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Comment-Date: Wed, 05 Apr 2023 13:20:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/msi/ms7d25/vboot-rwab.fmd: Add 32KiB HSPHY cache region
by Michał Żygowski (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Michał Żygowski, Michał Kopeć. Hello build bot (Jenkins), Michał Kopeć, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/68988 to look at the new patch set (#8). Change subject: mb/msi/ms7d25/vboot-rwab.fmd: Add 32KiB HSPHY cache region ...................................................................... mb/msi/ms7d25/vboot-rwab.fmd: Add 32KiB HSPHY cache region Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Change-Id: Ic4793fc9457f58e914ef3e18cce1294f230462bd --- M src/mainboard/msi/ms7d25/vboot-rwab.fmd 1 file changed, 13 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/68988/8 -- To view, visit https://review.coreboot.org/c/coreboot/+/68988 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic4793fc9457f58e914ef3e18cce1294f230462bd Gerrit-Change-Number: 68988 Gerrit-PatchSet: 8 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[master]: drivers/pc80/tpm/tis.c: Use fixed TPM ACPI path
by Michał Żygowski (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Michał Kopeć. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62493 ) Change subject: drivers/pc80/tpm/tis.c: Use fixed TPM ACPI path ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: > On Xeon-Sp the scope \_SB.PCI0 doesn't exist. If hardcoded it should be scope \_SB. […] Ohh, you may be right Patrick. We will check various scenarios when TPM is placed in DSDT -- To view, visit https://review.coreboot.org/c/coreboot/+/62493 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If0b3136e3eb8eb1bb132132a5f3a7034bdd3b424 Gerrit-Change-Number: 62493 Gerrit-PatchSet: 4 Gerrit-Owner: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Patrick Rudolph <siro(a)das-labor.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Michał Kopeć <michal.kopec(a)3mdeb.com> Gerrit-Comment-Date: Wed, 05 Apr 2023 13:01:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Crawford <tcrawford(a)system76.com> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Comment-In-Reply-To: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: drivers/wwan/fm: Use RTD3 root port mutex for reset methods
by Felix Held (Code Review) 05 Apr '23

05 Apr '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/73381 ) Change subject: drivers/wwan/fm: Use RTD3 root port mutex for reset methods ...................................................................... drivers/wwan/fm: Use RTD3 root port mutex for reset methods The RTD3 driver should have its 'use_rp_mutex' set in the devicetree. BRANCH=firmware-brya-14505.B TEST=boot to OS and check the generated SSDT table for the WWAN The RTD3 RPMX mutex should be used in the reset Methods Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Change-Id: Ic3fe20c56b67c2b5177f55f4845610087a30dc7f Reviewed-on: https://review.coreboot.org/c/coreboot/+/73381 Reviewed-by: Jérémy Compostella <jeremy.compostella(a)intel.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Bora Guvendik <bora.guvendik(a)intel.com> --- M src/drivers/wwan/fm/acpi_fm350gl.c 1 file changed, 84 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Bora Guvendik: Looks good to me, approved Jérémy Compostella: Looks good to me, but someone else must approve diff --git a/src/drivers/wwan/fm/acpi_fm350gl.c b/src/drivers/wwan/fm/acpi_fm350gl.c index df91389..46f8ecb 100644 --- a/src/drivers/wwan/fm/acpi_fm350gl.c +++ b/src/drivers/wwan/fm/acpi_fm350gl.c @@ -3,6 +3,7 @@ #include <acpi/acpigen.h> #include <acpi/acpi_device.h> #include "chip.h" +#include "soc/intel/common/block/include/intelblocks/acpi.h" #include "soc/intel/common/block/pcie/rtd3/chip.h" /* FCPO# to RESET# delay time during WWAN ON */ @@ -56,6 +57,17 @@ { acpigen_write_method_serialized("FHRF", 1); { + char mutex_path[128]; + const struct soc_intel_common_block_pcie_rtd3_config *rtd3_config; + + rtd3_config = config_of(config->rtd3dev); + if (rtd3_config->use_rp_mutex) { + snprintf(mutex_path, sizeof(mutex_path), acpi_device_path_join(parent_dev, + RP_MUTEX_NAME)); + /* Acquire root port mutex in case FHRF is called directly and not called from _RST */ + acpigen_write_acquire(mutex_path, ACPI_MUTEX_NO_TIMEOUT); + } + /* LOCAL0 = PERST# */ acpigen_get_tx_gpio(&config->perst_gpio); acpigen_write_if_lequal_op_int(LOCAL0_OP, 0); @@ -97,6 +109,9 @@ acpigen_write_if_end(); /* If */ } acpigen_pop_len(); /* Else */ + + if (rtd3_config->use_rp_mutex) + acpigen_write_release(mutex_path); } acpigen_write_method_end(); /* Method */ } @@ -109,6 +124,17 @@ { acpigen_write_method_serialized("SHRF", 0); { + char mutex_path[128]; + const struct soc_intel_common_block_pcie_rtd3_config *rtd3_config; + + rtd3_config = config_of(config->rtd3dev); + if (rtd3_config->use_rp_mutex) { + snprintf(mutex_path, sizeof(mutex_path), acpi_device_path_join(parent_dev, + RP_MUTEX_NAME)); + /* Acquire root port mutex */ + acpigen_write_acquire(mutex_path, ACPI_MUTEX_NO_TIMEOUT); + } + /* call rtd3 method to Disable ModPHY Power Gating. */ if (wwan_fm350gl_get_rtd3_method_support(config) & ACPI_PCIE_RP_EMIT_PSD0) { @@ -137,6 +163,9 @@ "L23D")); } acpigen_write_sleep(FM350GL_TIME_HW_INIT); + + if (rtd3_config->use_rp_mutex) + acpigen_write_release(mutex_path); } acpigen_write_method_end(); /* Method */ } @@ -150,6 +179,17 @@ { acpigen_write_method_serialized("_RST", 0); { + char mutex_path[128]; + const struct soc_intel_common_block_pcie_rtd3_config *rtd3_config; + + rtd3_config = config_of(config->rtd3dev); + if (rtd3_config->use_rp_mutex) { + snprintf(mutex_path, sizeof(mutex_path), acpi_device_path_join(parent_dev, + RP_MUTEX_NAME)); + /* Acquire root port mutex */ + acpigen_write_acquire(mutex_path, ACPI_MUTEX_NO_TIMEOUT); + } + /* Perform 1st Half of FLDR Flow for soft reset: FHRF(0) */ acpigen_emit_namestring("FHRF"); acpigen_emit_byte(RESET_TYPE_WARM); @@ -158,6 +198,9 @@ /* Indicates that the following _Off will be skipped. */ acpigen_emit_byte(INCREMENT_OP); acpigen_emit_namestring(acpi_device_path_join(parent_dev, "RTD3.OFSK")); + + if (rtd3_config->use_rp_mutex) + acpigen_write_release(mutex_path); } acpigen_write_method_end(); /* Method */ } @@ -171,6 +214,17 @@ { acpigen_write_method_serialized("_RST", 0); { + char mutex_path[128]; + const struct soc_intel_common_block_pcie_rtd3_config *rtd3_config; + + rtd3_config = config_of(config->rtd3dev); + if (rtd3_config->use_rp_mutex) { + snprintf(mutex_path, sizeof(mutex_path), acpi_device_path_join(parent_dev, + RP_MUTEX_NAME)); + /* Acquire root port mutex */ + acpigen_write_acquire(mutex_path, ACPI_MUTEX_NO_TIMEOUT); + } + /* Perform 1st Half of FLDR Flow for cold reset: FHRF (1) */ acpigen_emit_namestring("FHRF"); acpigen_emit_byte(RESET_TYPE_COLD); @@ -180,6 +234,9 @@ driver removal */ acpigen_emit_byte(INCREMENT_OP); acpigen_emit_namestring(acpi_device_path_join(parent_dev, "RTD3.OFSK")); + + if (rtd3_config->use_rp_mutex) + acpigen_write_release(mutex_path); } acpigen_write_method_end(); /* Method */ } @@ -211,6 +268,7 @@ const struct drivers_wwan_fm_config *config = config_of(dev); const struct device *parent = dev->bus->dev; const char *scope = acpi_device_path(parent); + const struct soc_intel_common_block_pcie_rtd3_config *rtd3_config; if (!is_dev_enabled(parent)) { printk(BIOS_ERR, "%s: root port not enabled\n", __func__); @@ -226,6 +284,12 @@ __func__, scope); return; } + + rtd3_config = config_of(config->rtd3dev); + if (!rtd3_config->use_rp_mutex) + printk(BIOS_WARNING, "%s: RTD3 must use root port mutex.\n", + __func__); + printk(BIOS_INFO, "%s: Enable WWAN for %s (%s)\n", scope, dev_path(parent), config->desc ?: dev->chip_ops->name); acpigen_write_scope(scope); -- To view, visit https://review.coreboot.org/c/coreboot/+/73381 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic3fe20c56b67c2b5177f55f4845610087a30dc7f Gerrit-Change-Number: 73381 Gerrit-PatchSet: 5 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.corp-partner.google.com> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: soc/intel/common/block/pcie/rtd3: Add root port mutex support
by Felix Held (Code Review) 05 Apr '23

05 Apr '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/73380 ) Change subject: soc/intel/common/block/pcie/rtd3: Add root port mutex support ...................................................................... soc/intel/common/block/pcie/rtd3: Add root port mutex support When 'use_rp_mutex' (default = 0) is set in the device tree, a root port mutex will be added. This mutex is used in _ON and _OFF method, where the GPIO reset and/or enable GPIO value is changed. The companion driver, such as WWAN driver, needs to acquire this root port mutex when accessing the same GPIO pins. Using this common mutex prevents those invoked methods from being called from different thread while one is not completed. An example is that WWAN driver calling _RST method to reset the device and does remove/rescan for the device while the pm runtime work might call RTD3 _OFF. For those root port without additional driver, this mutex is not needed. BRANCH=firmware-brya-14505.B TEST=boot to OS and check the generated SSDT table for the root port. The RPMX mutex should be generated and _ON and _OFF should use this mutex. Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Change-Id: Ibc077528692b2d7076132384fb7bd441be502511 Reviewed-on: https://review.coreboot.org/c/coreboot/+/73380 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Jérémy Compostella <jeremy.compostella(a)intel.com> Reviewed-by: Bora Guvendik <bora.guvendik(a)intel.com> --- M src/soc/intel/common/block/pcie/rtd3/chip.h M src/soc/intel/common/block/pcie/rtd3/rtd3.c 2 files changed, 65 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Bora Guvendik: Looks good to me, approved Jérémy Compostella: Looks good to me, but someone else must approve diff --git a/src/soc/intel/common/block/pcie/rtd3/chip.h b/src/soc/intel/common/block/pcie/rtd3/chip.h index ff22adc..f247028 100644 --- a/src/soc/intel/common/block/pcie/rtd3/chip.h +++ b/src/soc/intel/common/block/pcie/rtd3/chip.h @@ -5,6 +5,8 @@ #include <acpi/acpi_device.h> +#define RP_MUTEX_NAME "RPMX" + enum acpi_pcie_rp_pm_emit { ACPI_PCIE_RP_EMIT_NONE = 0x00, /* None */ ACPI_PCIE_RP_EMIT_L23 = 0x01, /* L23 */ @@ -86,6 +88,13 @@ * the device driver. */ bool skip_on_off_support; + + /* + * Indicates the root port mutex is used for _ON and _OFF, the companion device driver + * such as WWAN driver should also acquire this mutex in methods that access the same HW + * resource, such as PERST# GPIO pin. + */ + bool use_rp_mutex; }; #endif /* __SOC_INTEL_COMMON_BLOCK_PCIE_RTD3_CHIP_H__ */ diff --git a/src/soc/intel/common/block/pcie/rtd3/rtd3.c b/src/soc/intel/common/block/pcie/rtd3/rtd3.c index 1519e6f..8b372ed 100644 --- a/src/soc/intel/common/block/pcie/rtd3/rtd3.c +++ b/src/soc/intel/common/block/pcie/rtd3/rtd3.c @@ -154,6 +154,10 @@ acpigen_write_return_op(ONE_OP); acpigen_write_if_end(); + if (config->use_rp_mutex) + acpigen_write_acquire(acpi_device_path_join(parent, RP_MUTEX_NAME), + ACPI_MUTEX_NO_TIMEOUT); + /* Disable modPHY power gating for PCH RPs. */ if (rp_type == PCIE_RP_PCH) pcie_rtd3_enable_modphy_pg(pcie_rp, PG_DISABLE); @@ -180,6 +184,9 @@ if (!config->disable_l23) pcie_rtd3_acpi_l23_exit(); + if (config->use_rp_mutex) + acpigen_write_release(acpi_device_path_join(parent, RP_MUTEX_NAME)); + if (config->skip_on_off_support) { /* If current _ON is skipped, ONSK is decremented so that _ON will be * executed normally until _OFF is skipped again. @@ -196,8 +203,12 @@ static void pcie_rtd3_acpi_method_off(int pcie_rp, const struct soc_intel_common_block_pcie_rtd3_config *config, - enum pcie_rp_type rp_type) + enum pcie_rp_type rp_type, + const struct device *dev) + { + const struct device *parent = dev->bus->dev; + acpigen_write_method_serialized("_OFF", 0); /* When this feature is enabled, ONSK is checked to see if the device @@ -208,6 +219,10 @@ if (config->skip_on_off_support) acpigen_write_if_lequal_namestr_int("OFSK", 0); + if (config->use_rp_mutex) + acpigen_write_acquire(acpi_device_path_join(parent, RP_MUTEX_NAME), + ACPI_MUTEX_NO_TIMEOUT); + /* Trigger L23 ready entry flow unless disabled by config. */ if (!config->disable_l23) pcie_rtd3_acpi_l23_entry(); @@ -234,6 +249,9 @@ acpigen_write_sleep(config->enable_off_delay_ms); } + if (config->use_rp_mutex) + acpigen_write_release(acpi_device_path_join(parent, RP_MUTEX_NAME)); + if (config->skip_on_off_support) { /* If current _OFF is skipped, ONSK is incremented so that the * following _ON will also be skipped. In addition, OFSK is decremented @@ -422,6 +440,9 @@ /* The RTD3 power resource is added to the root port, not the device. */ acpigen_write_scope(scope); + if (config->use_rp_mutex) + acpigen_write_mutex(RP_MUTEX_NAME, 0); + if (config->desc) acpigen_write_name_string("_DDN", config->desc); @@ -464,7 +485,7 @@ pcie_rtd3_acpi_method_status(config); pcie_rtd3_acpi_method_on(pcie_rp, config, rp_type, dev); - pcie_rtd3_acpi_method_off(pcie_rp, config, rp_type); + pcie_rtd3_acpi_method_off(pcie_rp, config, rp_type, dev); acpigen_pop_len(); /* PowerResource */ /* Indicate to the OS that device supports hotplug in D3. */ -- To view, visit https://review.coreboot.org/c/coreboot/+/73380 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibc077528692b2d7076132384fb7bd441be502511 Gerrit-Change-Number: 73380 Gerrit-PatchSet: 3 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: soc/intel/cmn/cse: Fix unnecessary End-Of-Post cases
by Subrata Banik (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Tarun Tuli, Jérémy Compostella. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74205 ) Change subject: soc/intel/cmn/cse: Fix unnecessary End-Of-Post cases ...................................................................... Patch Set 3: (1 comment) File src/soc/intel/common/block/cse/cse_eop.c: https://review.coreboot.org/c/coreboot/+/74205/comment/46e592ea_860f6f37 PS3, Line 295: do_send_end_of_post(true); > ``` > /* Don't send EOP if below conditions being met > * 1. For a CSE-Lite SKU > * 2. if the CSE is running RO FW > * 3. The board is running vboot in recovery mode > * The expectation is the CSE is in SOFT TEMP DISABLE state, hence, need to > * skip sending EOP cmd to CSE > */ > static bool is_cse_eop_supported(void) > { > if (CONFIG(SOC_INTEL_CSE_LITE_SKU) && vboot_recovery_mode_enabled() && > cse_is_hfs1_com_soft_temp_disable()) { > printk(BIOS_INFO, "HECI: coreboot in recovery mode; found CSE in expected SOFT " > "TEMP DISABLE state, skipping EOP\n"); > return false; > } > return true; > } > ``` > > ``` > static void send_cse_eop_with_late_finalize(void *unused) > { > if (is_cse_eop_supported()) > do_send_end_of_post(true); > if (CONFIG(SOC_INTEL_CSE_SEND_EOP_LATE) || > CONFIG(SOC_INTEL_CSE_SEND_EOP_ASYNC)) > cse_late_finalize(); > } > ``` relanded the original CL with proposed changes integrated into it https://review.coreboot.org/c/coreboot/+/74214/2/src/soc/intel/common/block… -- To view, visit https://review.coreboot.org/c/coreboot/+/74205 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id2d23df668ab698213e8b1af652ef9b90ccad046 Gerrit-Change-Number: 74205 Gerrit-PatchSet: 3 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Comment-Date: Wed, 05 Apr 2023 12:41:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/skyrim: Remove unused sleep GPIO table
by Felix Held (Code Review) 05 Apr '23

05 Apr '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/73852 ) Change subject: mb/google/skyrim: Remove unused sleep GPIO table ...................................................................... mb/google/skyrim: Remove unused sleep GPIO table On Skyrim, there isn't a need for a sleep GPIO table. Remove the TODO and filler table and function to reduce unnecessary function overhead. BUG=None BRANCH=Skyrim TEST=Build Skyrim BIOS image. Change-Id: Ia9d55a5e2295bb2e2c2957c4f5207362f616022c Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/73852 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Jon Murphy <jpmurphy(a)google.com> --- M src/mainboard/google/skyrim/variants/baseboard/gpio.c M src/mainboard/google/skyrim/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/skyrim/variants/baseboard/smihandler.c 3 files changed, 20 insertions(+), 20 deletions(-) Approvals: build bot (Jenkins): Verified Jon Murphy: Looks good to me, approved diff --git a/src/mainboard/google/skyrim/variants/baseboard/gpio.c b/src/mainboard/google/skyrim/variants/baseboard/gpio.c index 932acaa..969a995 100644 --- a/src/mainboard/google/skyrim/variants/baseboard/gpio.c +++ b/src/mainboard/google/skyrim/variants/baseboard/gpio.c @@ -168,11 +168,6 @@ PAD_INT(GPIO_18, PULL_NONE, EDGE_LOW, STATUS_DELIVERY), }; -/* GPIO configuration for sleep */ -static const struct soc_amd_gpio sleep_gpio_table[] = { - /* TODO: Fill sleep gpio configuration */ -}; - /* GPIO configuration in bootblock */ static const struct soc_amd_gpio bootblock_gpio_table[] = { /* Enable WLAN */ @@ -252,12 +247,6 @@ *gpio = NULL; } -__weak void variant_sleep_gpio_table(const struct soc_amd_gpio **gpio, size_t *size) -{ - *size = ARRAY_SIZE(sleep_gpio_table); - *gpio = sleep_gpio_table; -} - __weak void variant_espi_gpio_table(const struct soc_amd_gpio **gpio, size_t *size) { *size = ARRAY_SIZE(espi_gpio_table); diff --git a/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/variants.h b/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/variants.h index a340530..e49b7c4 100644 --- a/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/variants.h +++ b/src/mainboard/google/skyrim/variants/baseboard/include/baseboard/variants.h @@ -29,9 +29,6 @@ /* This function allows variant to override any early GPIO init in early bootblock or psp. */ void variant_early_override_gpio_table(const struct soc_amd_gpio **gpio, size_t *size); -/* This function provides GPIO settings before entering sleep. */ -void variant_sleep_gpio_table(const struct soc_amd_gpio **gpio, size_t *size); - /* This function provides GPIO settings for eSPI bus. */ void variant_espi_gpio_table(const struct soc_amd_gpio **gpio, size_t *size); diff --git a/src/mainboard/google/skyrim/variants/baseboard/smihandler.c b/src/mainboard/google/skyrim/variants/baseboard/smihandler.c index 0243747..ff59b2b 100644 --- a/src/mainboard/google/skyrim/variants/baseboard/smihandler.c +++ b/src/mainboard/google/skyrim/variants/baseboard/smihandler.c @@ -11,13 +11,7 @@ void mainboard_smi_sleep(u8 slp_typ) { - size_t num_gpios; - const struct soc_amd_gpio *gpios; - chromeec_smi_sleep(slp_typ, MAINBOARD_EC_S3_WAKE_EVENTS, MAINBOARD_EC_S5_WAKE_EVENTS); - - variant_sleep_gpio_table(&gpios, &num_gpios); - gpio_configure_pads(gpios, num_gpios); } int mainboard_smi_apmc(u8 apmc) -- To view, visit https://review.coreboot.org/c/coreboot/+/73852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia9d55a5e2295bb2e2c2957c4f5207362f616022c Gerrit-Change-Number: 73852 Gerrit-PatchSet: 2 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: mb/google/skyrim: Remove BT power sequencing TODO comment
by Felix Held (Code Review) 05 Apr '23

05 Apr '23

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/73851 ) Change subject: mb/google/skyrim: Remove BT power sequencing TODO comment ...................................................................... mb/google/skyrim: Remove BT power sequencing TODO comment In the first wave of designs, there is only one Bluetooth vendor. Hence there is no need to handle different power sequence requirements. BUG=None BRANCH=Skyrim TEST=Build Skyrim BIOS image. Change-Id: Id477dd64de6523d066aa1a5e498381e0bf4b2cc1 Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com> --- M src/mainboard/google/skyrim/variants/baseboard/devicetree.cb 1 file changed, 17 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved diff --git a/src/mainboard/google/skyrim/variants/baseboard/devicetree.cb b/src/mainboard/google/skyrim/variants/baseboard/devicetree.cb index 6892742..61d22f6 100644 --- a/src/mainboard/google/skyrim/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/skyrim/variants/baseboard/devicetree.cb @@ -174,7 +174,6 @@ register "type" = "UPC_TYPE_INTERNAL" register "has_power_resource" = "true" register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPIO_10)" - # TODO(b/263161283): Confirm the delay meets the requirement of all BT controllers register "enable_delay_ms" = "500" register "enable_off_delay_ms" = "200" register "use_gpio_for_status" = "true" -- To view, visit https://review.coreboot.org/c/coreboot/+/73851 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id477dd64de6523d066aa1a5e498381e0bf4b2cc1 Gerrit-Change-Number: 73851 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: mb/google/skyrim: Remove BT power sequencing TODO comment
by Felix Held (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Jason Nien, Jon Murphy, Martin Roth, Karthik Ramasubramanian. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73851 ) Change subject: mb/google/skyrim: Remove BT power sequencing TODO comment ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/73851 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id477dd64de6523d066aa1a5e498381e0bf4b2cc1 Gerrit-Change-Number: 73851 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 05 Apr 2023 12:40:55 +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 April 2023