coreboot-gerrit April 2024

coreboot-gerrit@coreboot.org

1 participants
3293 discussions

[M] Change in coreboot[main]: nb/intel/sandybridge: Pre-render constants in MRC pei_data
by Angel Pons (Code Review) 14 Apr '24

14 Apr '24

Attention is currently required from: Keith Hui, Nico Huber. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78206?usp=email ) Change subject: nb/intel/sandybridge: Pre-render constants in MRC pei_data ...................................................................... Patch Set 4: (1 comment) File src/northbridge/intel/sandybridge/raminit_mrc.c: https://review.coreboot.org/c/coreboot/+/78206/comment/fa2732ee_4f8df9e3 : PS4, Line 404: struct pei_data pei_data; > This is a (pretty extreme) optimization attempt. […] Considering that the behaviour of struct initialisers in this regard is defined by the C standard, I don't think comments should be needed. -- To view, visit https://review.coreboot.org/c/coreboot/+/78206?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: Ic0f429a7be11ea497ce31cde007966ea988a256f Gerrit-Change-Number: 78206 Gerrit-PatchSet: 4 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Comment-Date: Sun, 14 Apr 2024 19:28:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Keith Hui <buurin(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: nb/intel/sandybridge: Pre-render constants in MRC pei_data
by Keith Hui (Code Review) 14 Apr '24

14 Apr '24

Attention is currently required from: Angel Pons, Nico Huber. Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78206?usp=email ) Change subject: nb/intel/sandybridge: Pre-render constants in MRC pei_data ...................................................................... Patch Set 4: (1 comment) File src/northbridge/intel/sandybridge/raminit_mrc.c: https://review.coreboot.org/c/coreboot/+/78206/comment/de126dd3_f9d50a4a : PS4, Line 404: struct pei_data pei_data; > This much easier to maintain, and also allows initialising fields whose value is always the result o […] This is a (pretty extreme) optimization attempt. With struct initialiser, there is still runtime code to populate the struct, although more optimized still, to the point it took me some time to understand what gcc did. I did a code size comparison of baseline (before this patch), cut struct (my proposal), struct initialiser (yours). raminit_mrc.o size: 45052/45724/44888 (bytes, respectively) perform_raminit() length: 1957/1862/1926 End of code offset before reset vector: 7feed0/7feef0/7feed0 bootblock and romstage (if separated) size are same at 57344 and 52848 bytes respectively. I don't see any further savings including system_type and gbe_enable in the struct initialiser. Finally, as struct initialiser also generated code to zero the struct for us, I'm comfortable dropping memset() and anything that set a field to 0, but I would need to add a comment saying compiler did it for us, lest someone thinks we have (another) uninitialized variable issue. -- To view, visit https://review.coreboot.org/c/coreboot/+/78206?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: Ic0f429a7be11ea497ce31cde007966ea988a256f Gerrit-Change-Number: 78206 Gerrit-PatchSet: 4 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 14 Apr 2024 17:46:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: intel/alderlake: Add helper functions for Power Management
by Nico Huber (Code Review) 14 Apr '24

14 Apr '24

Attention is currently required from: Dinesh Gehlot, Kapil Porwal, Lean Sheng Tan, Matt DeVillier, Nick Vaccaro, Sean Rhodes, Subrata Banik. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81638?usp=email ) Change subject: intel/alderlake: Add helper functions for Power Management ...................................................................... Patch Set 4: (2 comments) File src/soc/intel/alderlake/fsp_params.c: https://review.coreboot.org/c/coreboot/+/81638/comment/b4d4fc6d_f30da9c7 : PS4, Line 504: default "auto" > Right, so if someone (google) uses a modified version of FSP, shouldn't they add a quirk? Sorry, my bad, should have explained better. To coreboot, default (e.g. ASPM_DEFAULT) means the value we get when we don't override. That the FSP UPD has a default value itself and that the integrator could have changed this in the binary already is another layer. When I read this comment, I understood "use auto unless coreboot overwrites it". But now I see it could also mean "use the default, which is auto unless already overwritten in the binary". So it seems ambiguous. I wouldn't mention the default value. Maybe just say, use pre-set FSP value if nothing is set in the dt. If somebody doesn't want to set the value in the devicetree, they should know what they are doing, i.e. know what value their binary has. https://review.coreboot.org/c/coreboot/+/81638/comment/2d8d75c6_a9247b68 : PS4, Line 506: s_cfg->PcieRpAspm[index] = rp_cfg->pcie_rp_aspm; > `> 0`? I don't follow. Because of the if, the value we write would always be `> 0`, is that what you mean? `0` is a valid value to write, though, should mean disable (AIUI, the header comments seem incomplete). In coreboot, ASPM_DISABLE is `1`, though. So we'd have to `- 1` (assuming all the values of `enum ASPM_control` are exactly off by 1, compared to FSP). -- To view, visit https://review.coreboot.org/c/coreboot/+/81638?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: I9db18859f9a04ad4b7c0c3f7992b09e0f9484a81 Gerrit-Change-Number: 81638 Gerrit-PatchSet: 4 Gerrit-Owner: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <ericllai(a)google.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Comment-Date: Sun, 14 Apr 2024 10:41:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <sean(a)starlabs.systems> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: drivers/uart/pl011: Enhance struct documentation
by Nico Huber (Code Review) 14 Apr '24

