Josie Nordrum has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44640 )
Change subject: soc/amd/common: Move interrupt and wake status clear ......................................................................
soc/amd/common: Move interrupt and wake status clear
Move interrupt status and wake status clearing to after GPIO config.
BUG=b:164892883, b:165342107 TEST=None BRANCH=None
Signed-off-by: Josie Nordrum josienordrum@google.com Change-Id: If4a5db4bfa6a2ee9827f38e9595f487a4dcfac2c --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 2 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/44640/1
diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c index 7fb6622..74ea696 100644 --- a/src/soc/amd/common/block/gpio_banks/gpio.c +++ b/src/soc/amd/common/block/gpio_banks/gpio.c @@ -214,10 +214,9 @@
soc_gpio_hook(gpio, mux);
- /* Clear interrupt and wake status (write 1-to-clear bits) */ - control |= GPIO_INT_STATUS | GPIO_WAKE_STATUS; __gpio_setbits32(gpio, PAD_CFG_MASK, control); - + /* Clear interrupt and wake status (write 1-to-clear bits) */ + __gpio_or32(gpio, GPIO_INT_STATUS | GPIO_WAKE_STATUS); if (control_flags == 0) continue;
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44640 )
Change subject: soc/amd/common: Move interrupt and wake status clear ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44640/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44640/1//COMMIT_MSG@9 PS1, Line 9: Move interrupt status and wake status clearing to after GPIO config. It would be good to explain the reason behind the change here.
Hello build bot (Jenkins), Furquan Shaikh, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44640
to look at the new patch set (#2).
Change subject: soc/amd/common: Move interrupt and wake status clear ......................................................................
soc/amd/common: Move interrupt and wake status clear
Move interrupt status and wake status clearing to after GPIO config.
BUG=b:164892883, b:165342107 TEST=None BRANCH=None
Signed-off-by: Josie Nordrum josienordrum@google.com Change-Id: If4a5db4bfa6a2ee9827f38e9595f487a4dcfac2c --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 2 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/44640/2
Hello build bot (Jenkins), Furquan Shaikh, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44640
to look at the new patch set (#3).
Change subject: soc/amd/common: Move interrupt and wake status clear ......................................................................
soc/amd/common: Move interrupt and wake status clear
Move interrupt status and wake status clearing to after GPIO config so that configuration does not incorrectly set interrupt or wake status.
BUG=b:164892883, b:165342107 TEST=None BRANCH=None
Signed-off-by: Josie Nordrum josienordrum@google.com Change-Id: If4a5db4bfa6a2ee9827f38e9595f487a4dcfac2c --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 2 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/44640/3
Josie Nordrum has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44640 )
Change subject: soc/amd/common: Move interrupt and wake status clear ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44640/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44640/1//COMMIT_MSG@9 PS1, Line 9: Move interrupt status and wake status clearing to after GPIO config.
It would be good to explain the reason behind the change here.
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44640 )
Change subject: soc/amd/common: Move interrupt and wake status clear ......................................................................
Patch Set 3: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44640 )
Change subject: soc/amd/common: Move interrupt and wake status clear ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44640/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44640/3//COMMIT_MSG@10 PS3, Line 10: that configuration does not incorrectly set interrupt or wake status. Can you please add an example as well i.e. when you configured PULL_UP on a pad, it resulted in interrupt status being set after the write to pad control register even though interrupt status bit was set to 1. Thus, interrupt/wake status need to be cleared after the initial pad configuration is done.
Hello build bot (Jenkins), Furquan Shaikh, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44640
to look at the new patch set (#4).
Change subject: soc/amd/common: Move interrupt and wake status clear ......................................................................
soc/amd/common: Move interrupt and wake status clear
Move interrupt status and wake status clearing to after GPIO config so that configuration does not incorrectly set interrupt or wake status.
i.e. when PULL_UP is configured on a pad, it incorrectly sets in the interrupt status bit. Thus, the interrupt status bit must be cleared after initial pad configuration is complete.
BUG=b:164892883, b:165342107 TEST=None BRANCH=None
Signed-off-by: Josie Nordrum josienordrum@google.com Change-Id: If4a5db4bfa6a2ee9827f38e9595f487a4dcfac2c --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 2 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/44640/4
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44640 )
Change subject: soc/amd/common: Move interrupt and wake status clear ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/44640/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44640/3//COMMIT_MSG@10 PS3, Line 10: that configuration does not incorrectly set interrupt or wake status.
Can you please add an example as well i.e. […]
Done
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44640 )
Change subject: soc/amd/common: Move interrupt and wake status clear ......................................................................
soc/amd/common: Move interrupt and wake status clear
Move interrupt status and wake status clearing to after GPIO config so that configuration does not incorrectly set interrupt or wake status.
i.e. when PULL_UP is configured on a pad, it incorrectly sets in the interrupt status bit. Thus, the interrupt status bit must be cleared after initial pad configuration is complete.
BUG=b:164892883, b:165342107 TEST=None BRANCH=None
Signed-off-by: Josie Nordrum josienordrum@google.com Change-Id: If4a5db4bfa6a2ee9827f38e9595f487a4dcfac2c Reviewed-on: https://review.coreboot.org/c/coreboot/+/44640 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/common/block/gpio_banks/gpio.c 1 file changed, 2 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/amd/common/block/gpio_banks/gpio.c b/src/soc/amd/common/block/gpio_banks/gpio.c index 7fb6622..74ea696 100644 --- a/src/soc/amd/common/block/gpio_banks/gpio.c +++ b/src/soc/amd/common/block/gpio_banks/gpio.c @@ -214,10 +214,9 @@
soc_gpio_hook(gpio, mux);
- /* Clear interrupt and wake status (write 1-to-clear bits) */ - control |= GPIO_INT_STATUS | GPIO_WAKE_STATUS; __gpio_setbits32(gpio, PAD_CFG_MASK, control); - + /* Clear interrupt and wake status (write 1-to-clear bits) */ + __gpio_or32(gpio, GPIO_INT_STATUS | GPIO_WAKE_STATUS); if (control_flags == 0) continue;
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)
-
Josie Nordrum (Code Review)