EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48944 )
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
soc/amd/picasso: Add GRXS and GTXS method
Add GRXS and GTXS into gpiolib. We can align with Intel ACPI method for the better usage. This benefit acpi.c to be more clear, too.
BUG=b:176270381 BRANCH=zork TEST=Confirm the Goodix touchscreen functional.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I1aa6a8f44f20577e679336889c849dd67cb99f2d --- M src/soc/amd/common/acpi/gpio_bank_lib.asl M src/soc/amd/picasso/acpi.c 2 files changed, 40 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/48944/1
diff --git a/src/soc/amd/common/acpi/gpio_bank_lib.asl b/src/soc/amd/common/acpi/gpio_bank_lib.asl index eb13ab9..b142be5 100644 --- a/src/soc/amd/common/acpi/gpio_bank_lib.asl +++ b/src/soc/amd/common/acpi/gpio_bank_lib.asl @@ -2,6 +2,8 @@
#include <soc/iomap.h>
+#define GPIO_INPUT_SHIFT 16 +#define GPIO_INPUT_VALUE (1 << GPIO_INPUT_SHIFT) #define GPIO_OUTPUT_SHIFT 22 #define GPIO_OUTPUT_VALUE (1 << GPIO_OUTPUT_SHIFT)
@@ -148,3 +150,35 @@ } VAL0 &= ~GPIO_OUTPUT_VALUE } + +/* + * Get GPIO Rx Value + * Arg0 - GPIO Number + */ +Method (GRXS, 1, Serialized) +{ + OperationRegion (PREG, SystemMemory, GPAD (Arg0), 4) + Field (PREG, AnyAcc, NoLock, Preserve) + { + VAL0, 32 + } + Local0 = (GPIO_INPUT_VALUE & VAL0) >> GPIO_INPUT_SHIFT + + Return (Local0) +} + +/* + * Get GPIO Tx Value + * Arg0 - GPIO Number + */ +Method (GTXS, 1, Serialized) +{ + OperationRegion (PREG, SystemMemory, GPAD (Arg0), 4) + Field (PREG, AnyAcc, NoLock, Preserve) + { + VAL0, 32 + } + Local0 = (GPIO_OUTPUT_VALUE & VAL0) >> GPIO_OUTPUT_SHIFT + + Return (Local0) +} diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c index 63ba26c..ef0d6e2 100644 --- a/src/soc/amd/picasso/acpi.c +++ b/src/soc/amd/picasso/acpi.c @@ -431,57 +431,23 @@ } }
-static void acpigen_soc_get_gpio_in_local5(uintptr_t addr) +static int acpigen_soc_get_gpio_state(const char *op, unsigned int gpio_num) { - /* - * Store (_SB.GPR2 (addr), Local5) - * _SB.GPR2 is used to read control byte 2 from control register. - * / It is defined in gpio_lib.asl. - */ + /* Store (op (gpio_num), Local0) */ acpigen_write_store(); - acpigen_emit_namestring("\_SB.GPR2"); - acpigen_write_integer(addr); - acpigen_emit_byte(LOCAL5_OP); -} - -static int acpigen_soc_get_gpio_val(unsigned int gpio_num, uint32_t mask) -{ - if (gpio_num >= SOC_GPIO_TOTAL_PINS) { - printk(BIOS_WARNING, "Warning: Pin %d should be smaller than" - " %d\n", gpio_num, SOC_GPIO_TOTAL_PINS); - return -1; - } - uintptr_t addr = gpio_get_address(gpio_num); - - acpigen_soc_get_gpio_in_local5(addr); - - /* If (And (Local5, mask)) */ - acpigen_write_if_and(LOCAL5_OP, mask); - - /* Store (One, Local0) */ - acpigen_write_store_ops(ONE_OP, LOCAL0_OP); - - acpigen_pop_len(); /* If */ - - /* Else */ - acpigen_write_else(); - - /* Store (Zero, Local0) */ - acpigen_write_store_ops(ZERO_OP, LOCAL0_OP); - - acpigen_pop_len(); /* Else */ - + acpigen_soc_gpio_op(op, gpio_num); + acpigen_emit_byte(LOCAL0_OP); return 0; }
int acpigen_soc_read_rx_gpio(unsigned int gpio_num) { - return acpigen_soc_get_gpio_val(gpio_num, GPIO_PIN_IN); + return acpigen_soc_get_gpio_state("\_SB.GRXS", gpio_num); }
int acpigen_soc_get_tx_gpio(unsigned int gpio_num) { - return acpigen_soc_get_gpio_val(gpio_num, GPIO_PIN_OUT); + return acpigen_soc_get_gpio_state("\_SB.GTXS", gpio_num); }
static int acpigen_soc_gpio_op(const char *op, unsigned int gpio_num)
Hello build bot (Jenkins), Jason Glenesk, Marshall Dawson, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48944
to look at the new patch set (#2).
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
soc/amd/picasso: Add GRXS and GTXS method
Add GRXS and GTXS into gpiolib. We can align with Intel ACPI method for the better usage. This benefit acpi.c to be more clear, too.
BUG=b:176270381 BRANCH=zork TEST=Confirm the Goodix touchscreen functional.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I1aa6a8f44f20577e679336889c849dd67cb99f2d --- M src/soc/amd/common/acpi/gpio_bank_lib.asl M src/soc/amd/picasso/acpi.c 2 files changed, 53 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/48944/2
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48944 )
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
Patch Set 2:
@AMD, please help review this, I want we can use this in cezanne for guybrush project :)
Hello build bot (Jenkins), Jason Glenesk, Furquan Shaikh, Marshall Dawson, Mathew King, Tim Wawrzynczak, Kyösti Mälkki, chris wang, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48944
to look at the new patch set (#3).
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
soc/amd/picasso: Add GRXS and GTXS method
Add GRXS and GTXS into gpiolib. We can align with Intel ACPI method for the better usage. This benefit acpi.c to be more clear, too.
BUG=b:176270381 BRANCH=zork TEST=Confirm the Goodix touchscreen functional.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I1aa6a8f44f20577e679336889c849dd67cb99f2d --- M src/soc/amd/common/acpi/gpio_bank_lib.asl M src/soc/amd/picasso/acpi.c 2 files changed, 53 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/48944/3
Hello build bot (Jenkins), Jason Glenesk, Furquan Shaikh, Marshall Dawson, Mathew King, Tim Wawrzynczak, Kyösti Mälkki, chris wang, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48944
to look at the new patch set (#4).
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
soc/amd/picasso: Add GRXS and GTXS method
Add GRXS and GTXS into gpiolib. We can align with Intel ACPI method for the better usage. This benefit acpi.c to be more clear, too.
BUG=b:176270381 BRANCH=zork TEST=Confirm the Goodix touchscreen functional.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I1aa6a8f44f20577e679336889c849dd67cb99f2d --- M src/soc/amd/common/acpi/gpio_bank_lib.asl M src/soc/amd/picasso/acpi.c 2 files changed, 53 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/48944/4
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48944 )
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
Patch Set 4: Code-Review+1
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48944 )
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48944/4/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/c/coreboot/+/48944/4/src/soc/amd/picasso/acpi.c@... PS4, Line 444: { keep the check for the upper bound of gpio_num in here?
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48944 )
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48944/4/src/soc/amd/picasso/acpi.c File src/soc/amd/picasso/acpi.c:
https://review.coreboot.org/c/coreboot/+/48944/4/src/soc/amd/picasso/acpi.c@... PS4, Line 444: {
keep the check for the upper bound of gpio_num in here?
Done
Hello build bot (Jenkins), Jason Glenesk, Furquan Shaikh, Marshall Dawson, Mathew King, Tim Wawrzynczak, Kyösti Mälkki, chris wang, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48944
to look at the new patch set (#6).
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
soc/amd/picasso: Add GRXS and GTXS method
Add GRXS and GTXS into gpiolib. We can align with Intel ACPI method for the better usage. This benefit acpi.c to be more clear, too.
BUG=b:176270381 BRANCH=zork TEST=Confirm the Goodix touchscreen functional.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I1aa6a8f44f20577e679336889c849dd67cb99f2d --- M src/soc/amd/common/acpi/gpio_bank_lib.asl M src/soc/amd/picasso/acpi.c 2 files changed, 58 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/48944/6
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48944 )
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
Patch Set 6: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48944 )
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48944/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48944/6//COMMIT_MSG@9 PS6, Line 9: Intel ACPI method If they are the same, could they be shared?
https://review.coreboot.org/c/coreboot/+/48944/6//COMMIT_MSG@10 PS6, Line 10: benefit benefits
Hello build bot (Jenkins), Jason Glenesk, Furquan Shaikh, Marshall Dawson, Mathew King, Tim Wawrzynczak, Kyösti Mälkki, chris wang, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48944
to look at the new patch set (#7).
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
soc/amd/picasso: Add GRXS and GTXS method
Add GRXS and GTXS into gpiolib. We can align with Intel ACPI method for the better usage. This benefits acpi.c to be more clear, too.
BUG=b:176270381 BRANCH=zork TEST=Confirm the Goodix touchscreen functional.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I1aa6a8f44f20577e679336889c849dd67cb99f2d --- M src/soc/amd/common/acpi/gpio_bank_lib.asl M src/soc/amd/picasso/acpi.c 2 files changed, 58 insertions(+), 53 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/48944/7
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48944 )
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48944/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48944/6//COMMIT_MSG@10 PS6, Line 10: benefit
benefits
Done
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48944 )
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48944/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48944/6//COMMIT_MSG@9 PS6, Line 9: Intel ACPI method
If they are the same, could they be shared?
No.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48944 )
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
Patch Set 8: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/48944/8/src/soc/amd/common/acpi/gpi... File src/soc/amd/common/acpi/gpio_bank_lib.asl:
https://review.coreboot.org/c/coreboot/+/48944/8/src/soc/amd/common/acpi/gpi... PS8, Line 5: #define GPIO_INPUT_SHIFT 16 : #define GPIO_INPUT_VALUE (1 << GPIO_INPUT_SHIFT) Same comment as previous CL.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48944 )
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
Patch Set 8:
(1 comment)
We can do clean up by move some define out of gpiobank. But should decide by AMD team.
https://review.coreboot.org/c/coreboot/+/48944/8/src/soc/amd/common/acpi/gpi... File src/soc/amd/common/acpi/gpio_bank_lib.asl:
https://review.coreboot.org/c/coreboot/+/48944/8/src/soc/amd/common/acpi/gpi... PS8, Line 5: #define GPIO_INPUT_SHIFT 16 : #define GPIO_INPUT_VALUE (1 << GPIO_INPUT_SHIFT)
Same comment as previous CL.
Ack
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48944 )
Change subject: soc/amd/picasso: Add GRXS and GTXS method ......................................................................
soc/amd/picasso: Add GRXS and GTXS method
Add GRXS and GTXS into gpiolib. We can align with Intel ACPI method for the better usage. This benefits acpi.c to be more clear, too.
BUG=b:176270381 BRANCH=zork TEST=Confirm the Goodix touchscreen functional.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: I1aa6a8f44f20577e679336889c849dd67cb99f2d Reviewed-on: https://review.coreboot.org/c/coreboot/+/48944 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M src/soc/amd/common/acpi/gpio_bank_lib.asl M src/soc/amd/picasso/acpi.c 2 files changed, 58 insertions(+), 53 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/amd/common/acpi/gpio_bank_lib.asl b/src/soc/amd/common/acpi/gpio_bank_lib.asl index f209f44..8640cc1 100644 --- a/src/soc/amd/common/acpi/gpio_bank_lib.asl +++ b/src/soc/amd/common/acpi/gpio_bank_lib.asl @@ -2,6 +2,8 @@
#include <soc/iomap.h>
+#define GPIO_INPUT_SHIFT 16 +#define GPIO_INPUT_VALUE (1 << GPIO_INPUT_SHIFT) #define GPIO_OUTPUT_SHIFT 22 #define GPIO_OUTPUT_VALUE (1 << GPIO_OUTPUT_SHIFT)
@@ -148,3 +150,35 @@ } VAL0 &= ~GPIO_OUTPUT_VALUE } + +/* + * Get GPIO Input Value + * Arg0 - GPIO Number + */ +Method (GRXS, 1, Serialized) +{ + OperationRegion (GPDW, SystemMemory, GPAD (Arg0), 4) + Field (GPDW, AnyAcc, NoLock, Preserve) + { + VAL0, 32 + } + Local0 = (GPIO_INPUT_VALUE & VAL0) >> GPIO_INPUT_SHIFT + + Return (Local0) +} + +/* + * Get GPIO Output Value + * Arg0 - GPIO Number + */ +Method (GTXS, 1, Serialized) +{ + OperationRegion (GPDW, SystemMemory, GPAD (Arg0), 4) + Field (GPDW, AnyAcc, NoLock, Preserve) + { + VAL0, 32 + } + Local0 = (GPIO_OUTPUT_VALUE & VAL0) >> GPIO_OUTPUT_SHIFT + + Return (Local0) +} diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c index 445815a..b89974b 100644 --- a/src/soc/amd/picasso/acpi.c +++ b/src/soc/amd/picasso/acpi.c @@ -432,59 +432,6 @@ } }
-static void acpigen_soc_get_gpio_in_local5(uintptr_t addr) -{ - /* - * Store (_SB.GPR2 (addr), Local5) - * _SB.GPR2 is used to read control byte 2 from control register. - * / It is defined in gpio_lib.asl. - */ - acpigen_write_store(); - acpigen_emit_namestring("\_SB.GPR2"); - acpigen_write_integer(addr); - acpigen_emit_byte(LOCAL5_OP); -} - -static int acpigen_soc_get_gpio_val(unsigned int gpio_num, uint32_t mask) -{ - if (gpio_num >= SOC_GPIO_TOTAL_PINS) { - printk(BIOS_WARNING, "Warning: Pin %d should be smaller than" - " %d\n", gpio_num, SOC_GPIO_TOTAL_PINS); - return -1; - } - uintptr_t addr = gpio_get_address(gpio_num); - - acpigen_soc_get_gpio_in_local5(addr); - - /* If (And (Local5, mask)) */ - acpigen_write_if_and(LOCAL5_OP, mask); - - /* Store (One, Local0) */ - acpigen_write_store_ops(ONE_OP, LOCAL0_OP); - - acpigen_pop_len(); /* If */ - - /* Else */ - acpigen_write_else(); - - /* Store (Zero, Local0) */ - acpigen_write_store_ops(ZERO_OP, LOCAL0_OP); - - acpigen_pop_len(); /* Else */ - - return 0; -} - -int acpigen_soc_read_rx_gpio(unsigned int gpio_num) -{ - return acpigen_soc_get_gpio_val(gpio_num, GPIO_PIN_IN); -} - -int acpigen_soc_get_tx_gpio(unsigned int gpio_num) -{ - return acpigen_soc_get_gpio_val(gpio_num, GPIO_PIN_OUT); -} - static int acpigen_soc_gpio_op(const char *op, unsigned int gpio_num) { if (gpio_num >= SOC_GPIO_TOTAL_PINS) { @@ -498,6 +445,30 @@ return 0; }
+static int acpigen_soc_get_gpio_state(const char *op, unsigned int gpio_num) +{ + if (gpio_num >= SOC_GPIO_TOTAL_PINS) { + printk(BIOS_WARNING, "Warning: Pin %d should be smaller than" + " %d\n", gpio_num, SOC_GPIO_TOTAL_PINS); + return -1; + } + /* Store (op (gpio_num), Local0) */ + acpigen_write_store(); + acpigen_soc_gpio_op(op, gpio_num); + acpigen_emit_byte(LOCAL0_OP); + return 0; +} + +int acpigen_soc_read_rx_gpio(unsigned int gpio_num) +{ + return acpigen_soc_get_gpio_state("\_SB.GRXS", gpio_num); +} + +int acpigen_soc_get_tx_gpio(unsigned int gpio_num) +{ + return acpigen_soc_get_gpio_state("\_SB.GTXS", gpio_num); +} + int acpigen_soc_set_tx_gpio(unsigned int gpio_num) { return acpigen_soc_gpio_op("\_SB.STXS", gpio_num);
-
EricR Lai (Code Review) -
Felix Held (Code Review)
-
Furquan Shaikh (Code Review)
-
Paul Menzel (Code Review)
-
Tim Wawrzynczak (Code Review)