coreboot-gerrit March 2022

coreboot-gerrit@coreboot.org

1 participants
6700 discussions

Change in coreboot[master]: soc/intel/alderlake: Define Kconfigs for Descriptor Region
by Tim Wawrzynczak (Code Review) 04 Mar '22

04 Mar '22

Attention is currently required from: Maulik V Vaghela, Sridhar Siricilla, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62263 ) Change subject: soc/intel/alderlake: Define Kconfigs for Descriptor Region ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/62263 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia3481acefbda885617607675aef2afbb81c21c77 Gerrit-Change-Number: 62263 Gerrit-PatchSet: 4 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 04 Mar 2022 16:04:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Add support to control coreboot and Intel SoC features
by Tim Wawrzynczak (Code Review) 04 Mar '22

04 Mar '22

Attention is currently required from: Nico Huber, Subrata Banik, Sridhar Siricilla, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61380 ) Change subject: soc/intel/common: Add support to control coreboot and Intel SoC features ...................................................................... Patch Set 10: (5 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/61380/comment/0263ff91_4096625e PS10, Line 13: rebuilding the coreboot. `rebuilding coreboot.` https://review.coreboot.org/c/coreboot/+/61380/comment/a04b6edc_3d81128c PS10, Line 17: The OEM section starts from offset:0xf00 till end of the Descriptor : Region(0xfff). Either bring up to the previous line or add a blank line between paragraphs File src/soc/intel/common/block/cse/cse_lite.c: https://review.coreboot.org/c/coreboot/+/61380/comment/ff2759a2_cc32cae7 PS10, Line 668: if (CONFIG(SOC_INTEL_COMMON_BLOCK_DEBUG_FEATURE) && CONFIG(SOC_INTEL_CSE_RW_UPDATE)) : return !is_debug_cse_fw_update_disable(); : : return CONFIG(SOC_INTEL_CSE_RW_UPDATE); suggestion, does this make the logic a little more clear? ``` if (!CONFIG(SOC_INTEL_CSE_RW_UPDATE)) return false; if (CONFIG(SOC_INTEL_COMMON_BLOCK_DEBUG_FEATURE) return !is_debug_cse_fw_update_disable(); return true; ``` File src/soc/intel/common/block/debug/debug_feature.c: https://review.coreboot.org/c/coreboot/+/61380/comment/2c16aa79_8a2c358c PS10, Line 14: /* Byte location: 0xF00 */ nit: this comment seems redundant, you have a well-named #define for this https://review.coreboot.org/c/coreboot/+/61380/comment/6910d3c8_1ec5b1bb PS10, Line 28: pre_mem_debug.cse_fw_update_disable == 1; It seems the default value for the OEM section is 0xFFFF_FFFF, so if I am thinking correctly, on a normal image, as soon as you enable the DEBUG_FEATURE Kconfig, the CSE RW update will be disabled? As in there is extra work required (go and set that bit in the descriptor) to enable CSE RW update once you enable the Kconfig? -- To view, visit https://review.coreboot.org/c/coreboot/+/61380 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5ba40926bd9ad909654f152e48cdd648b28afd62 Gerrit-Change-Number: 61380 Gerrit-PatchSet: 10 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 04 Mar 2022 16:04:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: libpayload: cbmem_console: Drop loglevel markers from snapshot
by Raul Rangel (Code Review) 04 Mar '22

04 Mar '22

Raul Rangel has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62506 ) Change subject: libpayload: cbmem_console: Drop loglevel markers from snapshot ...................................................................... libpayload: cbmem_console: Drop loglevel markers from snapshot coreboot recently introduced non-printable loglevel markers in the CBMEM console. Payloads were generally unaffected since they don't use log levels and it is still legal to append lines without a marker to the log. However, payloads using cbmem_console_snapshot() to display existing logs from coreboot have started seeing '?' characters in place of the markers. This patch fixes the issue by filtering out marker characters. BUG=b:221909874 Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I4a9e5d464508320cf43ea572d62896d38c2a128d Reviewed-on: https://review.coreboot.org/c/coreboot/+/62506 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M payloads/libpayload/drivers/cbmem_console.c 1 file changed, 24 insertions(+), 12 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/payloads/libpayload/drivers/cbmem_console.c b/payloads/libpayload/drivers/cbmem_console.c index 22d5312..df40ad0 100644 --- a/payloads/libpayload/drivers/cbmem_console.c +++ b/payloads/libpayload/drivers/cbmem_console.c @@ -80,11 +80,28 @@ do_write(buffer, count); } +static void snapshot_putc(char *console, uint32_t *cursor, char c) +{ + /* This is BIOS_LOG_IS_MARKER() from coreboot. Due to stupid + licensing restrictions, we can't use it directly. */ + if (c >= 0x10 && c <= 0x18) + return; + + /* Slight memory corruption may occur between reboots and give us a few + unprintable characters like '\0'. Replace them with '?' on output. */ + if (!isprint(c) && !isspace(c)) + console[*cursor] = '?'; + else + console[*cursor] = c; + + *cursor += 1; +} + char *cbmem_console_snapshot(void) { const struct cbmem_console *const console_p = phys_to_virt(cbmem_console_p); char *console_c; - uint32_t size, cursor, overflow; + uint32_t size, cursor, overflow, newc, oldc; if (!console_p) { printf("ERROR: No cbmem console found in coreboot table\n"); @@ -104,24 +121,19 @@ size); return NULL; } - console_c[size] = '\0'; + newc = 0; if (overflow) { if (cursor >= size) { printf("ERROR: CBMEM console struct is corrupted\n"); return NULL; } - memcpy(console_c, console_p->body + cursor, size - cursor); - memcpy(console_c + size - cursor, console_p->body, cursor); - } else { - memcpy(console_c, console_p->body, size); + for (oldc = cursor; oldc < size; oldc++) + snapshot_putc(console_c, &newc, console_p->body[oldc]); } - - /* Slight memory corruption may occur between reboots and give us a few - unprintable characters like '\0'. Replace them with '?' on output. */ - for (cursor = 0; cursor < size; cursor++) - if (!isprint(console_c[cursor]) && !isspace(console_c[cursor])) - console_c[cursor] = '?'; + for (oldc = 0; oldc < size && oldc < cursor; oldc++) + snapshot_putc(console_c, &newc, console_p->body[oldc]); + console_c[newc] = '\0'; return console_c; } -- To view, visit https://review.coreboot.org/c/coreboot/+/62506 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4a9e5d464508320cf43ea572d62896d38c2a128d Gerrit-Change-Number: 62506 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> 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-MessageType: merged

