Change in coreboot[master]: mb/google/hatch: Disable GPIO community dynamic clock gating
Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34176 )
Change subject: mb/google/hatch: Disable GPIO community dynamic clock gating ......................................................................
mb/google/hatch: Disable GPIO community dynamic clock gating
The dynamic clock gating is causing boards to miss interrupts whose pulses are shorter than 4us. Disable it using FSP UPDs.
BUG=b:130764684 BRANCH=none TEST=Compiles
Change-Id: I8f1ec8f7c31192bce2a761ec99b86638435dc27c Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb 1 file changed, 5 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/34176/1
diff --git a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb index 9a912539..36b5e8b 100644 --- a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb @@ -95,12 +95,11 @@ register "gpio_override_pm" = "1"
# GPIO community PM configuration - register "gpio_pm[COMM_0]" = "MISCCFG_ENABLE_GPIO_PM_CONFIG" - register "gpio_pm[COMM_1]" = "MISCCFG_ENABLE_GPIO_PM_CONFIG" - register "gpio_pm[COMM_2]" = "MISCCFG_ENABLE_GPIO_PM_CONFIG" - register "gpio_pm[COMM_3]" = "MISCCFG_ENABLE_GPIO_PM_CONFIG" - # Disable clock gating on this community so that cr50's short irq - # pulses won't be missed. + # Disable dynamic clock gating + register "gpio_pm[COMM_0]" = "0" + register "gpio_pm[COMM_1]" = "0" + register "gpio_pm[COMM_2]" = "0" + register "gpio_pm[COMM_3]" = "0" register "gpio_pm[COMM_4]" = "0"
device cpu_cluster 0 on
Hello Subrata Banik, Duncan Laurie, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34176
to look at the new patch set (#2).
Change subject: mb/google/hatch: Disable GPIO community dynamic clock gating ......................................................................
mb/google/hatch: Disable GPIO community dynamic clock gating
The dynamic clock gating is causing boards to miss interrupts whose pulses are shorter than 4us. Disable it using FSP UPDs.
BUG=b:130764684 BRANCH=none TEST=Compiles
Change-Id: I8f1ec8f7c31192bce2a761ec99b86638435dc27c Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb M src/soc/intel/cannonlake/acpi/lpit.asl 2 files changed, 16 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/34176/2
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34176 )
Change subject: mb/google/hatch: Disable GPIO community dynamic clock gating ......................................................................
Patch Set 3: Code-Review+1
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34176 )
Change subject: mb/google/hatch: Disable GPIO community dynamic clock gating ......................................................................
Patch Set 3: Code-Review+2
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34176 )
Change subject: mb/google/hatch: Disable GPIO community dynamic clock gating ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34176/3/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/34176/3/src/mainboard/google/hatch/... PS3, Line 98: Disable dynamic clock gating Can you say why, like the old comment did. Also, as was mentioned on the bug, we need to understand how much power we're sacrificing here. It's possible there were a few flags that we could still leave on that helped power but didn't put us at risk to miss short pulses. I wanted to try the experiment in comment #52 of b/130764684, but hadn't had a chance to do it yet.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34176 )
Change subject: mb/google/hatch: Disable GPIO community dynamic clock gating ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34176/3/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/34176/3/src/mainboard/google/hatch/... PS3, Line 98: Disable dynamic clock gating
Can you say why, like the old comment did. […]
Ack
Hello Patrick Rudolph, Paul Fagerburg, Subrata Banik, Duncan Laurie, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34176
to look at the new patch set (#4).
Change subject: mb/google/hatch: Disable GPIO community dynamic clock gating ......................................................................
mb/google/hatch: Disable GPIO community dynamic clock gating
The dynamic clock gating is causing boards to miss interrupts whose pulses are shorter than 4us. Disable it using FSP UPDs.
BUG=b:130764684 BRANCH=none TEST=Compiles
Change-Id: I8f1ec8f7c31192bce2a761ec99b86638435dc27c Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb 1 file changed, 6 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/34176/4
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34176 )
Change subject: mb/google/hatch: Disable GPIO community dynamic clock gating ......................................................................
Patch Set 4: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34176 )
Change subject: mb/google/hatch: Disable GPIO community dynamic clock gating ......................................................................
Patch Set 4: Code-Review+2
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34176 )
Change subject: mb/google/hatch: Disable GPIO community dynamic clock gating ......................................................................
Patch Set 4: Code-Review+1
Thanks! I'm not allowed to +2, but here's a +1 for you!
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34176 )
Change subject: mb/google/hatch: Disable GPIO community dynamic clock gating ......................................................................
Patch Set 4: Code-Review+2
wondering if this CL is needed for Sarine/Arcada. Don't you see such issue there ?
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34176 )
Change subject: mb/google/hatch: Disable GPIO community dynamic clock gating ......................................................................
mb/google/hatch: Disable GPIO community dynamic clock gating
The dynamic clock gating is causing boards to miss interrupts whose pulses are shorter than 4us. Disable it using FSP UPDs.
BUG=b:130764684 BRANCH=none TEST=Compiles
Change-Id: I8f1ec8f7c31192bce2a761ec99b86638435dc27c Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/34176 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Fagerburg pfagerburg@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Evan Green evgreen@chromium.org Reviewed-by: Subrata Banik subrata.banik@intel.com --- M src/mainboard/google/hatch/variants/baseboard/devicetree.cb 1 file changed, 6 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Subrata Banik: Looks good to me, approved Evan Green: Looks good to me, but someone else must approve Paul Fagerburg: Looks good to me, approved
diff --git a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb index 9a912539..a470032 100644 --- a/src/mainboard/google/hatch/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/hatch/variants/baseboard/devicetree.cb @@ -95,12 +95,12 @@ register "gpio_override_pm" = "1"
# GPIO community PM configuration - register "gpio_pm[COMM_0]" = "MISCCFG_ENABLE_GPIO_PM_CONFIG" - register "gpio_pm[COMM_1]" = "MISCCFG_ENABLE_GPIO_PM_CONFIG" - register "gpio_pm[COMM_2]" = "MISCCFG_ENABLE_GPIO_PM_CONFIG" - register "gpio_pm[COMM_3]" = "MISCCFG_ENABLE_GPIO_PM_CONFIG" - # Disable clock gating on this community so that cr50's short irq - # pulses won't be missed. + # Disable dynamic clock gating; with bits 0-5 set in these registers, + # some short interrupt pulses were missed (esp. cr50 irq) + register "gpio_pm[COMM_0]" = "0" + register "gpio_pm[COMM_1]" = "0" + register "gpio_pm[COMM_2]" = "0" + register "gpio_pm[COMM_3]" = "0" register "gpio_pm[COMM_4]" = "0"
device cpu_cluster 0 on
-
Evan Green (Code Review) -
Furquan Shaikh (Code Review)
-
Martin Roth (Code Review)
-
Paul Fagerburg (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)