14 Apr '24

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80180?usp=email ) Change subject: drivers/uart/pl011: Enhance struct documentation ...................................................................... drivers/uart/pl011: Enhance struct documentation Source: PrimeCell UART (PL011) Technical Reference Manual Revision: r1p5 Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: I58409b23e3790a052d3bc0ecf6a6bede15b4d76f Reviewed-on: https://review.coreboot.org/c/coreboot/+/80180 Reviewed-by: Nico Huber <nico.h(a)gmx.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/drivers/uart/pl011.h 1 file changed, 25 insertions(+), 25 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/src/drivers/uart/pl011.h b/src/drivers/uart/pl011.h index 1f5f0f7..bfdff87 100644 --- a/src/drivers/uart/pl011.h +++ b/src/drivers/uart/pl011.h @@ -7,31 +7,31 @@ /* PL011 r1p5 registers */ struct pl011_uart { - u32 dr; - u32 rsr_ecr; - u8 rsvd1[0x10]; - u32 fr; - u8 rsvd2[0x4]; - u32 ilpr; - u32 ibrd; - u32 fbrd; - u32 lcr_h; - u32 cr; - u32 ifls; - u32 imsc; - u32 ris; - u32 mis; - u32 icr; - u32 dmacr; - u8 rsvd3[0xf94]; - u32 periphid0; - u32 periphid1; - u32 periphid2; - u32 periphid3; - u32 cellid0; - u32 cellid1; - u32 cellid2; - u32 cellid3; + u32 dr; // offset: 0x0 Data Register + u32 rsr_ecr; // offset: 0x4 Receive Status Register / Error Clear Register + u8 rsvd1[0x10]; // offset: 0x8 Reserved + u32 fr; // offset: 0x18 Flag Register + u8 rsvd2[0x4]; // offset: 0x1C Reserved + u32 ilpr; // offset: 0x20 (IrDA) Low-Power Counter Register + u32 ibrd; // offset: 0x24 Integer Baud Rate Register + u32 fbrd; // offset: 0x28 Fractional Baud Rate Register + u32 lcr_h; // offset: 0x2C Line Control Register + u32 cr; // offset: 0x30 Control Register + u32 ifls; // offset: 0x34 Interrupt FIFO Level Select Register + u32 imsc; // offset: 0x38 Interrupt Mask Set/Clear Register + u32 ris; // offset: 0x3C Raw Interrupt Status Register + u32 mis; // offset: 0x40 Masked Interrupt status Register + u32 icr; // offset: 0x44 Interrupt Clear Register + u32 dmacr; // offset: 0x48 DMA Control Register + u8 rsvd3[0xf94]; // offset: 0x4C Reserved + u32 periphid0; // offset: 0xFE0 UART PeriphID0 Register + u32 periphid1; // offset: 0xFE4 UART PeriphID1 Register + u32 periphid2; // offset: 0xFE8 UART PeriphID2 Register + u32 periphid3; // offset: 0xFEC UART PeriphID3 Register + u32 cellid0; // offset: 0xFF0 UART CellID0 Register + u32 cellid1; // offset: 0xFF4 UART CellID1 Register + u32 cellid2; // offset: 0xFF8 UART CellID2 Register + u32 cellid3; // offset: 0xFFC UART CellID3 Register }; check_member(pl011_uart, cellid3, 0xffc); -- To view, visit https://review.coreboot.org/c/coreboot/+/80180?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: I58409b23e3790a052d3bc0ecf6a6bede15b4d76f Gerrit-Change-Number: 80180 Gerrit-PatchSet: 3 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[main]: drivers/uart/pl011: Enhance struct documentation
by Nico Huber (Code Review) 14 Apr '24

