Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38154 )
Change subject: soc/intel/{apl,cnl,icl,skl,tgl}: Clean up SA ASL code ......................................................................
Patch Set 5:
(5 comments)
Sorry about the late comments. Somehow I had missed posting these earlier. I just looked at CNL northbridge.asl file and noticed some differences compared to EDS.
https://review.coreboot.org/c/coreboot/+/38154/4/src/soc/intel/cannonlake/ac... File src/soc/intel/cannonlake/acpi/northbridge.asl:
https://review.coreboot.org/c/coreboot/+/38154/4/src/soc/intel/cannonlake/ac... PS4, Line 43: 31:15 As per EDS, isn't this 38:15?
https://review.coreboot.org/c/coreboot/+/38154/4/src/soc/intel/cannonlake/ac... PS4, Line 49: 31:26 Isn't this supposed to be 38:28?
https://review.coreboot.org/c/coreboot/+/38154/4/src/soc/intel/cannonlake/ac... PS4, Line 54: 31:12 38:12?
https://review.coreboot.org/c/coreboot/+/38154/4/src/soc/intel/cannonlake/ac... PS4, Line 243: 26 I don't think this is correct. It should be 28?
https://review.coreboot.org/c/coreboot/+/38154/4/src/soc/intel/cannonlake/ac... PS4, Line 257: ShiftLeft (_SB.PCI0.MCHC.DIBR, 12, Local0) : Return (Local0) Wondering if it would be worth switching to ASL2.0 syntax?