Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39113 )
Change subject: soc/intel/tigerlake: Add Jasper Lake GPIO definitions ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/c/coreboot/+/39113/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/39113/2/src/soc/intel/tigerlake/acp... PS2, Line 15: : #if CONFIG(SOC_INTEL_TIGERLAKE) : #include "gpio_tgl.asl" : #else : #include "gpio_jsl.asl" : #endif These two files are nearly identical. The only differences I see are:
1) the _HID for the GPIO controllers 2) The GADD Method.
Would it make sense to factor these out into very small macros, so that the rest of the ASL is the same?
https://review.coreboot.org/c/coreboot/+/39113/2/src/soc/intel/tigerlake/gpi... File src/soc/intel/tigerlake/gpio_jsl.c:
https://review.coreboot.org/c/coreboot/+/39113/2/src/soc/intel/tigerlake/gpi... PS2, Line 125: , Just to clarify, TGL does not have a COMM_3?
https://review.coreboot.org/c/coreboot/+/39113/2/src/soc/intel/tigerlake/gpi... PS2, Line 183: GPP_GPD nit: align this with the others
https://review.coreboot.org/c/coreboot/+/39113/2/src/soc/intel/tigerlake/gpi... PS2, Line 185: TODO file a bug?