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 140:
(8 comments)
https://review.coreboot.org/c/coreboot/+/35523/119/src/mainboard/acer/aspire... File src/mainboard/acer/aspire_vn7_572g/Kconfig:
https://review.coreboot.org/c/coreboot/+/35523/119/src/mainboard/acer/aspire... PS119, Line 75: #config TPM_PIRQ : # hex : # default 0x1e # GPP_A6_IRQ
I've double-checked the schematics and found that the interrupt is SERIRQ, so I'm removing this. […]
This is a consumer laptop. Perhaps I'm being overly critical, but I don't expect to see a model with a physical TPM anytime soon.
https://review.coreboot.org/c/coreboot/+/35523/122/src/mainboard/acer/aspire... File src/mainboard/acer/aspire_vn7_572g/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/35523/122/src/mainboard/acer/aspire... PS122, Line 11: EB0A, 1, : , 2, : EB0R, 1, : EB0L, 1, : EB0F, 1, : EB0N, 1
That does seem correct. […]
Done
https://review.coreboot.org/c/coreboot/+/35523/122/src/mainboard/acer/aspire... PS122, Line 41: Offset (0xE0),
Right, I had noticed that the offsets were the same, but that full 'paging' wasn't implemented the w […]
The vendor firmware didn't need to implement paging, because they read from their block in memory. This explains why I get garbage for "vendor," "model," "serial" and "energy*" values in upower or "/sys/class/power_supply/BAT0/".
https://review.coreboot.org/c/coreboot/+/35523/122/src/mainboard/acer/aspire... PS122, Line 283: (Local1 & 0x08)
Well, endianness (byte order) only matters if there are multiple bytes. […]
Thanks for the explanation. Done.
(You were probably correct anyways. With regards to PCI configuration done in ACPI, I have seen that byte order was applied and relevant in the Intel datasheet. So, on the chip itself.)
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 actually backs SATA. Port 8 is NGFF. […]
Done
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 53: 0
RP7/SATA0 is muxed with RP8/SATA1 to make an x2 port for mSATA. […]
Wired over both. Done.
https://review.coreboot.org/c/coreboot/+/35523/62/src/mainboard/acer/aspire_... PS62, Line 232: [PchSerialIoIndexUart2] = PchSerialIoDisabled,
Should CONSOLE_SERIAL be enabled? If it is an LPSS UART and in-memory, should the coreboot + payload […]
I don't know why I thought that LPSS was in-memory, but in any event, it is probably safer to leave CONSOLE_SERIAL disabled.
https://review.coreboot.org/c/coreboot/+/35523/119/src/mainboard/acer/aspire... File src/mainboard/acer/aspire_vn7_572g/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/35523/119/src/mainboard/acer/aspire... PS119, Line 68: # TODO: register "DspEndpointDmic" = "3"
The microphone is attached to the webcam and the schematics assert that the pins are "reserved for S […]
The driver would set the pin_cfg, but it isn't working yet anyway. Maybe I'll test with Windows.