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 64:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/acpi/mbdefines.asl:
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... PS62, Line 29: 0xFE800000
Given the high location, this will be some hardware MMIO. Which may be […]
Definitely the vendor firmware, or is this value universal? In any case, I'll have to work on this.
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... PS62, Line 34: 0x7CE7D018
That does sound very bad. How should this be handled instead? […]
I've fixed this by factoring it out and including some functionality from EC_ACPI. However, I haven't managed to implement everything I've removed and this remains untested.
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... PS62, Line 46: // Mainboard specific
Is it necessary to use two different files? If so, can any of them be renamed to, for example, `ec. […]
Were you referring to mbdefines.asl? If so, the remainder of that has since been refactored into dsdt.asl after including EC_ACPI.
This remains untested; I haven't managed to implement everything that I've removed.