Attention is currently required from: Nico Huber, Eugene Myers, Benjamin Doron, Paul Menzel, Michael Niewöhner. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35523 )
Change subject: mb/acer: Add Acer Aspire VN7-572G ......................................................................
Patch Set 192:
(7 comments)
File src/mainboard/acer/aspire_vn7_572g/bootblock.c:
https://review.coreboot.org/c/coreboot/+/35523/comment/471026c5_2578fc13 PS192, Line 18: Is a wait required? Likely
File src/mainboard/acer/aspire_vn7_572g/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35523/comment/1b00ca3b_9cdc48f9 PS192, Line 14: .chipset_lockdown = CHIPSET_LOCKDOWN_COREBOOT, Should no longer be needed, it's the default now.
File src/mainboard/acer/aspire_vn7_572g/romstage.c:
https://review.coreboot.org/c/coreboot/+/35523/comment/da2ac735_9b8d54d8 PS192, Line 13: /* TODO: Search vendor FW for Dq/Dqs */ What does this mean? DQ/DQS mapping is only needed for LPDDR.
File src/mainboard/acer/aspire_vn7_572g/smihandler.c:
https://review.coreboot.org/c/coreboot/+/35523/comment/13dcd26c_57e3f5b2 PS192, Line 23: /* Keep in sync with dsdt.asl; could insert into SSDT at runtime */ To avoid things falling out of sync, you could define this in a header and include it from dsdt.asl and here
https://review.coreboot.org/c/coreboot/+/35523/comment/953e7956_6368814d PS192, Line 26: IDA_Disable Where does this name come from? IDA was the name from Core 2 era processors, it's called "TURBO_MODE_DISABLE" on SKL.
https://review.coreboot.org/c/coreboot/+/35523/comment/eb7b28a5_eb12ac92 PS192, Line 32: 0x1A0 Why not use the `IA32_MISC_ENABLE` macro from src/include/cpu/x86/msr.h ?
https://review.coreboot.org/c/coreboot/+/35523/comment/c7a67f3d_1bfa5ae4 PS192, Line 54: unused_was_osys Does this still work?