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 45: Code-Review+1
(8 comments)
https://review.coreboot.org/c/coreboot/+/35523/45//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35523/45//COMMIT_MSG@23 PS45, Line 23: ACPI You can dump the DSDT of the vendor firmware. For EC stuffs, that is usually the only way to make it work. For example, do you have a battery device? Even if not using Arch Linux, you can use this guide to dump and decompile the DSDT:
https://wiki.archlinux.org/index.php/DSDT#Recompiling_it_yourself
You don't need to recompile it. Rather, use the source as a reference to check things against.
https://review.coreboot.org/c/coreboot/+/35523/45//COMMIT_MSG@28 PS45, Line 28: - Remaining RAM slots (need other SPD addresses) How many RAM slots does this thing have?
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)
Vendor BIOS calls this method inside the graphics device, which I need to rewrite or remove. […]
Ack
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))
Vendor BIOS calls the prepare-to-sleep method on the TPM's ACPI device. […]
Ack
https://review.coreboot.org/c/coreboot/+/35523/13/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35523/13/src/mainboard/acer/aspire_... PS13, Line 150: # register "PcieRpEnable[6]" = "1"
Port 7 is enabled in OEM BIOS. […]
NVMe should have x4 lanes. If it does, it might be it
https://review.coreboot.org/c/coreboot/+/35523/11/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/35523/11/src/mainboard/acer/aspire_... PS11, Line 74:
Assuming here that another "{double ,}function reset" would undo previous configuration
I think so. I'll mark this as resolved.
https://review.coreboot.org/c/coreboot/+/35523/45/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/35523/45/src/mainboard/acer/aspire_... PS45, Line 29: 0x10251037 No need to repeat the literal hex value in the comment
https://review.coreboot.org/c/coreboot/+/35523/45/src/mainboard/acer/aspire_... PS45, Line 38: 0x1A nit: use lowercase for these hex constants