14 Apr '24

Attention is currently required from: Maximilian Brune. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80180?usp=email ) Change subject: drivers/uart/pl011: Enhance struct documentation ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80180?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: I58409b23e3790a052d3bc0ecf6a6bede15b4d76f Gerrit-Change-Number: 80180 Gerrit-PatchSet: 2 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Comment-Date: Sun, 14 Apr 2024 10:09:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: cpu/intel/model_206ax: Allow to configure VR settings
by Patrick Rudolph (Code Review) 14 Apr '24

14 Apr '24

Attention is currently required from: Alexander Couzens, Felix Held. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81597?usp=email ) Change subject: cpu/intel/model_206ax: Allow to configure VR settings ...................................................................... Patch Set 2: (1 comment) File src/mainboard/lenovo/x220/devicetree.cb: https://review.coreboot.org/c/coreboot/+/81597/comment/2ef72fe6_45e31dc9 : PS2, Line 41: 98 > Acknowledged Yes, schematics and vendor firmware are not in sync. It's unlikely that a 35W TDP CPU will draw 98Amps, so this is probably copy pasted and was never changed because "it worked". -- To view, visit https://review.coreboot.org/c/coreboot/+/81597?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: I59edab47fc4fbe0240e1dd7d25647f7549b4def2 Gerrit-Change-Number: 81597 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sun, 14 Apr 2024 07:53:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[main]: mb/erying: Add Erying Polestar G613 Pro (TGL-H)
by Alicja Michalska (Code Review) 14 Apr '24

14 Apr '24

