coreboot-gerrit August 2024

coreboot-gerrit@coreboot.org

1 participants
3913 discussions

[XS] Change in coreboot[main]: mb/google/brox/jubilant: Update GPE0 routing
by Subrata Banik (Code Review) 29 Aug '24

29 Aug '24

Attention is currently required from: Karthik Ramasubramanian, Nick Vaccaro, Ren Kuo. Subrata Banik has posted comments on this change by Ren Kuo. ( https://review.coreboot.org/c/coreboot/+/84124?usp=email ) Change subject: mb/google/brox/jubilant: Update GPE0 routing ...................................................................... Patch Set 4: (1 comment) File src/mainboard/google/brox/variants/jubilant/overridetree.cb: https://review.coreboot.org/c/coreboot/+/84124/comment/4062a7e7_6bbbd6b7?us… : PS4, Line 25: register "pmc_gpe0_dw1" = "GPP_F" > I am afraid this is going to break all the wake sources from EC - Lid Open, Key Press events, AC Connect/Disconnect etc. Did you check that? > > Remember GPP_D0/D1 are where the EC interrupt and wake signals are routed to. It is unfortunate that the wake sources are routed to more than 3 GPP groups. > > WLAN Wake -> GPP_B > EC Wake -> GPP_D > GSC, TCHPAD Wake -> GPP_E > FP Wake -> GPP_F > > We need to limit it to 3 GPIO groups. if wake sources are scattered across more than 3 GPIO bank then you can use GPIO Tier 1 register to add additional entries. Tier 1 also provides 3 more GPIO bank like Tier 0 -- To view, visit https://review.coreboot.org/c/coreboot/+/84124?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic4a7ca07eab0dab234ab025cf77bbb8093b6b9d1 Gerrit-Change-Number: 84124 Gerrit-PatchSet: 4 Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Bob Moragues <moragues(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kenneth Chan <kenneth.chan(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 29 Aug 2024 17:23:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>

1 0

[XS] Change in coreboot[main]: mb/google/brox/jubilant: Update GPE0 routing
by Karthik Ramasubramanian (Code Review) 29 Aug '24

29 Aug '24

Attention is currently required from: Nick Vaccaro, Ren Kuo, Subrata Banik. Karthik Ramasubramanian has posted comments on this change by Ren Kuo. ( https://review.coreboot.org/c/coreboot/+/84124?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: mb/google/brox/jubilant: Update GPE0 routing ...................................................................... Patch Set 4: (1 comment) File src/mainboard/google/brox/variants/jubilant/overridetree.cb: https://review.coreboot.org/c/coreboot/+/84124/comment/7ed6b1b4_e67a990c?us… : PS4, Line 25: register "pmc_gpe0_dw1" = "GPP_F" I am afraid this is going to break all the wake sources from EC - Lid Open, Key Press events, AC Connect/Disconnect etc. Did you check that? Remember GPP_D0/D1 are where the EC interrupt and wake signals are routed to. It is unfortunate that the wake sources are routed to more than 3 GPP groups. WLAN Wake -> GPP_B EC Wake -> GPP_D GSC, TCHPAD Wake -> GPP_E FP Wake -> GPP_F We need to limit it to 3 GPIO groups. -- To view, visit https://review.coreboot.org/c/coreboot/+/84124?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic4a7ca07eab0dab234ab025cf77bbb8093b6b9d1 Gerrit-Change-Number: 84124 Gerrit-PatchSet: 4 Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Bob Moragues <moragues(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kenneth Chan <kenneth.chan(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Thu, 29 Aug 2024 17:17:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in coreboot[main]: mb/google/brox/jubilant: Update dptf settings
by Karthik Ramasubramanian (Code Review) 29 Aug '24

29 Aug '24

Attention is currently required from: Nick Vaccaro, Peter Ou, Ren Kuo. Karthik Ramasubramanian has posted comments on this change by Ren Kuo. ( https://review.coreboot.org/c/coreboot/+/84123?usp=email ) Change subject: mb/google/brox/jubilant: Update dptf settings ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84123?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2d59eedea9fb25565709e118abc1a14b4c2a64e7 Gerrit-Change-Number: 84123 Gerrit-PatchSet: 3 Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Bob Moragues <moragues(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kenneth Chan <kenneth.chan(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Peter Ou <peter.ou(a)quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Peter Ou <peter.ou(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Thu, 29 Aug 2024 17:04:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: mb/google/brox/jubilant: Update dptf settings
by Bob Moragues (Code Review) 29 Aug '24

29 Aug '24

Attention is currently required from: Karthik Ramasubramanian, Nick Vaccaro, Peter Ou, Ren Kuo. Bob Moragues has posted comments on this change by Ren Kuo. ( https://review.coreboot.org/c/coreboot/+/84123?usp=email ) Change subject: mb/google/brox/jubilant: Update dptf settings ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/84123?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I2d59eedea9fb25565709e118abc1a14b4c2a64e7 Gerrit-Change-Number: 84123 Gerrit-PatchSet: 3 Gerrit-Owner: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Bob Moragues <moragues(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kenneth Chan <kenneth.chan(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Peter Ou <peter.ou(a)quanta.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Ren Kuo <ren.kuo(a)quanta.corp-partner.google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Peter Ou <peter.ou(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Thu, 29 Aug 2024 16:45:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: soc/intel/common/block/cpu: Fix ways count computation regression
by Jérémy Compostella (Code Review) 29 Aug '24

29 Aug '24

Jérémy Compostella has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/83948?usp=email ) Change subject: soc/intel/common/block/cpu: Fix ways count computation regression ...................................................................... Patch Set 11: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/83948/comment/396eabff_c2d26a68?us… : PS11, Line 32: TEST=Verified on PTL Intel reference platform > I performed the following experiment: […] Sorry, I though the question was for https://review.coreboot.org/c/coreboot/+/83946/10. To verify this particular issue, I use a configuration with `CONFIG_DCACHE_RAM_SIZE > Effective way size` and then there a couple of options to observe the bug/fix: 1. Use a debugger, run a step by step and look at the registers 2. Dump some register using the post_code() macro in the algorithm I used #2 as it was faster for me. I created the following temporary macro for this purpose. ``` modified src/include/cpu/x86/post_code.h @@ -9,7 +9,25 @@ #define post_code(value) \ movb $value, %al; \ outb %al, $CONFIG_POST_IO_PORT - +#define post_code_safe(value, backup) \ + mov %eax, backup; \ + movb $value, %al; \ + outb %al, $CONFIG_POST_IO_PORT; \ + mov backup, %eax; +#define dump_register(reg, backup) \ + mov %eax, backup; \ + movb $0xa, %al; \ + outb %al, $CONFIG_POST_IO_PORT; \ + mov backup, %eax; \ + mov reg, %eax; \ + outb %al, $CONFIG_POST_IO_PORT; \ + shrl $8, %eax; \ + outb %al, $CONFIG_POST_IO_PORT; \ + shrl $8, %eax; \ + outb %al, $CONFIG_POST_IO_PORT; \ + shrl $8, %eax; \ + outb %al, $CONFIG_POST_IO_PORT; \ + mov backup, %eax; #else #define post_code(value) ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/83948?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521d Gerrit-Change-Number: 83948 Gerrit-PatchSet: 11 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Thu, 29 Aug 2024 16:43:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[XS] Change in coreboot[main]: soc/intel/common/block/cpu: Fix ways count computation regression
by Jérémy Compostella (Code Review) 29 Aug '24

29 Aug '24

Jérémy Compostella has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/83948?usp=email ) Change subject: soc/intel/common/block/cpu: Fix ways count computation regression ...................................................................... Patch Set 11: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/83948/comment/f3cc5473_d9cefab0?us… : PS11, Line 32: TEST=Verified on PTL Intel reference platform > For the ignorant, how can this be verified? I performed the following experiment: On Panther Lake, I set `DCACHE_RAM_SIZE` to `0x400000` (4 MB). Without the patches the number of ways on a 18 MB is going to be `0x400000 / 0x180000` = 2.66 = 3. While actually according the specification (EDS / HAS), we should only consider the biggest number of two of the way size and the computation should be `0x400000 / 0x100000` = 4. It boots just fine with this patch but it crashes with page fault exception if I unselect `INTEL_CAR_ENEM_USE_EFFECTIVE_WAY_SIZE`. -- To view, visit https://review.coreboot.org/c/coreboot/+/83948?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5cb66da0aa977eecb64a0021268a6827747c521d Gerrit-Change-Number: 83948 Gerrit-PatchSet: 11 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Thu, 29 Aug 2024 16:38:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[S] Change in coreboot[main]: soc/intel/common/gpio: support 16-bit CPU Port ID
by Cliff Huang (Code Review) 29 Aug '24

29 Aug '24

Attention is currently required from: Bora Guvendik, Jamie Ryu, Jérémy Compostella, Paul Menzel, Saurabh Mishra, Subrata Banik, Wonkyu Kim. Cliff Huang has posted comments on this change by Cliff Huang. ( https://review.coreboot.org/c/coreboot/+/83981?usp=email ) Change subject: soc/intel/common/gpio: support 16-bit CPU Port ID ...................................................................... Patch Set 8: (1 comment) Patchset: PS7: > > Subrata, Can google request Panther Lake Platform Firmware Architecture Specification (FAS), docum […] 8.13.1 and 8.13.2 has 16-bit Port ID info. We are also working on vw info for external. -- To view, visit https://review.coreboot.org/c/coreboot/+/83981?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8c1a48d587bd41178b0c6bb0144fda93e292423d Gerrit-Change-Number: 83981 Gerrit-PatchSet: 8 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Thu, 29 Aug 2024 16:37:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>

1 0

[XL] Change in coreboot[main]: soc/intel/ptl: Do initial Panther Lake SoC commit till ramstage
by Jérémy Compostella (Code Review) 29 Aug '24

29 Aug '24

Attention is currently required from: Kapil Porwal, Pranava Y N, Saurabh Mishra, Subrata Banik. Jérémy Compostella has posted comments on this change by Saurabh Mishra. ( https://review.coreboot.org/c/coreboot/+/83798?usp=email ) Change subject: soc/intel/ptl: Do initial Panther Lake SoC commit till ramstage ...................................................................... Patch Set 62: (18 comments) File src/soc/intel/pantherlake/acpi.c: https://review.coreboot.org/c/coreboot/+/83798/comment/5e1ada12_0448e0fa?us… : PS50, Line 120: int size_t https://review.coreboot.org/c/coreboot/+/83798/comment/2981390b_114293a8?us… : PS50, Line 126: is_s0ix_enable I don't see the benefit of this intermediary variable. https://review.coreboot.org/c/coreboot/+/83798/comment/a45e25d5_f83b25e3?us… : PS50, Line 138: map As this is returning an inner static variable, shouldn't we have a mechanism to make sure we do not perform the same operation if the function is called again ? https://review.coreboot.org/c/coreboot/+/83798/comment/8c9aee35_605f3e67?us… : PS50, Line 157: 4 sizeof(uint32_t) File src/soc/intel/pantherlake/chip.h: https://review.coreboot.org/c/coreboot/+/83798/comment/5f357e23_8de488fd?us… : PS55, Line 7: #include <device/pci_ids.h> Move line 7 up. https://review.coreboot.org/c/coreboot/+/83798/comment/529c53fd_6bf26166?us… : PS55, Line 46: PTL_U_404_4_15W_CORE, /* 4 P-cores +0 E-cores + 4 LP E-cores + 4Xe25W*/ > one space after `+` And one space before ending `*/` https://review.coreboot.org/c/coreboot/+/83798/comment/129d8878_e932f41d?us… : PS55, Line 47: PTL_H_484_12_25W_CORE Instead of commenting out every single line, what about something like the following ? ``` /* * Panther Lake power limits naming convention: * PTL_<TYPE>_<PCORE#><ECORE#><LP-ECORE#>_<TDP>W_CORE */ ``` https://review.coreboot.org/c/coreboot/+/83798/comment/6486312a_d1a643fe?us… : PS55, Line 339: uint8_t enable_c6dram; : uint8_t pm_timer_disabled; following the logic of some fields above, shouldn't these two be booleans ? File src/soc/intel/pantherlake/chip.h: https://review.coreboot.org/c/coreboot/+/83798/comment/b8a860c1_8212dee6?us… : PS50, Line 7: #include <device/pci_ids.h> Move that one one line up. File src/soc/intel/pantherlake/chip.c: https://review.coreboot.org/c/coreboot/+/83798/comment/b6006751_09cebfd8?us… : PS55, Line 61: printk(BIOS_DEBUG, "dev->path.type=%x\n", dev->path.usb.port_type); Shouldn't this be an `BIOS_ERR` ? The message is wrong as it print the USB port_type not the path type. Also,shouldn't we print something like `Missing ACPI Name for USB port_type=0x%x\n` https://review.coreboot.org/c/coreboot/+/83798/comment/40147dac_015c3a13?us… : PS55, Line 65: printk(BIOS_DEBUG, "dev->path.type=%x\n", dev->path.type); Similar remark here. https://review.coreboot.org/c/coreboot/+/83798/comment/1c31ff9a_730b42af?us… : PS55, Line 84: case PCI_DEVFN_I2C0: return "I2C0"; : case PCI_DEVFN_I2C1: return "I2C1"; : case PCI_DEVFN_I2C2: return "I2C2"; : case PCI_DEVFN_I2C3: return "I2C3"; : case PCI_DEVFN_I2C4: return "I2C4"; : case PCI_DEVFN_I2C5: return "I2C5"; : case PCI_DEVFN_CSE: return "HECI"; : case PCI_DEVFN_PCIE1: return "RP01"; : case PCI_DEVFN_PCIE2: return "RP02"; : case PCI_DEVFN_PCIE3: return "RP03"; : case PCI_DEVFN_PCIE4: return "RP04"; : case PCI_DEVFN_PCIE5: return "RP05"; : case PCI_DEVFN_PCIE6: return "RP06"; : case PCI_DEVFN_PCIE7: return "RP07"; : case PCI_DEVFN_PCIE8: return "RP08"; : case PCI_DEVFN_PCIE9: return "RP09"; : case PCI_DEVFN_PCIE10: return "RP10"; : case PCI_DEVFN_PCIE11: return "RP11"; : case PCI_DEVFN_PCIE12: return "RP12"; : case PCI_DEVFN_PMC: return "PMC"; : case PCI_DEVFN_UART0: return "UAR0"; : case PCI_DEVFN_UART1: return "UAR1"; : case PCI_DEVFN_UART2: return "UAR2"; : case PCI_DEVFN_GSPI0: return "SPI0"; : case PCI_DEVFN_GSPI1: return "SPI1"; : /* Keeping ACPI device name coherent with ec.asl */ : case PCI_DEVFN_ESPI: return "LPCB"; : case PCI_DEVFN_HDA: return "HDAS"; : case PCI_DEVFN_SMBUS: return "SBUS"; : case PCI_DEVFN_SPI: return "FSPI"; : case PCI_DEVFN_GBE: return "GLAN"; Aren't those missing an extra tab ? https://review.coreboot.org/c/coreboot/+/83798/comment/932c4258_d52b059c?us… : PS55, Line 116: printk(BIOS_DEBUG, "Missing ACPI Name for PCI: 00:%02x.%01x\n", Shouldn't be `BIOS_ERR` ? File src/soc/intel/pantherlake/cpu.c: https://review.coreboot.org/c/coreboot/+/83798/comment/4ee3b365_c53e8613?us… : PS55, Line 59: (1 << 0) BIT(0) https://review.coreboot.org/c/coreboot/+/83798/comment/a9150677_a0cf2169?us… : PS55, Line 60: (1 << 3) BIT(3) ... https://review.coreboot.org/c/coreboot/+/83798/comment/dac546bd_63769f54?us… : PS55, Line 88: else unnecessary `else` keyword. File src/soc/intel/pantherlake/crashlog.c: https://review.coreboot.org/c/coreboot/+/83798/comment/8b91998e_8d67bd05?us… : PS55, Line 62: sram_bar = res->base; Why do we need this intermediate value ? Can't we just do `return res->base` ? https://review.coreboot.org/c/coreboot/+/83798/comment/9da269cf_0b34a497?us… : PS55, Line 98: while (cl_cur && cl_cur->next) { unnecessary brackets -- To view, visit https://review.coreboot.org/c/coreboot/+/83798?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Idc6fb11e9e84c28c7567ae2b7abc1ab832a88362 Gerrit-Change-Number: 83798 Gerrit-PatchSet: 62 Gerrit-Owner: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Pranava Y N <pranavayn(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-CC: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-CC: Balaji Manigandan <balaji.manigandan(a)intel.com> Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-CC: Cliff Huang <cliff.huang(a)intel.com> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-CC: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-CC: Krishna P Bhat D <krishna.p.bhat.d(a)intel.com> Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-CC: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-CC: Sanju Jose Thottan <sanjujose.thottan(a)intel.com> Gerrit-CC: Saurabh Mishra <mishra.saurabh(a)intel.corp-partner.google.com> Gerrit-CC: Vikrant L Jadeja <vikrant.l.jadeja(a)intel.com> Gerrit-CC: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Saurabh Mishra <mishra.saurabh(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Pranava Y N <pranavayn(a)google.com> Gerrit-Comment-Date: Thu, 29 Aug 2024 16:35:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>

1 0

[XS] Change in coreboot[main]: soc/intel/meteorlake: Hook up microcode from repository
by Nico Huber (Code Review) 29 Aug '24

29 Aug '24

Attention is currently required from: Eran Mitrani, Felix Singer, Jakub Czapiga, Kapil Porwal, Tarun. Nico Huber has posted comments on this change by Felix Singer. ( https://review.coreboot.org/c/coreboot/+/84125?usp=email ) Change subject: soc/intel/meteorlake: Hook up microcode from repository ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/84125?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I46021accacbb911d7a7ecfdbb52973a7da78f36e Gerrit-Change-Number: 84125 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Eran Mitrani <mitrani(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Tarun <tstuli(a)gmail.com> Gerrit-Comment-Date: Thu, 29 Aug 2024 16:15:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in libgfxinit[main]: gma display_probing: Make new TGL ports available
by Angel Pons (Code Review) 29 Aug '24

29 Aug '24

Attention is currently required from: Dinesh Gehlot, Elyes Haouas, Nico Huber, Paul Menzel, Thomas Heijligen. Angel Pons has posted comments on this change by Nico Huber. ( https://review.coreboot.org/c/libgfxinit/+/81522?usp=email ) Change subject: gma display_probing: Make new TGL ports available ...................................................................... Patch Set 7: (1 comment) File common/hw-gfx-gma-display_probing.adb: https://review.coreboot.org/c/libgfxinit/+/81522/comment/c18c45c1_fcc89d7e?… : PS7, Line 38: function Sibling_Port (Port : Port_Type) return Port_Type is > Yes, but DPx and HDMIx refer to the traditional (not Type-C capable) ports. I think this is fine. Hmmmmm, wait. HDMI_TCx and USBCx would need to be here, I think? -- To view, visit https://review.coreboot.org/c/libgfxinit/+/81522?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libgfxinit Gerrit-Branch: main Gerrit-Change-Id: I9d96673f931be0086536044694ecd127ba2a823d Gerrit-Change-Number: 81522 Gerrit-PatchSet: 7 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Dinesh Gehlot <digehlot(a)google.com> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Thu, 29 Aug 2024 15:34:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit August 2024