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