Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37887 )
Change subject: soc/intel/tglrvp: Add ACPI support for CNVi ......................................................................
Patch Set 10:
(4 comments)
https://review.coreboot.org/c/coreboot/+/37887/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37887/10//COMMIT_MSG@7 PS10, Line 7: tglrvp That's not correct.
https://review.coreboot.org/c/coreboot/+/37887/10/src/soc/intel/tigerlake/ac... File src/soc/intel/tigerlake/acpi/cnvi.asl:
https://review.coreboot.org/c/coreboot/+/37887/10/src/soc/intel/tigerlake/ac... PS10, Line 27: // : // Define a Memory Region that will allow access to the CNVi WiFi PCI Configuration Space : // Please use: /* * */
for multi-line comments here and in the rest of the file.
https://review.coreboot.org/c/coreboot/+/37887/10/src/soc/intel/tigerlake/ac... PS10, Line 75: _RST This was not required on any of the previous platforms. Why is this required for TGL?
https://review.coreboot.org/c/coreboot/+/37887/10/src/soc/intel/tigerlake/ac... PS10, Line 136: AOLD Who calls this method? Can you please provide a pointer to that?
-
Furquan Shaikh (Code Review)