Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39111 )
Change subject: soc/intel/tigerlake: Add Jasper lake GPIO support ......................................................................
Patch Set 11:
(6 comments)
https://review.coreboot.org/c/coreboot/+/39111/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/gpio_jsl.asl:
https://review.coreboot.org/c/coreboot/+/39111/2/src/soc/intel/tigerlake/acp... PS2, Line 20:
Yes Karthik, I'll check this and work on it.
Done
https://review.coreboot.org/c/coreboot/+/39111/2/src/soc/intel/tigerlake/acp... PS2, Line 31: Memory32Fixed (ReadWrite, 0, 0, COM2)
Do we want to export ACPI objects for GPP_GPD group? I don't see it for TGL. […]
No, this is not required. I have removed it from asl file.
https://review.coreboot.org/c/coreboot/+/39111/2/src/soc/intel/tigerlake/acp... PS2, Line 40: /* GPIO Community 0 */ : CreateDWordField (^RBUF, ^COM0._BAS, BAS0) : CreateDWordField (^RBUF, ^COM0._LEN, LEN0) : Store (^^PCRB (PID_GPIOCOM0), BAS0) : Store (GPIO_BASE_SIZE, LEN0) : : /* GPIO Community 1 */ : CreateDWordField (^RBUF, ^COM1._BAS, BAS1) : CreateDWordField (^RBUF, ^COM1._LEN, LEN1) : Store (^^PCRB (PID_GPIOCOM1), BAS1) : Store (GPIO_BASE_SIZE, LEN1) : : /* GPIO Community 2 */ : CreateDWordField (^RBUF, ^COM2._BAS, BAS2) : CreateDWordField (^RBUF, ^COM2._LEN, LEN2) : Store (^^PCRB (PID_GPIOCOM2), BAS2) : Store (GPIO_BASE_SIZE, LEN2) : : /* GPIO Community 4 */ : CreateDWordField (^RBUF, ^COM4._BAS, BAS4) : CreateDWordField (^RBUF, ^COM4._LEN, LEN4) : Store (^^PCRB (PID_GPIOCOM4), BAS4) : Store (GPIO_BASE_SIZE, LEN4) : : /* GPIO Community 5 */ : CreateDWordField (^RBUF, ^COM5._BAS, BAS5) : CreateDWordField (^RBUF, ^COM5._LEN, LEN5) : Store (^^PCRB (PID_GPIOCOM5), BAS5) : Store (GPIO_BASE_SIZE, LEN5)
Let me check this. since I am on vacation I may take a day or two to correct it.
Done
https://review.coreboot.org/c/coreboot/+/39111/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/gpio_op.asl:
PS2:
+1
Done
https://review.coreboot.org/c/coreboot/+/39111/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/gpio_tgl.asl:
https://review.coreboot.org/c/coreboot/+/39111/2/src/soc/intel/tigerlake/acp... PS2, Line 84: BAS4 = ^^PCRB (PID_GPIOCOM4)
Similar to what I mentioned in the gpio_jsl. […]
Done
https://review.coreboot.org/c/coreboot/+/39111/2/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/gpio_soc_defs_jsl.h:
https://review.coreboot.org/c/coreboot/+/39111/2/src/soc/intel/tigerlake/inc... PS2, Line 67: GPIO_RSVD_0
These are soc reserved gpio pins that's why name is reserved.
Done
-
Maulik V Vaghela (Code Review)