Shaunak Saha has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: TGL: Make the GPIO asl table in sync with kernel ......................................................................
TGL: Make the GPIO asl table in sync with kernel
Kernel pinctrl driver changed for Tiger lake and went to old scheme. Kernel patch: https://chromium-review.googlesource.com/c/chromiumos/ third_party/kernel/+/2116670
BRANCH=none TEST=Build and boot tgl board. In /sys/kernel/debug/pinctrl verify INTC34C5:00 listing all the pins.
Change-Id: I9f1d399ff7380125ad5b935f9590a7d9cc442b04 Signed-off-by: Shaunak Saha shaunak.saha@intel.com --- M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/ripto/gpio.c M src/soc/intel/tigerlake/acpi/gpio.asl M src/soc/intel/tigerlake/include/soc/gpio.h 4 files changed, 149 insertions(+), 101 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/39801/1
diff --git a/src/mainboard/google/volteer/variants/baseboard/gpio.c b/src/mainboard/google/volteer/variants/baseboard/gpio.c index fff0381..b332856 100644 --- a/src/mainboard/google/volteer/variants/baseboard/gpio.c +++ b/src/mainboard/google/volteer/variants/baseboard/gpio.c @@ -459,8 +459,8 @@ }
static const struct cros_gpio cros_gpios[] = { - CROS_GPIO_REC_AL(CROS_GPIO_VIRTUAL, CROS_GPIO_COMM1_NAME), - CROS_GPIO_WP_AH(GPIO_PCH_WP, CROS_GPIO_COMM1_NAME), + CROS_GPIO_REC_AL(CROS_GPIO_VIRTUAL, CROS_GPIO_DEVICE_NAME), + CROS_GPIO_WP_AH(GPIO_PCH_WP, CROS_GPIO_DEVICE_NAME), };
const struct cros_gpio *__weak variant_cros_gpios(size_t *num) diff --git a/src/mainboard/google/volteer/variants/ripto/gpio.c b/src/mainboard/google/volteer/variants/ripto/gpio.c index 4159cb8..290aa3b 100644 --- a/src/mainboard/google/volteer/variants/ripto/gpio.c +++ b/src/mainboard/google/volteer/variants/ripto/gpio.c @@ -455,8 +455,8 @@ }
static const struct cros_gpio cros_gpios[] = { - CROS_GPIO_REC_AL(CROS_GPIO_VIRTUAL, CROS_GPIO_COMM0_NAME), - CROS_GPIO_WP_AH(GPIO_PCH_WP, CROS_GPIO_COMM0_NAME), + CROS_GPIO_REC_AL(CROS_GPIO_VIRTUAL, CROS_GPIO_DEVICE_NAME), + CROS_GPIO_WP_AH(GPIO_PCH_WP, CROS_GPIO_DEVICE_NAME), };
const struct cros_gpio *variant_cros_gpios(size_t *num) diff --git a/src/soc/intel/tigerlake/acpi/gpio.asl b/src/soc/intel/tigerlake/acpi/gpio.asl index 9bf0c60..344a096 100644 --- a/src/soc/intel/tigerlake/acpi/gpio.asl +++ b/src/soc/intel/tigerlake/acpi/gpio.asl @@ -17,101 +17,153 @@ #include <intelblocks/gpio.h> #include "gpio_op.asl"
-Device (GCM0) -{ - Name (_HID, CROS_GPIO_NAME) - Name (_UID, 0) - Name (_DDN, "GPIO Controller Community 0") - - Name (RBUF, ResourceTemplate() +#if CONFIG(SOC_INTEL_TIGERLAKE) + Device (GPIO) { - Memory32Fixed (ReadWrite, 0, GPIO_BASE_SIZE, COM0) - Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) + Name (_HID, "INT34C5") + Name (_UID, 0) + Name (_DDN, "GPIO Controller") + + Name (RBUF, ResourceTemplate() + { + Memory32Fixed (ReadWrite, 0, 0, COM0) + Memory32Fixed (ReadWrite, 0, 0, COM1) + Memory32Fixed (ReadWrite, 0, 0, COM4) + Memory32Fixed (ReadWrite, 0, 0, COM5) + Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) + { GPIO_IRQ14 } + }) + Method (_CRS, 0, NotSerialized) + { + /* GPIO Community 0 */ + CreateDWordField (^RBUF, ^COM0._BAS, BAS0) + CreateDWordField (^RBUF, ^COM0._LEN, LEN0) + BAS0 = ^^PCRB (PID_GPIOCOM0) + LEN0 = GPIO_BASE_SIZE + + /* GPIO Community 1 */ + CreateDWordField (^RBUF, ^COM1._BAS, BAS1) + CreateDWordField (^RBUF, ^COM1._LEN, LEN1) + BAS1 = ^^PCRB (PID_GPIOCOM1) + LEN1 = GPIO_BASE_SIZE + + /* GPIO Community 4 */ + CreateDWordField (^RBUF, ^COM4._BAS, BAS4) + CreateDWordField (^RBUF, ^COM4._LEN, LEN4) + BAS4 = ^^PCRB (PID_GPIOCOM4) + LEN4 = GPIO_BASE_SIZE + + /* GPIO Community 5 */ + CreateDWordField (^RBUF, ^COM5._BAS, BAS5) + CreateDWordField (^RBUF, ^COM5._LEN, LEN5) + BAS5 = ^^PCRB (PID_GPIOCOM5) + LEN5 = GPIO_BASE_SIZE + + Return (RBUF) + } + + Method (_STA, 0, NotSerialized) + { + Return (0xF) + } + } +#else + Device (GCM0) + { + Name (_HID, CROS_GPIO_NAME) + Name (_UID, 0) + Name (_DDN, "GPIO Controller Community 0") + + Name (RBUF, ResourceTemplate() + { + Memory32Fixed (ReadWrite, 0, GPIO_BASE_SIZE, COM0) + Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) + { GPIO_IRQ14 } + }) + Method (_CRS, 0, NotSerialized) + { + CreateDWordField (^RBUF, ^COM0._BAS, BAS0) + BAS0 = ^^PCRB (PID_GPIOCOM0) + Return (^RBUF) + } + Method (_STA) + { + Return (0xF) + } + } + + Device (GCM1) + { + Name (_HID, CROS_GPIO_NAME) + Name (_UID, 1) + Name (_DDN, "GPIO Controller Community 1") + + Name (RBUF, ResourceTemplate() + { + Memory32Fixed (ReadWrite, 0, GPIO_BASE_SIZE, COM1) + Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) + { GPIO_IRQ14 } + }) + Method (_CRS, 0, NotSerialized) + { + CreateDWordField (^RBUF, ^COM1._BAS, BAS1) + BAS1 = ^^PCRB (PID_GPIOCOM1) + Return (^RBUF) + } + Method (_STA) + { + Return (0xF) + } + } + + Device (GCM4) + { + Name (_HID, CROS_GPIO_NAME) + Name (_UID, 4) + Name (_DDN, "GPIO Controller Community 4") + + Name (RBUF, ResourceTemplate() + { + Memory32Fixed (ReadWrite, 0, GPIO_BASE_SIZE, COM4) + Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) + { GPIO_IRQ14 } + }) + Method (_CRS, 0, NotSerialized) + { + CreateDWordField (^RBUF, ^COM4._BAS, BAS4) + BAS4 = ^^PCRB (PID_GPIOCOM4) + Return (^RBUF) + } + Method (_STA) + { + Return (0xF) + } + } + + Device (GCM5) + { + Name (_HID, CROS_GPIO_NAME) + Name (_UID, 5) + Name (_DDN, "GPIO Controller Community 5") + + Name (RBUF, ResourceTemplate() + { + Memory32Fixed (ReadWrite, 0, GPIO_BASE_SIZE, COM5) + Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) { GPIO_IRQ14 } - }) - Method (_CRS, 0, NotSerialized) - { - CreateDWordField (^RBUF, ^COM0._BAS, BAS0) - BAS0 = ^^PCRB (PID_GPIOCOM0) - Return (^RBUF) + }) + Method (_CRS, 0, NotSerialized) + { + CreateDWordField (^RBUF, ^COM5._BAS, BAS5) + BAS5 = ^^PCRB (PID_GPIOCOM5) + Return (^RBUF) + } + Method (_STA) + { + Return (0xF) + } } - Method (_STA) - { - Return (0xF) - } -} - -Device (GCM1) -{ - Name (_HID, CROS_GPIO_NAME) - Name (_UID, 1) - Name (_DDN, "GPIO Controller Community 1") - - Name (RBUF, ResourceTemplate() - { - Memory32Fixed (ReadWrite, 0, GPIO_BASE_SIZE, COM1) - Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) - { GPIO_IRQ14 } - }) - Method (_CRS, 0, NotSerialized) - { - CreateDWordField (^RBUF, ^COM1._BAS, BAS1) - BAS1 = ^^PCRB (PID_GPIOCOM1) - Return (^RBUF) - } - Method (_STA) - { - Return (0xF) - } -} - -Device (GCM4) -{ - Name (_HID, CROS_GPIO_NAME) - Name (_UID, 4) - Name (_DDN, "GPIO Controller Community 4") - - Name (RBUF, ResourceTemplate() - { - Memory32Fixed (ReadWrite, 0, GPIO_BASE_SIZE, COM4) - Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) - { GPIO_IRQ14 } - }) - Method (_CRS, 0, NotSerialized) - { - CreateDWordField (^RBUF, ^COM4._BAS, BAS4) - BAS4 = ^^PCRB (PID_GPIOCOM4) - Return (^RBUF) - } - Method (_STA) - { - Return (0xF) - } -} - -Device (GCM5) -{ - Name (_HID, CROS_GPIO_NAME) - Name (_UID, 5) - Name (_DDN, "GPIO Controller Community 5") - - Name (RBUF, ResourceTemplate() - { - Memory32Fixed (ReadWrite, 0, GPIO_BASE_SIZE, COM5) - Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) - { GPIO_IRQ14 } - }) - Method (_CRS, 0, NotSerialized) - { - CreateDWordField (^RBUF, ^COM5._BAS, BAS5) - BAS5 = ^^PCRB (PID_GPIOCOM5) - Return (^RBUF) - } - Method (_STA) - { - Return (0xF) - } -} +#endif
/* * Get GPIO DW0 Address diff --git a/src/soc/intel/tigerlake/include/soc/gpio.h b/src/soc/intel/tigerlake/include/soc/gpio.h index 7a6df7c..dee59a3 100644 --- a/src/soc/intel/tigerlake/include/soc/gpio.h +++ b/src/soc/intel/tigerlake/include/soc/gpio.h @@ -20,11 +20,7 @@
#if CONFIG(SOC_INTEL_TIGERLAKE)
- #define CROS_GPIO_NAME "INT34C5" - #define CROS_GPIO_COMM0_NAME "INT34C5:00" - #define CROS_GPIO_COMM1_NAME "INT34C5:01" - #define CROS_GPIO_COMM4_NAME "INT34C5:02" - #define CROS_GPIO_COMM5_NAME "INT34C5:03" + #define CROS_GPIO_DEVICE_NAME "INT34C5:00"
#elif CONFIG(SOC_INTEL_JASPERLAKE)
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39801
to look at the new patch set (#2).
Change subject: [WIP]TGL: Make the GPIO asl table in sync with kernel ......................................................................
[WIP]TGL: Make the GPIO asl table in sync with kernel
Kernel pinctrl driver changed for Tiger lake and went to old scheme. Kernel patch: https://chromium-review.googlesource.com/c/chromiumos/ third_party/kernel/+/2116670
BRANCH=none TEST=Build and boot tgl board. In /sys/kernel/debug/pinctrl verify INTC34C5:00 listing all the pins.
Change-Id: I9f1d399ff7380125ad5b935f9590a7d9cc442b04 Signed-off-by: Shaunak Saha shaunak.saha@intel.com --- M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/ripto/gpio.c M src/soc/intel/tigerlake/acpi/gpio.asl M src/soc/intel/tigerlake/include/soc/gpio.h 4 files changed, 149 insertions(+), 101 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/39801/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: [WIP]TGL: Make the GPIO asl table in sync with kernel ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39801/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/39801/2/src/soc/intel/tigerlake/acp... PS2, Line 20: #if CONFIG(SOC_INTEL_TIGERLAKE) There are some changes happening for JSL GPIO as well here: CB:39470
I think we should probably just split TGL and JSL SoCs(CB:39774) first and then take this change in. Adding Karthik and Aamir.
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: [WIP]TGL: Make the GPIO asl table in sync with kernel ......................................................................
Patch Set 2:
I would make this patch dependent on https://review.coreboot.org/c/coreboot/+/39470/ an also add Cq-Depend: chromium:2116670 in commit message.
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: [WIP]TGL: Make the GPIO asl table in sync with kernel ......................................................................
Patch Set 2:
Doing some test on this patch with the kernel. I need to pull back that old style 32 bit padding for each community in gpio.c or else the mapping is not correct with kernel.
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: [WIP]TGL: Make the GPIO asl table in sync with kernel ......................................................................
Patch Set 2:
Still waiting for the results to come back from the OS team.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: [WIP]TGL: Make the GPIO asl table in sync with kernel ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/c/coreboot/+/39801/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39801/2//COMMIT_MSG@7 PS2, Line 7: [WIP]TGL: Make the GPIO asl table in sync with kernel Maybe:
soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel
https://review.coreboot.org/c/coreboot/+/39801/2//COMMIT_MSG@9 PS2, Line 9: lake Lake
https://review.coreboot.org/c/coreboot/+/39801/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/39801/2/src/soc/intel/tigerlake/acp... PS2, Line 20: #if CONFIG(SOC_INTEL_TIGERLAKE) This is not mentioned in the commit message.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: [WIP]TGL: Make the GPIO asl table in sync with kernel ......................................................................
Patch Set 2:
Patch Set 2:
Still waiting for the results to come back from the OS team.
What's the latest here? Who on the OS team is supposed to provide the results? Can you please post updates on the bug?
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: [WIP]TGL: Make the GPIO asl table in sync with kernel ......................................................................
Patch Set 2:
(4 comments)
Patch Set 2:
Patch Set 2:
Still waiting for the results to come back from the OS team.
What's the latest here? Who on the OS team is supposed to provide the results? Can you please post updates on the bug?
Chintan's team is working on testing those. Also I made some local changes based on Aamir's chnage where they have 2 asl files(for tgl and jsl). but their recent version they seems to have back to 1 single file. So i am readjusting those change and checking on my side again. Will push the final change from cb side by tomorrow.
https://review.coreboot.org/c/coreboot/+/39801/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39801/2//COMMIT_MSG@7 PS2, Line 7: [WIP]TGL: Make the GPIO asl table in sync with kernel
Maybe: […]
Ack
https://review.coreboot.org/c/coreboot/+/39801/2//COMMIT_MSG@9 PS2, Line 9: lake
Lake
Ack
https://review.coreboot.org/c/coreboot/+/39801/2/src/soc/intel/tigerlake/acp... File src/soc/intel/tigerlake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/39801/2/src/soc/intel/tigerlake/acp... PS2, Line 20: #if CONFIG(SOC_INTEL_TIGERLAKE)
There are some changes happening for JSL GPIO as well here: CB:39470 […]
Ack
https://review.coreboot.org/c/coreboot/+/39801/2/src/soc/intel/tigerlake/acp... PS2, Line 20: #if CONFIG(SOC_INTEL_TIGERLAKE)
This is not mentioned in the commit message.
Ack
Hello build bot (Jenkins), Furquan Shaikh, Caveh Jalali, Tim Wawrzynczak, Nick Vaccaro, Aamir Bohra, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39801
to look at the new patch set (#3).
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel
Kernel pinctrl driver changed for Tiger Lake and went to old scheme. Kernel patch: https://chromium-review.googlesource.com/c/chromiumos/ third_party/kernel/+/2116670
BRANCH=none TEST=Build and boot tgl board. In /sys/kernel/debug/pinctrl verify INTC34C5:00 listing all the pins.
Change-Id: I9f1d399ff7380125ad5b935f9590a7d9cc442b04 Signed-off-by: Shaunak Saha shaunak.saha@intel.com --- M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/ripto/gpio.c M src/soc/intel/tigerlake/acpi/gpio.asl M src/soc/intel/tigerlake/gpio.c M src/soc/intel/tigerlake/include/soc/gpio.h 5 files changed, 55 insertions(+), 95 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/39801/3
Hello build bot (Jenkins), Furquan Shaikh, Caveh Jalali, Tim Wawrzynczak, Nick Vaccaro, Aamir Bohra, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39801
to look at the new patch set (#4).
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel
Kernel pinctrl driver changed for Tiger Lake and went to old scheme. Kernel patch: https://chromium-review.googlesource.com/c/chromiumos/ third_party/kernel/+/2116670
BRANCH=none TEST=Build and boot tgl board. In /sys/kernel/debug/pinctrl verify INTC34C5:00 listing all the pins.
Change-Id: I9f1d399ff7380125ad5b935f9590a7d9cc442b04 Signed-off-by: Shaunak Saha shaunak.saha@intel.com --- M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/ripto/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c M src/soc/intel/tigerlake/acpi/gpio.asl M src/soc/intel/tigerlake/gpio.c M src/soc/intel/tigerlake/include/soc/gpio.h 7 files changed, 57 insertions(+), 97 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/39801/4
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 4:
Build is still unstable for boards DELTAN and DELTAUR.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 4:
In file included from src/mainboard/google/deltaur/variants/baseboard/include/baseboard/variants.h:12, from src/mainboard/google/deltaur/variants/baseboard/gpio.c:8: src/mainboard/google/deltaur/variants/baseboard/gpio.c:448:34: error: 'CROS_GPIO_NAME' undeclared here (not in a function); did you mean 'CROS_GPIO_PE'? CROS_GPIO_REC_AL(GPIO_REC_MODE, CROS_GPIO_NAME), ^~~~~~~~~~~~~~ src/vendorcode/google/chromeos/chromeos.h:103:14: note: in definition of macro 'CROS_GPIO_INITIALIZER' .device = (dev), \ ^~~ src/vendorcode/google/chromeos/chromeos.h:110:2: note: in expansion of macro 'CROS_GPIO_REC_INITIALIZER' CROS_GPIO_REC_INITIALIZER(CROS_GPIO_ACTIVE_LOW, num, dev) ^~~~~~~~~~~~~~~~~~~~~~~~~ src/mainboard/google/deltaur/variants/baseboard/gpio.c:448:2: note: in expansion of macro 'CROS_GPIO_REC_AL' CROS_GPIO_REC_AL(GPIO_REC_MODE, CROS_GPIO_NAME),
You'll need to modify src/mainboard/google/deltaur/variants/baseboard/gpio.c as well.
Hello build bot (Jenkins), Furquan Shaikh, Caveh Jalali, Tim Wawrzynczak, Nick Vaccaro, Aamir Bohra, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39801
to look at the new patch set (#5).
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel
Kernel pinctrl driver changed for Tiger Lake and went to old scheme. Kernel patch: https://chromium-review.googlesource.com/c/chromiumos/ third_party/kernel/+/2116670
BRANCH=none TEST=Build and boot tgl board. In /sys/kernel/debug/pinctrl verify INTC34C5:00 listing all the pins.
Change-Id: I9f1d399ff7380125ad5b935f9590a7d9cc442b04 Signed-off-by: Shaunak Saha shaunak.saha@intel.com --- M src/mainboard/google/deltaur/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/ripto/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c M src/soc/intel/tigerlake/acpi/gpio.asl M src/soc/intel/tigerlake/gpio.c M src/soc/intel/tigerlake/include/soc/gpio.h 8 files changed, 59 insertions(+), 99 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/39801/5
Hello build bot (Jenkins), Furquan Shaikh, Caveh Jalali, Tim Wawrzynczak, Nick Vaccaro, Aamir Bohra, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39801
to look at the new patch set (#6).
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel
Kernel pinctrl driver changed for Tiger Lake and went to old scheme. Kernel patch: https://chromium-review.googlesource.com/c/chromiumos/ third_party/kernel/+/2116670
BRANCH=none Cq-Depend:chromium:2116670 TEST=Build and boot tgl board. In /sys/kernel/debug/pinctrl verify INTC34C5:00 listing all the pins.
Change-Id: I9f1d399ff7380125ad5b935f9590a7d9cc442b04 Signed-off-by: Shaunak Saha shaunak.saha@intel.com --- M src/mainboard/google/deltaur/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/ripto/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c M src/soc/intel/tigerlake/acpi/gpio.asl M src/soc/intel/tigerlake/gpio.c M src/soc/intel/tigerlake/include/soc/gpio.h 8 files changed, 59 insertions(+), 99 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/39801/6
Hello Bora Guvendik, build bot (Jenkins), Furquan Shaikh, Selma Bensaid, Caveh Jalali, Tim Wawrzynczak, Nick Vaccaro, Aamir Bohra, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39801
to look at the new patch set (#7).
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel
Kernel pinctrl driver changed for Tiger Lake and went to old scheme. Kernel patch: https://chromium-review.googlesource.com/c/chromiumos/ third_party/kernel/+/2116670
BRANCH=none Cq-Depend:chromium:2116670 TEST=Build and boot tgl board. In /sys/kernel/debug/pinctrl verify INTC34C5:00 listing all the pins.
Change-Id: I9f1d399ff7380125ad5b935f9590a7d9cc442b04 Signed-off-by: Shaunak Saha shaunak.saha@intel.com --- M src/mainboard/google/deltaur/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/ripto/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c M src/soc/intel/tigerlake/acpi/gpio.asl M src/soc/intel/tigerlake/gpio.c M src/soc/intel/tigerlake/include/soc/gpio.h 8 files changed, 59 insertions(+), 99 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/39801/7
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39801/7/src/soc/intel/tigerlake/gpi... File src/soc/intel/tigerlake/gpio.c:
https://review.coreboot.org/c/coreboot/+/39801/7/src/soc/intel/tigerlake/gpi... PS7, Line 53: GPP_A Comments don't match the groups.
https://review.coreboot.org/c/coreboot/+/39801/7/src/soc/intel/tigerlake/gpi... PS7, Line 74: INTEL_GPP_BASE(GPP_C0, GPP_L_BKLTEN, GPP_MLK_RSTB, 320), /* GPP_HVCMOS */ : INTEL_GPP_BASE(GPP_C0, GPP_E0, GPP_E_CLK_LOOPBK, 352), /* GPP_E */ This doesn't match what the kernel is doing: https://github.com/torvalds/linux/blob/master/drivers/pinctrl/intel/pinctrl-...
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39801/7/src/soc/intel/tigerlake/gpi... File src/soc/intel/tigerlake/gpio.c:
https://review.coreboot.org/c/coreboot/+/39801/7/src/soc/intel/tigerlake/gpi... PS7, Line 53: GPP_A
Comments don't match the groups.
Will fix.
https://review.coreboot.org/c/coreboot/+/39801/7/src/soc/intel/tigerlake/gpi... PS7, Line 74: INTEL_GPP_BASE(GPP_C0, GPP_L_BKLTEN, GPP_MLK_RSTB, 320), /* GPP_HVCMOS */ : INTEL_GPP_BASE(GPP_C0, GPP_E0, GPP_E_CLK_LOOPBK, 352), /* GPP_E */
This doesn't match what the kernel is doing: https://github. […]
INTEL_GPP should be good for these 2 then?
Hello Bora Guvendik, build bot (Jenkins), Furquan Shaikh, Selma Bensaid, Caveh Jalali, Duncan Laurie, Tim Wawrzynczak, Nick Vaccaro, Aamir Bohra, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39801
to look at the new patch set (#8).
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel
Kernel pinctrl driver changed for Tiger Lake and went to old scheme. Kernel patch: https://chromium-review.googlesource.com/c/chromiumos/ third_party/kernel/+/2116670
BRANCH=none Cq-Depend:chromium:2116670 TEST=Build and boot tgl board. In /sys/kernel/debug/pinctrl verify INTC34C5:00 listing all the pins.
Change-Id: I9f1d399ff7380125ad5b935f9590a7d9cc442b04 Signed-off-by: Shaunak Saha shaunak.saha@intel.com --- M src/mainboard/google/deltaur/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/ripto/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c M src/soc/intel/tigerlake/acpi/gpio.asl M src/soc/intel/tigerlake/gpio.c M src/soc/intel/tigerlake/include/soc/gpio.h 8 files changed, 56 insertions(+), 96 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/39801/8
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39801/7/src/soc/intel/tigerlake/gpi... File src/soc/intel/tigerlake/gpio.c:
https://review.coreboot.org/c/coreboot/+/39801/7/src/soc/intel/tigerlake/gpi... PS7, Line 74: INTEL_GPP_BASE(GPP_C0, GPP_L_BKLTEN, GPP_MLK_RSTB, 320), /* GPP_HVCMOS */ : INTEL_GPP_BASE(GPP_C0, GPP_E0, GPP_E_CLK_LOOPBK, 352), /* GPP_E */
INTEL_GPP should be good for these 2 then?
You will have to use INTEL_GPP for GPP_HVCMOS and GPP_JTAG. Also for GPP_SPI below. It would be good to check that the behavior is correct w.r.t. exporting ACPI GPIO for GPP_E from BIOS and its use in kernel.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 8: Code-Review+2
Hello Bora Guvendik, build bot (Jenkins), Furquan Shaikh, Selma Bensaid, Caveh Jalali, Duncan Laurie, Tim Wawrzynczak, Nick Vaccaro, Aamir Bohra, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39801
to look at the new patch set (#9).
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel
Kernel pinctrl driver changed for Tiger Lake and went to old scheme. Kernel patch: https://chromium-review.googlesource.com/c/chromiumos/ third_party/kernel/+/2116670
BRANCH=none Cq-Depend:chromium:2116670 TEST=Build and boot tgl board. In /sys/kernel/debug/pinctrl verify INTC34C5:00 listing all the pins.
Change-Id: I9f1d399ff7380125ad5b935f9590a7d9cc442b04 Signed-off-by: Shaunak Saha shaunak.saha@intel.com --- M src/mainboard/google/deltaur/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/ripto/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c M src/soc/intel/tigerlake/acpi/gpio.asl M src/soc/intel/tigerlake/gpio.c M src/soc/intel/tigerlake/include/soc/gpio.h 8 files changed, 57 insertions(+), 97 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/39801/9
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 9: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 9: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/39801/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39801/9//COMMIT_MSG@12 PS9, Line 12: BUG=?
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 9: Code-Review+2
Looking at the patches upstream it seems the change in how gpio communities are reported was also a surprise to them. How is this decided at Intel?
Hello Bora Guvendik, build bot (Jenkins), Furquan Shaikh, Selma Bensaid, Caveh Jalali, Duncan Laurie, Tim Wawrzynczak, Nick Vaccaro, Aamir Bohra, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39801
to look at the new patch set (#10).
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel
Kernel pinctrl driver changed for Tiger Lake and went to old scheme. Kernel patch: https://chromium-review.googlesource.com/c/chromiumos/ third_party/kernel/+/2116670
BUG=b:151683980 BRANCH=none Cq-Depend:chromium:2116670 TEST=Build and boot tgl board. In /sys/kernel/debug/pinctrl verify INTC34C5:00 listing all the pins.
Change-Id: I9f1d399ff7380125ad5b935f9590a7d9cc442b04 Signed-off-by: Shaunak Saha shaunak.saha@intel.com --- M src/mainboard/google/deltaur/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/ripto/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c M src/soc/intel/tigerlake/acpi/gpio.asl M src/soc/intel/tigerlake/gpio.c M src/soc/intel/tigerlake/include/soc/gpio.h 8 files changed, 57 insertions(+), 97 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/39801/10
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 10: Code-Review+2
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 10:
(2 comments)
Patch Set 9: Code-Review+2
Looking at the patches upstream it seems the change in how gpio communities are reported was also a surprise to them. How is this decided at Intel?
Yes. There were lots of changes on the kernel side. Eventually we will go to the newer schema maybe in next silicon. But for TGL the change was too late to do proper validation so Intel decided to use the older schema.
https://review.coreboot.org/c/coreboot/+/39801/9//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39801/9//COMMIT_MSG@12 PS9, Line 12:
BUG=?
Done
https://review.coreboot.org/c/coreboot/+/39801/7/src/soc/intel/tigerlake/gpi... File src/soc/intel/tigerlake/gpio.c:
https://review.coreboot.org/c/coreboot/+/39801/7/src/soc/intel/tigerlake/gpi... PS7, Line 74: INTEL_GPP_BASE(GPP_C0, GPP_L_BKLTEN, GPP_MLK_RSTB, 320), /* GPP_HVCMOS */ : INTEL_GPP_BASE(GPP_C0, GPP_E0, GPP_E_CLK_LOOPBK, 352), /* GPP_E */
You will have to use INTEL_GPP for GPP_HVCMOS and GPP_JTAG. Also for GPP_SPI below. […]
Done
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39801/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39801/10//COMMIT_MSG@15 PS10, Line 15: Cq-Depend:chromium:2116670 This needs to go just above Change-Id, or else the CQ bot will complain 😊
Hello Bora Guvendik, build bot (Jenkins), Furquan Shaikh, Selma Bensaid, Caveh Jalali, Tim Wawrzynczak, Duncan Laurie, Nick Vaccaro, Aamir Bohra, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39801
to look at the new patch set (#11).
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel
Kernel pinctrl driver changed for Tiger Lake and went to old scheme. Kernel patch: https://chromium-review.googlesource.com/c/chromiumos/ third_party/kernel/+/2116670
BUG=b:151683980 BRANCH=none TEST=Build and boot tgl board. In /sys/kernel/debug/pinctrl verify INTC34C5:00 listing all the pins. Cq-Depend:chromium:2116670
Change-Id: I9f1d399ff7380125ad5b935f9590a7d9cc442b04 Signed-off-by: Shaunak Saha shaunak.saha@intel.com --- M src/mainboard/google/deltaur/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/ripto/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c M src/soc/intel/tigerlake/acpi/gpio.asl M src/soc/intel/tigerlake/gpio.c M src/soc/intel/tigerlake/include/soc/gpio.h 8 files changed, 57 insertions(+), 97 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/39801/11
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39801/10//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39801/10//COMMIT_MSG@15 PS10, Line 15: Cq-Depend:chromium:2116670
This needs to go just above Change-Id, or else the CQ bot will complain 😊
Done
Shaunak Saha has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 11:
Furquan,Duncan. What is the plan for merging this one. if we merge this and kernel patch is not merged we would be seeing few regression 😞
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 11:
Patch Set 11:
Furquan,Duncan. What is the plan for merging this one. if we merge this and kernel patch is not merged we would be seeing few regression 😞
Basically, this is what needs to happen: 1. Submit this CL here 2. Tim/Nick can help cherry-pick this CL to chromium repo. 3. Once uploaded to chromium repo, use that CL ID and add it as Cq-Depend in kernel CL (That ensures circular dependency) 4. Hit Cq+2 on both.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 11:
Patch Set 11:
Patch Set 11:
Furquan,Duncan. What is the plan for merging this one. if we merge this and kernel patch is not merged we would be seeing few regression 😞
Basically, this is what needs to happen:
- Submit this CL here
- Tim/Nick can help cherry-pick this CL to chromium repo.
- Once uploaded to chromium repo, use that CL ID and add it as Cq-Depend in kernel CL (That ensures circular dependency)
- Hit Cq+2 on both.
We can send out a PSA as well, this would probably bite people who don't update their OS images frequently enough. (like me!)
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 11:
Patch Set 11:
Patch Set 11:
Patch Set 11:
Furquan,Duncan. What is the plan for merging this one. if we merge this and kernel patch is not merged we would be seeing few regression 😞
Basically, this is what needs to happen:
- Submit this CL here
- Tim/Nick can help cherry-pick this CL to chromium repo.
- Once uploaded to chromium repo, use that CL ID and add it as Cq-Depend in kernel CL (That ensures circular dependency)
- Hit Cq+2 on both.
We can send out a PSA as well, this would probably bite people who don't update their OS images frequently enough. (like me!)
Ah yes! That's a good idea.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 12:
Patch Set 11:
Patch Set 11:
Patch Set 11:
Patch Set 11:
Furquan,Duncan. What is the plan for merging this one. if we merge this and kernel patch is not merged we would be seeing few regression 😞
Basically, this is what needs to happen:
- Submit this CL here
- Tim/Nick can help cherry-pick this CL to chromium repo.
- Once uploaded to chromium repo, use that CL ID and add it as Cq-Depend in kernel CL (That ensures circular dependency)
- Hit Cq+2 on both.
We can send out a PSA as well, this would probably bite people who don't update their OS images frequently enough. (like me!)
Ah yes! That's a good idea.
Quick update, waiting for review of the corresponding kernel change.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39801/7/src/soc/intel/tigerlake/gpi... File src/soc/intel/tigerlake/gpio.c:
https://review.coreboot.org/c/coreboot/+/39801/7/src/soc/intel/tigerlake/gpi... PS7, Line 53: GPP_A
Will fix.
Done
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel
Kernel pinctrl driver changed for Tiger Lake and went to old scheme. Kernel patch: https://chromium-review.googlesource.com/c/chromiumos/ third_party/kernel/+/2116670
BUG=b:151683980 BRANCH=none TEST=Build and boot tgl board. In /sys/kernel/debug/pinctrl verify INTC34C5:00 listing all the pins. Cq-Depend:chromium:2116670
Change-Id: I9f1d399ff7380125ad5b935f9590a7d9cc442b04 Signed-off-by: Shaunak Saha shaunak.saha@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39801 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/deltaur/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/baseboard/gpio.c M src/mainboard/google/volteer/variants/ripto/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c M src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c M src/soc/intel/tigerlake/acpi/gpio.asl M src/soc/intel/tigerlake/gpio.c M src/soc/intel/tigerlake/include/soc/gpio.h 8 files changed, 57 insertions(+), 97 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/deltaur/variants/baseboard/gpio.c b/src/mainboard/google/deltaur/variants/baseboard/gpio.c index c56fff8..905a6d2 100644 --- a/src/mainboard/google/deltaur/variants/baseboard/gpio.c +++ b/src/mainboard/google/deltaur/variants/baseboard/gpio.c @@ -407,8 +407,8 @@ }
static const struct cros_gpio cros_gpios[] = { - CROS_GPIO_REC_AL(GPIO_REC_MODE, CROS_GPIO_NAME), - CROS_GPIO_WP_AH(GPIO_PCH_WP, CROS_GPIO_NAME), + CROS_GPIO_REC_AL(GPIO_REC_MODE, CROS_GPIO_DEVICE_NAME), + CROS_GPIO_WP_AH(GPIO_PCH_WP, CROS_GPIO_DEVICE_NAME), };
const struct cros_gpio *__weak variant_cros_gpios(size_t *num) diff --git a/src/mainboard/google/volteer/variants/baseboard/gpio.c b/src/mainboard/google/volteer/variants/baseboard/gpio.c index 37211ff..31ff3fc 100644 --- a/src/mainboard/google/volteer/variants/baseboard/gpio.c +++ b/src/mainboard/google/volteer/variants/baseboard/gpio.c @@ -459,8 +459,8 @@ }
static const struct cros_gpio cros_gpios[] = { - CROS_GPIO_REC_AL(CROS_GPIO_VIRTUAL, CROS_GPIO_COMM1_NAME), - CROS_GPIO_WP_AH(GPIO_PCH_WP, CROS_GPIO_COMM0_NAME), + CROS_GPIO_REC_AL(CROS_GPIO_VIRTUAL, CROS_GPIO_DEVICE_NAME), + CROS_GPIO_WP_AH(GPIO_PCH_WP, CROS_GPIO_DEVICE_NAME), };
const struct cros_gpio *__weak variant_cros_gpios(size_t *num) diff --git a/src/mainboard/google/volteer/variants/ripto/gpio.c b/src/mainboard/google/volteer/variants/ripto/gpio.c index 856f7fd..fcc1848 100644 --- a/src/mainboard/google/volteer/variants/ripto/gpio.c +++ b/src/mainboard/google/volteer/variants/ripto/gpio.c @@ -444,8 +444,8 @@ }
static const struct cros_gpio cros_gpios[] = { - CROS_GPIO_REC_AL(CROS_GPIO_VIRTUAL, CROS_GPIO_COMM0_NAME), - CROS_GPIO_WP_AH(GPIO_PCH_WP, CROS_GPIO_COMM0_NAME), + CROS_GPIO_REC_AL(CROS_GPIO_VIRTUAL, CROS_GPIO_DEVICE_NAME), + CROS_GPIO_WP_AH(GPIO_PCH_WP, CROS_GPIO_DEVICE_NAME), };
const struct cros_gpio *variant_cros_gpios(size_t *num) diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c index 79ffd38..5f36b0b 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/gpio.c @@ -105,7 +105,7 @@ }
static const struct cros_gpio cros_gpios[] = { - CROS_GPIO_REC_AL(CROS_GPIO_VIRTUAL, CROS_GPIO_COMM0_NAME), + CROS_GPIO_REC_AL(CROS_GPIO_VIRTUAL, CROS_GPIO_DEVICE_NAME), };
const struct cros_gpio *variant_cros_gpios(size_t *num) diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c index 27d610b..743d593 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/gpio.c @@ -102,7 +102,7 @@ }
static const struct cros_gpio cros_gpios[] = { - CROS_GPIO_REC_AL(CROS_GPIO_VIRTUAL, CROS_GPIO_COMM0_NAME), + CROS_GPIO_REC_AL(CROS_GPIO_VIRTUAL, CROS_GPIO_DEVICE_NAME), };
const struct cros_gpio *variant_cros_gpios(size_t *num) diff --git a/src/soc/intel/tigerlake/acpi/gpio.asl b/src/soc/intel/tigerlake/acpi/gpio.asl index 9b8a175..d1e4955 100644 --- a/src/soc/intel/tigerlake/acpi/gpio.asl +++ b/src/soc/intel/tigerlake/acpi/gpio.asl @@ -1,103 +1,57 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* This file is part of the coreboot project. */ +#include <intelblocks/gpio.h> #include <soc/gpio_defs.h> +#include <soc/intel/common/acpi/gpio.asl> #include <soc/irq.h> #include <soc/pcr_ids.h> -#include <intelblocks/gpio.h> -#include <soc/intel/common/acpi/gpio.asl> #include "gpio_op.asl"
-Device (GCM0) +Device (GPIO) { - Name (_HID, CROS_GPIO_NAME) + Name (_HID, "INT34C5") Name (_UID, 0) - Name (_DDN, "GPIO Controller Community 0") + Name (_DDN, "GPIO Controller")
Name (RBUF, ResourceTemplate() { - Memory32Fixed (ReadWrite, 0, GPIO_BASE_SIZE, COM0) + Memory32Fixed (ReadWrite, 0, 0, COM0) + Memory32Fixed (ReadWrite, 0, 0, COM1) + Memory32Fixed (ReadWrite, 0, 0, COM4) + Memory32Fixed (ReadWrite, 0, 0, COM5) Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) { GPIO_IRQ14 } }) Method (_CRS, 0, NotSerialized) { + /* GPIO Community 0 */ CreateDWordField (^RBUF, ^COM0._BAS, BAS0) + CreateDWordField (^RBUF, ^COM0._LEN, LEN0) BAS0 = ^^PCRB (PID_GPIOCOM0) - Return (^RBUF) - } - Method (_STA) - { - Return (0xF) - } -} + LEN0 = GPIO_BASE_SIZE
-Device (GCM1) -{ - Name (_HID, CROS_GPIO_NAME) - Name (_UID, 1) - Name (_DDN, "GPIO Controller Community 1") - - Name (RBUF, ResourceTemplate() - { - Memory32Fixed (ReadWrite, 0, GPIO_BASE_SIZE, COM1) - Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) - { GPIO_IRQ14 } - }) - Method (_CRS, 0, NotSerialized) - { + /* GPIO Community 1 */ CreateDWordField (^RBUF, ^COM1._BAS, BAS1) + CreateDWordField (^RBUF, ^COM1._LEN, LEN1) BAS1 = ^^PCRB (PID_GPIOCOM1) - Return (^RBUF) - } - Method (_STA) - { - Return (0xF) - } -} + LEN1 = GPIO_BASE_SIZE
-Device (GCM4) -{ - Name (_HID, CROS_GPIO_NAME) - Name (_UID, 4) - Name (_DDN, "GPIO Controller Community 4") - - Name (RBUF, ResourceTemplate() - { - Memory32Fixed (ReadWrite, 0, GPIO_BASE_SIZE, COM4) - Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) - { GPIO_IRQ14 } - }) - Method (_CRS, 0, NotSerialized) - { + /* GPIO Community 4 */ CreateDWordField (^RBUF, ^COM4._BAS, BAS4) + CreateDWordField (^RBUF, ^COM4._LEN, LEN4) BAS4 = ^^PCRB (PID_GPIOCOM4) - Return (^RBUF) - } - Method (_STA) - { - Return (0xF) - } -} + LEN4 = GPIO_BASE_SIZE
-Device (GCM5) -{ - Name (_HID, CROS_GPIO_NAME) - Name (_UID, 5) - Name (_DDN, "GPIO Controller Community 5") - - Name (RBUF, ResourceTemplate() - { - Memory32Fixed (ReadWrite, 0, GPIO_BASE_SIZE, COM5) - Interrupt (ResourceConsumer, Level, ActiveLow, Shared,,, GIRQ) - { GPIO_IRQ14 } - }) - Method (_CRS, 0, NotSerialized) - { + /* GPIO Community 5 */ CreateDWordField (^RBUF, ^COM5._BAS, BAS5) + CreateDWordField (^RBUF, ^COM5._LEN, LEN5) BAS5 = ^^PCRB (PID_GPIOCOM5) - Return (^RBUF) + LEN5 = GPIO_BASE_SIZE + + Return (RBUF) } - Method (_STA) + + Method (_STA, 0, NotSerialized) { Return (0xF) } diff --git a/src/soc/intel/tigerlake/gpio.c b/src/soc/intel/tigerlake/gpio.c index cfdd0ac..f96262a 100644 --- a/src/soc/intel/tigerlake/gpio.c +++ b/src/soc/intel/tigerlake/gpio.c @@ -37,21 +37,30 @@ };
/* - * This layout matches the Linux kernel pinctrl map for TGL-LP at: + * The GPIO pinctrl driver for Tiger Lake on Linux expects 32 GPIOs per pad + * group, regardless of whether or not there is a physical pad for each + * exposed GPIO number. + * + * This results in the OS having a sparse GPIO map, and devices that need + * to export an ACPI GPIO must use the OS expected number. + * + * Not all pins are usable as GPIO and those groups do not have a pad base. + * + * This layout matches the Linux kernel pinctrl map for TGL at: * linux/drivers/pinctrl/intel/pinctrl-tigerlake.c */ static const struct pad_group tgl_community0_groups[] = { - INTEL_GPP(GPP_B0, GPP_B0, GPP_B25), /* GPP_B */ - INTEL_GPP(GPP_B0, GPP_T0, GPP_T15), /* GPP_T */ - INTEL_GPP(GPP_B0, GPP_A0, GPP_A24), /* GPP_A */ + INTEL_GPP_BASE(GPP_B0, GPP_B0, GPP_B25, 0), /* GPP_B */ + INTEL_GPP_BASE(GPP_B0, GPP_T0, GPP_T15, 32), /* GPP_T */ + INTEL_GPP_BASE(GPP_B0, GPP_A0, GPP_A24, 64), /* GPP_A */ };
static const struct pad_group tgl_community1_groups[] = { - INTEL_GPP(GPP_S0, GPP_S0, GPP_S7), /* GPP_S */ - INTEL_GPP(GPP_S0, GPP_H0, GPP_H23), /* GPP_H */ - INTEL_GPP(GPP_S0, GPP_D0, GPP_GSPI2_CLK_LOOPBK), /* GPP_D */ - INTEL_GPP(GPP_S0, GPP_U0, GPP_GSPI6_CLK_LOOPBK), /* GPP_U */ - INTEL_GPP(GPP_S0, CNV_BTEN, vI2S2_RXD), /* GPP_VGPIO */ + INTEL_GPP_BASE(GPP_S0, GPP_S0, GPP_S7, 96), /* GPP_S */ + INTEL_GPP_BASE(GPP_S0, GPP_H0, GPP_H23, 128), /* GPP_H */ + INTEL_GPP_BASE(GPP_S0, GPP_D0, GPP_GSPI2_CLK_LOOPBK, 160), /* GPP_D */ + INTEL_GPP_BASE(GPP_S0, GPP_U0, GPP_GSPI6_CLK_LOOPBK, 192), /* GPP_U */ + INTEL_GPP_BASE(GPP_S0, CNV_BTEN, vI2S2_RXD, 224), /* GPP_VGPIO */ };
/* This community is not visible to the OS */ @@ -60,15 +69,15 @@ };
static const struct pad_group tgl_community4_groups[] = { - INTEL_GPP(GPP_C0, GPP_C0, GPP_C23), /* GPP_C */ - INTEL_GPP(GPP_C0, GPP_F0, GPP_F_CLK_LOOPBK), /* GPP_F */ + INTEL_GPP_BASE(GPP_C0, GPP_C0, GPP_C23, 256), /* GPP_C */ + INTEL_GPP_BASE(GPP_C0, GPP_F0, GPP_F_CLK_LOOPBK, 288), /* GPP_F */ INTEL_GPP(GPP_C0, GPP_L_BKLTEN, GPP_MLK_RSTB), /* GPP_HVCMOS */ - INTEL_GPP(GPP_C0, GPP_E0, GPP_E_CLK_LOOPBK), /* GPP_E */ + INTEL_GPP_BASE(GPP_C0, GPP_E0, GPP_E_CLK_LOOPBK, 320), /* GPP_E */ INTEL_GPP(GPP_C0, GPP_JTAG_TDO, GPP_DBG_PMODE), /* GPP_JTAG */ };
static const struct pad_group tgl_community5_groups[] = { - INTEL_GPP(GPP_R0, GPP_R0, GPP_R7), /* GPP_R */ + INTEL_GPP_BASE(GPP_R0, GPP_R0, GPP_R7, 352), /* GPP_R */ INTEL_GPP(GPP_R0, GPP_SPI_IO_2, GPP_CLK_LOOPBK), /* GPP_SPI */ };
diff --git a/src/soc/intel/tigerlake/include/soc/gpio.h b/src/soc/intel/tigerlake/include/soc/gpio.h index ccf6663..0ac0033 100644 --- a/src/soc/intel/tigerlake/include/soc/gpio.h +++ b/src/soc/intel/tigerlake/include/soc/gpio.h @@ -7,10 +7,7 @@ #include <soc/gpio_defs.h> #include <intelblocks/gpio.h>
-#define CROS_GPIO_NAME "INT34C5" -#define CROS_GPIO_COMM0_NAME "INT34C5:00" -#define CROS_GPIO_COMM1_NAME "INT34C5:01" -#define CROS_GPIO_COMM4_NAME "INT34C5:02" -#define CROS_GPIO_COMM5_NAME "INT34C5:03" + +#define CROS_GPIO_DEVICE_NAME "INT34C5:00"
#endif
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39801 )
Change subject: soc/intel/tgl: Synchronize GPIO ASL table with Linux kernel ......................................................................
Patch Set 13:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/3231 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3230 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3229 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3228
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Duncan Laurie (Code Review)
-
Furquan Shaikh (Code Review)
-
Paul Menzel (Code Review)
-
Shaunak Saha (Code Review)
-
Tim Wawrzynczak (Code Review)