1 0

Change in coreboot[master]: [UNTESTED] soc/amd/common/block/lpc/lpc: rework resource index handling
by Raul Rangel (Code Review) 04 Mar '22

04 Mar '22

Attention is currently required from: Jason Glenesk, Marshall Dawson, Fred Reitberger, Felix Held. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62579 ) Change subject: [UNTESTED] soc/amd/common/block/lpc/lpc: rework resource index handling ...................................................................... Patch Set 1: (4 comments) File src/soc/amd/common/block/lpc/lpc.c: https://review.coreboot.org/c/coreboot/+/62579/comment/ad833f27_45528ec6 PS1, Line 96: IOINDEX_SUBTRACTIVE lol.... what! https://review.coreboot.org/c/coreboot/+/62579/comment/968f96af_c15c1944 PS1, Line 102: IORESOURCE_SUBTRACTIVE I don't understand why this is subtractive? https://review.coreboot.org/c/coreboot/+/62579/comment/51b2e091_15d40b57 PS1, Line 114: IORESOURCE_SUBTRACTIVE Same with this? https://review.coreboot.org/c/coreboot/+/62579/comment/2f6432de_bcd021c0 PS1, Line 130: STAY Who disables them? -- To view, visit https://review.coreboot.org/c/coreboot/+/62579 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I38b2845c982e01f263e70e9639dc2d8a3a7f6925 Gerrit-Change-Number: 62579 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 04 Mar 2022 15:56:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/amd/chausie/devicetree: enable GFX HDA, ACP and XHCI2 devices
by Fred Reitberger (Code Review) 04 Mar '22

04 Mar '22

Attention is currently required from: Jason Glenesk, Marshall Dawson, Felix Held. Fred Reitberger has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62571 ) Change subject: mb/amd/chausie/devicetree: enable GFX HDA, ACP and XHCI2 devices ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/62571 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I186797a832470eb17752e06aa2fcc0b5c9db0398 Gerrit-Change-Number: 62571 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 04 Mar 2022 15:52:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/*/lpc: rename SPIROM_BASE_ADDRESS_REGISTER
by Raul Rangel (Code Review) 04 Mar '22

04 Mar '22

Attention is currently required from: Jason Glenesk, Marshall Dawson, Fred Reitberger, Felix Held. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62578 ) Change subject: soc/amd/*/lpc: rename SPIROM_BASE_ADDRESS_REGISTER ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/62578 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd9f98bd01b1c7197b80d642a45657c97f708bcd Gerrit-Change-Number: 62578 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 04 Mar 2022 15:50:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Revert "mb/google/brya/var/taeko: Fix PLD group order (W/A)"
by Tim Wawrzynczak (Code Review) 04 Mar '22

