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 12:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39111/12/src/soc/intel/common/acpi/... File src/soc/intel/common/acpi/gpio_op.asl:
https://review.coreboot.org/c/coreboot/+/39111/12/src/soc/intel/common/acpi/... PS12, Line 4: Copyright (C) 2020 Intel Corporation.
Please use "Copyright 2020 The coreboot project Authors. […]
Done I am updating copyright line, for SPDX license header can we push separate patch for all files?
https://review.coreboot.org/c/coreboot/+/39111/12/src/soc/intel/common/acpi/... PS12, Line 76: /* : * Set Pad mode : * Arg0 - GPIO Number : * Arg1 - Pad mode : * 0 = GPIO control pad : * 1 = Native Function 1 : * 2 = Native Function 2 : * 3 = Native Function 3 : */ : Method (GPMO, 2, Serialized) : { : OperationRegion (PREG, SystemMemory, GADD (Arg0), 4) : Field (PREG, AnyAcc, NoLock, Preserve) : { : VAL0, 32 : } : Store (VAL0, Local0) : And (Not (GPIOPADMODE_MASK), Local0, Local0) : And (ShiftLeft (Arg1, GPIOPADMODE_SHIFT, Arg1), GPIOPADMODE_MASK, Arg1) : Or (Local0, Arg1, VAL0) : } : : /* : * Enable/Disable Tx buffer : * Arg0 - GPIO Number : * Arg1 - TxBuffer state : * 0 = Disable Tx Buffer : * 1 = Enable Tx Buffer : */ : Method (GTXE, 2, Serialized) : { : OperationRegion (PREG, SystemMemory, GADD (Arg0), 4) : Field (PREG, AnyAcc, NoLock, Preserve) : { : VAL0, 32 : } : : If (LEqual (Arg1, 1)) { : And (Not (GPIOTXBUFDIS_MASK), VAL0, VAL0) : } ElseIf (LEqual (Arg1, 0)){ : Or (GPIOTXBUFDIS_MASK, VAL0, VAL0) : } : } : : /* : * Enable/Disable Rx buffer : * Arg0 - GPIO Number : * Arg1 - RxBuffer state : * 0 = Disable Rx Buffer : * 1 = Enable Rx Buffer : */ : Method (GRXE, 2, Serialized) : { : OperationRegion (PREG, SystemMemory, GADD (Arg0), 4) : Field (PREG, AnyAcc, NoLock, Preserve) : { : VAL0, 32 : } : : If (LEqual (Arg1, 1)) { : And (Not (GPIORXBUFDIS_MASK), VAL0, VAL0) : } ElseIf (LEqual (Arg1, 0)){ : Or (GPIORXBUFDIS_MASK, VAL0, VAL0) : } : }
Looks like these got added when moving code into gpio_op.asl. […]
Yes Furquan, we're planning to use it for camera related ASL files.
https://review.coreboot.org/c/coreboot/+/39111/12/src/soc/intel/tigerlake/ac... File src/soc/intel/tigerlake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/39111/12/src/soc/intel/tigerlake/ac... PS12, Line 122: rg0 >= GPP_B0 && Arg0 <= GPP_A24)
Actually all the ASL files in this CL seems to be written using legacy ASL. […]
Since this file was copied from earlier project like icelake, it was in legacy asl format. I have updated it to use ASL 2.0. Ack and Done
https://review.coreboot.org/c/coreboot/+/39111/12/src/soc/intel/tigerlake/ac... PS12, Line 21: Device (GPIO)
Why was this device changed? This is going to break compatibility with Linux kernel driver for handl […]
Done
https://review.coreboot.org/c/coreboot/+/39111/12/src/soc/intel/tigerlake/ac... PS12, Line 90: /* GPIO Community 4 */
Here GPIO community 2 is removed for TGL. […]
Done
-
Maulik V Vaghela (Code Review)