Attention is currently required from: Felix Singer, Michał Żygowski, Nicholas Chin, Paul Menzel. Hello Felix Singer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80853?usp=email to look at the new patch set (#7). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: mb/erying: Add Erying Polestar G613 Pro (TGL-H) ...................................................................... mb/erying: Add Erying Polestar G613 Pro (TGL-H) Erying is a Chinese manufacturer selling desktop motherboards with laptop SoCs and custom shim to mount desktop coolers. Working: - Serial port (IT8613E RS232) - All rear USB ports (3.0, 2.0) - Both HDMI ports - Realtek GbE NIC - Internal audio (ALC897/ TGL-H HDMI) - Environment Controller (SuperIO fan control) - All SATA ports - All PCI-E/M.2 ports (somewhat) - M.2 NGFF WiFi - PCI-E Resizable BAR (ReBAR) - VT-x (PCI-E passtrough, broken on stock) WIP/Broken: - PCI-E ASPM (even though I force-disabled it, I'm still getting AERs) - S3/s0ix (also broken on stock, setting 3VSB register didn't help - system goes to sleep, but RAM loses power) - DisplayPort on I/O panel (simple fix, need to re-configure GPIOs) - One of USB2 FP connectors, as well as NGFF USB isn't mapped (yet) - Automatic fan control (IT8613E can't read CPU_TIN at the moment) Can be flashed using `flashrom -p internal -w build/coreboot.rom`, as vendor hasn't enabled any protections on SPI chip. I'd like to get ASPM working as it makes big difference in idle power consumtion (25 vs 60W measured from the wall at 230V). Likewise, I can't wrap my head around PCI-E AERs I'm getting if I boot the machine without `pcie_aspm=off` parameter: - BadTLP - BadDLLP - Timeout - Rollover Adjusting LaneEq's didn't change anything, all settings are configured in (mostly) the same way as they were on stock firmware. Starting to suspect Intel's FSP might be buggy, as I haven't had those issues when I initially started working on this project when 4.20 tree was current. TEST=Flash coreboot build onto the motherboard, install following PCI-E cards: Radeon RX 7800XT, Kingston KC3000, Optane 900P, Audigy X-Fi. Power the system up and boot into Windows 10 to check ACPI sanity, then reboot into Fedora Linux (kernel 6.7.4) and launch 3D application, disk benchmark, compilation at the same time to check system's stability. Change-Id: Iffb9e357da2eb686bdcd9a9837df8a60fa94011e Signed-off-by: Alicja Michalska <ahplka19(a)gmail.com> --- A src/mainboard/erying/Kconfig A src/mainboard/erying/Kconfig.name A src/mainboard/erying/tgl/Kconfig A src/mainboard/erying/tgl/Kconfig.name A src/mainboard/erying/tgl/Makefile.inc A src/mainboard/erying/tgl/board_info.txt A src/mainboard/erying/tgl/bootblock.c A src/mainboard/erying/tgl/cmos.layout A src/mainboard/erying/tgl/data.vbt A src/mainboard/erying/tgl/devicetree.cb A src/mainboard/erying/tgl/dsdt.asl A src/mainboard/erying/tgl/gpio.h A src/mainboard/erying/tgl/hda_verb.c A src/mainboard/erying/tgl/ramstage.c A src/mainboard/erying/tgl/romstage_fsp_params.c 15 files changed, 856 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/80853/7 -- To view, visit https://review.coreboot.org/c/coreboot/+/80853?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: Iffb9e357da2eb686bdcd9a9837df8a60fa94011e Gerrit-Change-Number: 80853 Gerrit-PatchSet: 7 Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[L] Change in coreboot[main]: mb/erying: Add Erying Polestar G613 Pro (TGL-H)
by Alicja Michalska (Code Review) 14 Apr '24

14 Apr '24

Attention is currently required from: Felix Singer, Michał Żygowski, Nicholas Chin, Paul Menzel. Alicja Michalska has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80853?usp=email ) Change subject: mb/erying: Add Erying Polestar G613 Pro (TGL-H) ...................................................................... Patch Set 6: (15 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/80853/comment/2990f4fb_8df237a1 : PS1, Line 25: even though I force-disabled it, I'm still getting AERs > No, of course not. […] Done https://review.coreboot.org/c/coreboot/+/80853/comment/af028987_d4868af9 : PS1, Line 39: : Likewise, I can't wrap my head around PCI-E AERs I'm getting if I boot : the machine without `pcie_aspm=off` parameter: : - BadTLP : - BadDLLP : - Timeout : - Rollover : : Adjusting LaneEq's didn't change anything, all settings are configured : in (mostly) the same way as they were on stock firmware. > Yes, that's true. I have disabled all ASPM I could find, though: […] Acknowledged File src/mainboard/erying/tgl/devicetree.cb: https://review.coreboot.org/c/coreboot/+/80853/comment/ff0c3a89_987c3d90 : PS6, Line 19: register "SkipExtGfxScan" = "0" > Set to 0, which is the default. Remove. Done https://review.coreboot.org/c/coreboot/+/80853/comment/b9c7feca_8dcdfb60 : PS6, Line 21: register "s0ix_enable" = "0" > Set to 0, which is the default. Remove. Acknowledged https://review.coreboot.org/c/coreboot/+/80853/comment/630d6f1b_a7907a4c : PS6, Line 23: # Actual device tree begins here: > Seems superfluous. Acknowledged https://review.coreboot.org/c/coreboot/+/80853/comment/2adade26_d58922eb : PS6, Line 26: device ref system_agent on end > System agent is enabled by default, remove. Done, but will test resulting build before publishing new patch set. I suspect removal of `device domain 0 on [...] end` broke mp_init last time around. https://review.coreboot.org/c/coreboot/+/80853/comment/86747e7b_a995f39a : PS6, Line 56: [0] = USB2_PORT_MID(OC0), /* Rear, bottom right */ : [1] = USB2_PORT_MID(OC0), /* Rear, bottom left */ : [2] = USB2_PORT_MID(OC2), /* NIC left */ : [3] = USB2_PORT_MID(OC2), /* NIC right */ : [4] = USB2_PORT_MID(OC2), /* Front Panel 1 */ : [5] = USB2_PORT_MID(OC2), /* Front Panel 2 */ : [8] = USB2_PORT_MID(OC0), /* Front Panel 1 (USB3) */ : [9] = USB2_PORT_MID(OC0), /* Front Panel 2 (USB3) */ : [10] = USB2_PORT_MID(OC0), /* Rear, top left */ : > Add one more tab Done https://review.coreboot.org/c/coreboot/+/80853/comment/8c1d0695_bc55e7f5 : PS6, Line 69: [0] = USB3_PORT_DEFAULT(OC0), /* Rear, bottom right */ : [1] = USB3_PORT_DEFAULT(OC0), /* Rear, bottom left */ : [2] = USB3_PORT_DEFAULT(OC0), /* Front Panel 1 */ : [3] = USB3_PORT_DEFAULT(OC0), /* Front Panel 2 */ : [4] = USB3_PORT_DEFAULT(OC0), /* Rear, top left */ : > Add one more tab Done https://review.coreboot.org/c/coreboot/+/80853/comment/7bb11847_ed3ebccb : PS6, Line 81: register "SataMode" = "0" > AHCI is the default, remove. Done File src/mainboard/erying/tgl/ramstage.c: PS6: > Same as with romstage_fsp_params. […] Done File src/mainboard/erying/tgl/romstage_fsp_params.c: https://review.coreboot.org/c/coreboot/+/80853/comment/0cb6bce3_7666cba2 : PS1, Line 99: gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table)); > Not really, will take a look. […] Done, moved to ramstage - will test resulting build tomorrow. File src/mainboard/erying/tgl/romstage_fsp_params.c: PS6: > I marked a few options which seemed obvious to me. […] Thanks, I've been planning on cleaning those options up but forgot to do so 😊 https://review.coreboot.org/c/coreboot/+/80853/comment/97b8cb9f_6ad45aa2 : PS6, Line 25: mupd->FspmConfig.HyperThreading = 1; > Remove. This is hooked up to the runtime setting `hyper_threading` and also to Kconfig. Done https://review.coreboot.org/c/coreboot/+/80853/comment/87ce5a86_fc2b6e57 : PS6, Line 58: mupd->FspmConfig.VmxEnable = 1; > Remove. This option is hooked up to the `ENABLE_VMX` Kconfig option. Done https://review.coreboot.org/c/coreboot/+/80853/comment/89797af6_c8454272 : PS6, Line 59: mupd->FspmConfig.SmbusEnable = 1; > Remove. That option is hooked up to the devicetree and it's enabled there. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/80853?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: Iffb9e357da2eb686bdcd9a9837df8a60fa94011e Gerrit-Change-Number: 80853 Gerrit-PatchSet: 6 Gerrit-Owner: Alicja Michalska <ahplka19(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Sun, 14 Apr 2024 01:00:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Alicja Michalska <ahplka19(a)gmail.com> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: drivers/uart/pl011: Enhance struct documentation
by Maximilian Brune (Code Review) 14 Apr '24

14 Apr '24

Attention is currently required from: Nico Huber. Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80180?usp=email ) Change subject: drivers/uart/pl011: Enhance struct documentation ...................................................................... Patch Set 2: (1 comment) File src/drivers/uart/pl011.h: https://review.coreboot.org/c/coreboot/+/80180/comment/68ee0e85_0f471359 : PS1, Line 15: u32 ilpr; // offset: 0x20 Low-Power Counter Register > The `i' seems to be for IrDA specifically, so please mention. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/80180?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: I58409b23e3790a052d3bc0ecf6a6bede15b4d76f Gerrit-Change-Number: 80180 Gerrit-PatchSet: 2 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Sat, 13 Apr 2024 23:55:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: drivers/uart/pl011: Enhance struct documentation
by Maximilian Brune (Code Review) 14 Apr '24

14 Apr '24

Attention is currently required from: Maximilian Brune, Nico Huber. Hello Nico Huber, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80180?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by Nico Huber, Verified+1 by build bot (Jenkins) Change subject: drivers/uart/pl011: Enhance struct documentation ...................................................................... drivers/uart/pl011: Enhance struct documentation Source: PrimeCell UART (PL011) Technical Reference Manual Revision: r1p5 Signed-off-by: Maximilian Brune <maximilian.brune(a)9elements.com> Change-Id: I58409b23e3790a052d3bc0ecf6a6bede15b4d76f --- M src/drivers/uart/pl011.h 1 file changed, 25 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/80180/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80180?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: I58409b23e3790a052d3bc0ecf6a6bede15b4d76f Gerrit-Change-Number: 80180 Gerrit-PatchSet: 2 Gerrit-Owner: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-MessageType: newpatchset

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2024