04 Mar '22

Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62565 ) Change subject: Revert "mb/google/brya/var/taeko: Fix PLD group order (W/A)" ...................................................................... Revert "mb/google/brya/var/taeko: Fix PLD group order (W/A)" This revert commit acb17fec34a609c5b674ad0d2af04d47800530e2. This issue was fixed in the OS, therefore the workaround can be reverted. BUG=b:210497855 BRANCH=firmware-brya-14505.B TEST=build coreboot and boot into OS. Signed-off-by: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com> Change-Id: Ic836e0cf53c2f9d30bd12851be285d864b2256b8 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62565 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> --- M src/mainboard/google/brya/variants/taeko/overridetree.cb 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved diff --git a/src/mainboard/google/brya/variants/taeko/overridetree.cb b/src/mainboard/google/brya/variants/taeko/overridetree.cb index 760ffa6..ac83e58 100644 --- a/src/mainboard/google/brya/variants/taeko/overridetree.cb +++ b/src/mainboard/google/brya/variants/taeko/overridetree.cb @@ -466,7 +466,7 @@ register "desc" = ""USB3 Type-C Port C1 (DB)"" register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" register "use_custom_pld" = "true" - register "custom_pld" = "ACPI_PLD_TYPE_C(RIGHT, LEFT, ACPI_PLD_GROUP(2, 1))" + register "custom_pld" = "ACPI_PLD_TYPE_C(RIGHT, LEFT, ACPI_PLD_GROUP(3, 1))" device ref tcss_usb3_port3 on probe DB_USB DB_USB3_NO_A probe DB_USB DB_USB3_1C_1A @@ -489,7 +489,7 @@ register "desc" = ""USB2 Type-C Port C1 (DB)"" register "type" = "UPC_TYPE_C_USB2_SS_SWITCH" register "use_custom_pld" = "true" - register "custom_pld" = "ACPI_PLD_TYPE_C(RIGHT, LEFT, ACPI_PLD_GROUP(2, 1))" + register "custom_pld" = "ACPI_PLD_TYPE_C(RIGHT, LEFT, ACPI_PLD_GROUP(3, 1))" device ref usb2_port3 on probe DB_USB DB_USB3_NO_A probe DB_USB DB_USB3_1C_1A 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/coreboot/+/62565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic836e0cf53c2f9d30bd12851be285d864b2256b8 Gerrit-Change-Number: 62565 Gerrit-PatchSet: 4 Gerrit-Owner: Kevin Chang <kevin.chang(a)lcfc.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: YH Lin <yueherngl(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Joey Peng <joey.peng(a)lcfc.corp-partner.google.com> Gerrit-CC: Melo Chuang <melo.chuang(a)lcfc.corp-partner.google.com> Gerrit-CC: Rasheed Hsueh <rasheed.hsueh(a)lcfc.corp-partner.google.com> Gerrit-CC: Sunshine Chao <sunshine.chao(a)lcfc.corp-partner.google.com> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: src/driver/wifi: Add _DSM method for DDRRFIM
by Tim Wawrzynczak (Code Review) 04 Mar '22

04 Mar '22

Attention is currently required from: Varshit B Pandya, Subrata Banik, Maulik V Vaghela, Paul Menzel, Sugnan Prabhu S, Tim Wawrzynczak. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61020 ) Change subject: src/driver/wifi: Add _DSM method for DDRRFIM ...................................................................... Patch Set 18: (1 comment) File src/drivers/wifi/generic/acpi.c: https://review.coreboot.org/c/coreboot/+/61020/comment/fefc8519_45f26bc9 PS18, Line 606: if (*is_cnviDdrRfim_enabled) { : dsm_ids[dsm_count].uuid = ACPI_DSM_RFIM_WIFI_UUID; : dsm_ids[dsm_count].callbacks = &wifi_dsm2_callbacks[0]; : dsm_ids[dsm_count].count = ARRAY_SIZE(wifi_dsm2_callbacks); : dsm_ids[dsm_count].arg = (void *)is_cnviDdrRfim_enabled; : dsm_count++; : } Also taking another look at this, would we ever add the DSM subfunction for CNVi DDR RFIM enabled" that would return 0, not enabled? This code effectively would never do that, because the if(is_cnvi_ddr_rfim_enabled) skips writing out the DSM sub function is false, so in theory wifi_dsm_ddrrfim_func3_cb could just be acpigen_write_return_integer(1); No big deal though -- To view, visit https://review.coreboot.org/c/coreboot/+/61020 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I217b736df3d4224a6732d1941a160abcddbd8f37 Gerrit-Change-Number: 61020 Gerrit-PatchSet: 18 Gerrit-Owner: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-CC: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Fri, 04 Mar 2022 15:46:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: src/driver/wifi: Add _DSM method for DDRRFIM
by Tim Wawrzynczak (Code Review) 04 Mar '22

