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 40:
(6 comments)
https://review.coreboot.org/c/coreboot/+/35523/40//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35523/40//COMMIT_MSG@28 PS40, Line 28: - Display (with libgfxinit) Would be good to test
https://review.coreboot.org/c/coreboot/+/35523/40/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/35523/40/src/mainboard/acer/aspire_... PS40, Line 110: // ^^^GFX0.GLID (KLID) ?
https://review.coreboot.org/c/coreboot/+/35523/40/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/acpi/mainboard.asl:
https://review.coreboot.org/c/coreboot/+/35523/40/src/mainboard/acer/aspire_... PS40, Line 42: // If (CondRefOf (_SB.TPM.PTS)) ?
https://review.coreboot.org/c/coreboot/+/35523/40/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35523/40/src/mainboard/acer/aspire_... PS40, Line 71: register "SaGv" = "3" There should be an enum for SaGv values (not sure if Skylake has it)
https://review.coreboot.org/c/coreboot/+/35523/40/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/35523/40/src/mainboard/acer/aspire_... PS40, Line 50: // Chipset specific sleep states "common" isn't really "chipset specific", so maybe drop the comment?
https://review.coreboot.org/c/coreboot/+/35523/40/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/35523/40/src/mainboard/acer/aspire_... PS40, Line 34: /* Pin Complex (NID 0x12) */ Please remove these comments, they don't provide useful info