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 62:
(6 comments)
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/Kconfig:
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... PS62, Line 55: #config MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID This can go away, I guess?
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... PS62, Line 155: Method (_Q1C, 0, NotSerialized) // _Qxx: EC Query, xx=0x00-0xFF Have you figured out what these EC queries do? It would be nice to comment what they are about.
With Linux, you can add ASL "debug prints" and apply them with a DSDT override, all while using the vendor firmware. The idea to see what vendor firmware does with this.
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... PS62, Line 107: #| AC LoadLine | 10.3 mOhm | 2.4 mOhm | 3.1 mOhm | 3.1 mOhm | Out of curiosity, where do you get the loadline values from?
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.asl` ?
https://review.coreboot.org/c/coreboot/+/35523/54/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/ramstage.c:
https://review.coreboot.org/c/coreboot/+/35523/54/src/mainboard/acer/aspire_... PS54, Line 33: gpio_get(DGPU_PRESENT)
I hope that there's no policy against using them, but yes. […]
I haven't heard of any policy against wanting to do things properly :)
Personally, I strongly prefer using schematics. It is much better than guessing things.
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... File src/mainboard/acer/aspire_vn7_572g/ramstage.c:
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... PS62, Line 34: system76 hmmmmm