04 Mar '22

Attention is currently required from: Varshit B Pandya, Subrata Banik, Maulik V Vaghela, Paul Menzel, Sugnan Prabhu S, Tim Wawrzynczak. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61020 ) Change subject: src/driver/wifi: Add _DSM method for DDRRFIM ...................................................................... Patch Set 18: (5 comments) File src/drivers/wifi/generic/acpi.c: https://review.coreboot.org/c/coreboot/+/61020/comment/58602f18_f91dfb56 PS18, Line 40: //bool is_cnviDdrRfim_enabled = false; delete this https://review.coreboot.org/c/coreboot/+/61020/comment/1d0c0c5b_b31d2520 PS18, Line 160: bool *is_cnvi_ddr_rfim_enabled = false; This is creating a pointer that points to the value `false`, presumably 0... do you mean `bool *is_cnvi_ddr_rfim_enabled = NULL;` ? https://review.coreboot.org/c/coreboot/+/61020/comment/4341cb63_87dfadb3 PS18, Line 542: bool *is_cnviDdrRfim_enabled = false; : *is_cnviDdrRfim_enabled = config->enable_cnvi_ddr_rfim; This would look cleaner if we just use a regular bool here, e.g. ``` bool is_cnvi_ddr_rfim_enabled; is_cnvi_ddr_rfim_enabled = config->enable_cnvi_ddr_rfim; ``` https://review.coreboot.org/c/coreboot/+/61020/comment/950e368a_58ca4644 PS18, Line 606: if (*is_cnviDdrRfim_enabled) then `if (is_cnvi_ddr_rfim_enabled)` https://review.coreboot.org/c/coreboot/+/61020/comment/d13715d4_9ebf71e2 PS18, Line 610: dsm_ids[dsm_count].arg = (void *)is_cnviDdrRfim_enabled; `dsm_ids[dsm_count].arg = &is_cnvi_ddr_rfim_enabled;` -- To view, visit https://review.coreboot.org/c/coreboot/+/61020 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I217b736df3d4224a6732d1941a160abcddbd8f37 Gerrit-Change-Number: 61020 Gerrit-PatchSet: 18 Gerrit-Owner: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-CC: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Varshit B Pandya <varshit.b.pandya(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Maulik V Vaghela <maulik.v.vaghela(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sugnan Prabhu S <sugnan.prabhu.s(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Fri, 04 Mar 2022 15:42:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/var/primus{4es}: add enable pin to rtd3-cold
by Tim Wawrzynczak (Code Review) 04 Mar '22

04 Mar '22

Attention is currently required from: Casper Chang. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62586 ) Change subject: mb/google/brya/var/primus{4es}: add enable pin to rtd3-cold ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/62586/comment/57b4b5ce_0de52704 PS2, Line 9: add enable pin to rtd3-cold for primus{4es}. suggestion: ``` Currently the BayHub eMMC controller is only going into its reset state when the RTD3 sequence is initiated. This causes it to still consume too much power in suspend states. This CL adds the power enable GPIO into the RTD3 sequence as well, which will turn off the eMMC controller (a true D3cold state) during the RTD3 sequence. ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/62586 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2fec6a30707fb1a258cdcc73b0ce38252b6f77c8 Gerrit-Change-Number: 62586 Gerrit-PatchSet: 2 Gerrit-Owner: Casper Chang <casper_chang(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Ariel Fang <ariel_fang(a)wistron.corp-partner.google.com> Gerrit-CC: Cyan Yang <cyan.yang(a)intel.corp-partner.google.com> Gerrit-CC: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-CC: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-Attention: Casper Chang <casper_chang(a)wistron.corp-partner.google.com> Gerrit-Comment-Date: Fri, 04 Mar 2022 15:39:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2022