Attention is currently required from: Nico Huber, Eugene Myers, Paul Menzel, Angel Pons, Michael Niewöhner. Benjamin Doron has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35523 )
Change subject: mb/acer: Add Acer Aspire VN7-572G ......................................................................
Patch Set 193:
(5 comments)
File src/mainboard/acer/aspire_vn7_572g/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35523/comment/af4980a8_c7cdfcf0 PS192, Line 14: .chipset_lockdown = CHIPSET_LOCKDOWN_COREBOOT,
Should no longer be needed, it's the default now.
Done
File src/mainboard/acer/aspire_vn7_572g/romstage.c:
https://review.coreboot.org/c/coreboot/+/35523/comment/44184a18_7d8f01be PS192, Line 13: /* TODO: Search vendor FW for Dq/Dqs */
What does this mean? DQ/DQS mapping is only needed for LPDDR.
The FSP binary has non-zero defaults for Dq/Dqs. Do we know if the FSP detects LPDDR and only applies Dq/Dqs if it's present? If so, I actually can drop this. Otherwise, I should find the optimum settings for this board.
File src/mainboard/acer/aspire_vn7_572g/smihandler.c:
https://review.coreboot.org/c/coreboot/+/35523/comment/87c55eb2_7c0527b2 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 […]
I think that was the definition in the SDM I had? Anyways, done.
https://review.coreboot.org/c/coreboot/+/35523/comment/39f15413_d9438d3f PS192, Line 32: 0x1A0
Why not use the `IA32_MISC_ENABLE` macro from src/include/cpu/x86/msr. […]
Done
https://review.coreboot.org/c/coreboot/+/35523/comment/5d9302d6_87bcc264 PS192, Line 54: unused_was_osys
Does this still work?
Not really, but OSYS wasn't actually implemented before the recent-ish commits to drop it either. I think that this underlies whether EC supports some features, such as the touchpad toggle hotkey, so I want to get _OSI method back in coreboot.
However, the vendor implements the touchpad toggle hotkey in WMI. While coreboot obviously won't do that, perhaps simply telling the EC to support the feature so that the enable bit works isn't enough. I may need to reverse engineer the WMI blob as well.
I'm adding a TODO for this and marking it as done?