Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37887 )
Change subject: mb/intel/tglrvp : Add ACPI support for CNVi ......................................................................
Patch Set 3:
(7 comments)
https://review.coreboot.org/c/coreboot/+/37887/3/src/mainboard/intel/tglrvp/... File src/mainboard/intel/tglrvp/acpi/cnvi.asl:
https://review.coreboot.org/c/coreboot/+/37887/3/src/mainboard/intel/tglrvp/... PS3, Line 19:
Distance doesn’t need to be this big?
Ack
https://review.coreboot.org/c/coreboot/+/37887/3/src/mainboard/intel/tglrvp/... PS3, Line 59: #endif
Remove.
Ack
https://review.coreboot.org/c/coreboot/+/37887/3/src/mainboard/intel/tglrvp/... PS3, Line 67: // PowerResource expects to have _STA, _ON and _OFF Method per ACPI Spec. Not having one of them will cause BSOD
Tested with Microsoft Windows?
Ack
https://review.coreboot.org/c/coreboot/+/37887/3/src/mainboard/intel/tglrvp/... PS3, Line 74:
Trailing space.
Ack
https://review.coreboot.org/c/coreboot/+/37887/3/src/mainboard/intel/tglrvp/... PS3, Line 93: On
on? enabled?
On
https://review.coreboot.org/c/coreboot/+/37887/3/src/mainboard/intel/tglrvp/... PS3, Line 133:
One space.
Ack
https://review.coreboot.org/c/coreboot/+/37887/3/src/mainboard/intel/tglrvp/... PS3, Line 146:
Remove.
Ack