Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons. 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 172:
(2 comments)
Patchset:
PS172:
Very nice. It’d be great if you wrapped the comments as the linter scripts commented, but the rest looks nice.
I will address this in the next patchset.
Out of curiosity, how did you create the ASL files?
I started with the tables decoded by iASL, then modified and improved them (in some instances) based on comments here. For instance, while the EC (ACPI; over I/O) memory map is being used, a certain register has to be written to obtain different pieces of information about the battery state. In an MMIO-based memory map (LPC device has a "generic memory register" - LGMR), the data does not overlap and this isn't needed.
PS172:
Build failure for patch set 171: […]
The vendor firmware used some I/O port writes to seemingly 'trigger' the EC to handle commands. However, a different firmware phase (DXE) uses `Stall(15)` equivalently - in coreboot, this would be `mdelay` - and reads from the port always return 0xFF.
Therefore, I will remove this.