Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32485
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
soc/intel/common: Add new PAD_CFG macro.
Added macro named PAD_CFG_GPI_GPIO_DRIVER_SCI, for pads that need to be configured as GPI, GPIO Driver mode, and SCI interrupt.
BUG=none BRANCH=none TEST=Compiles
Change-Id: I0332c64e2fa62ce29c772444606adbfdf9c9afc4 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/32485/1
diff --git a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h index 0ad3e5c..26531f8 100644 --- a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h +++ b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h @@ -244,6 +244,13 @@ PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE, \ PAD_PULL(pull) | PAD_CFG1_GPIO_DRIVER | PAD_IOSSTATE(TxDRxE))
+/* GPI, GPIO Driver, SCI interrupt */ +#define PAD_CFG_GPI_GPIO_DRIVER_SCI(pad, pull, rst, trig, inv) \ + _PAD_CFG_STRUCT(pad, \ + PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \ + PAD_IRQ_CFG(SCI, trig, inv), \ + PAD_PULL(pull) | PAD_CFG1_GPIO_DRIVER | PAD_IOSSTATE(TxDRxE)) + #define PAD_CFG_GPIO_DRIVER_HI_Z(pad, pull, rst, iosstate, iosterm) \ _PAD_CFG_STRUCT(pad, \ PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32485 )
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/#/c/32485/1/src/soc/intel/common/block/include/i... File src/soc/intel/common/block/include/intelblocks/gpio_defs.h:
https://review.coreboot.org/#/c/32485/1/src/soc/intel/common/block/include/i... PS1, Line 248: PAD_CFG_GPI_GPIO_DRIVER_SCI Let's move this down along with PAD_CFG_GPI_IRQ_WAKE under SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT
https://review.coreboot.org/#/c/32485/1/src/soc/intel/common/block/include/i... PS1, Line 251: extra tab?
https://review.coreboot.org/#/c/32485/1/src/soc/intel/common/block/include/i... PS1, Line 401: PAD_CFG_GPI_IRQ_WAKE It would be good to rename this to PAD_CFG_GPI_APIC_SCI() to make it clear what it is being configured for. You will also have to update mainboards using this. Probably can do that as a separate CL from PAD_CFG_GPI_GPIO_DRIVER_SCI.
https://review.coreboot.org/#/c/32485/1/src/soc/intel/common/block/include/i... PS1, Line 402: PAD_CFG_GPI_DUAL_ROUTE While we are here, we should also get rid of SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT and just copy its definition to PAD_CFG_GPI_IRQ_WAKE.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32485 )
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/#/c/32485/1/src/soc/intel/common/block/include/i... File src/soc/intel/common/block/include/intelblocks/gpio_defs.h:
https://review.coreboot.org/#/c/32485/1/src/soc/intel/common/block/include/i... PS1, Line 248: PAD_CFG_GPI_GPIO_DRIVER_SCI
Let's move this down along with PAD_CFG_GPI_IRQ_WAKE under SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SU […]
Done
https://review.coreboot.org/#/c/32485/1/src/soc/intel/common/block/include/i... PS1, Line 251:
extra tab?
Some macros have the extra indent to indicate it's a continuation line. PAD_CFG_GPI_INT for example; PAD_CFG_GPIO_DRIVER_HI_Z is a counterexample. No consistent style here :) Anyone have any opinions on which is better? I can work on a CL to clean this up.
https://review.coreboot.org/#/c/32485/1/src/soc/intel/common/block/include/i... PS1, Line 401: PAD_CFG_GPI_IRQ_WAKE
It would be good to rename this to PAD_CFG_GPI_APIC_SCI() to make it clear what it is being configur […]
Sure, I'll work on that.
https://review.coreboot.org/#/c/32485/1/src/soc/intel/common/block/include/i... PS1, Line 402: PAD_CFG_GPI_DUAL_ROUTE
While we are here, we should also get rid of SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT and just […]
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32485 )
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32485/1/src/soc/intel/common/block/include/i... File src/soc/intel/common/block/include/intelblocks/gpio_defs.h:
https://review.coreboot.org/#/c/32485/1/src/soc/intel/common/block/include/i... PS1, Line 251:
Some macros have the extra indent to indicate it's a continuation line. […]
Aah I see. I like it aligned at same tab level, but that's just my personal opinion. We can leave it as is if you want :).
Hello Patrick Rudolph, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32485
to look at the new patch set (#2).
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
soc/intel/common: Add new PAD_CFG macro.
Added macro named PAD_CFG_GPI_GPIO_DRIVER_SCI, for pads that need to be configured as GPI, GPIO Driver mode, and SCI interrupt.
BUG=none BRANCH=none TEST=Compiles
Change-Id: I0332c64e2fa62ce29c772444606adbfdf9c9afc4 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 1 file changed, 15 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/32485/2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32485 )
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
Patch Set 2:
Apparently that change breaks a bunch of octopus boards who use PAD_CFG_GPI_IRQ_WAKE to assume IOAPIC and SCI routes.
Hello Patrick Rudolph, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32485
to look at the new patch set (#3).
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
soc/intel/common: Add new PAD_CFG macro.
Added macro named PAD_CFG_GPI_GPIO_DRIVER_SCI, for pads that need to be configured as GPI, GPIO Driver mode, and SCI interrupt.
BUG=none BRANCH=none TEST=Compiles
Change-Id: I0332c64e2fa62ce29c772444606adbfdf9c9afc4 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 1 file changed, 15 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/32485/3
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32485 )
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
Patch Set 3:
Okay to assume IOAPIC and SCI for the _IRQ_WAKE() macro?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32485 )
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
Patch Set 3:
(1 comment)
Patch Set 3:
Okay to assume IOAPIC and SCI for the _IRQ_WAKE() macro?
Yes. That was the intent of the marco. _IRQ_ is a bit confusing name, hence I suggesting changing IOAPIC_SCI. IRQ could mean legacy GPIO_IRQ or APIC as well.
https://review.coreboot.org/#/c/32485/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32485/3//COMMIT_MSG@11 PS3, Line 11: Commit message doesn't reflect the other change that is being made. It would be good to mention that too.
Hello Patrick Rudolph, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32485
to look at the new patch set (#4).
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
soc/intel/common: Add new PAD_CFG macro.
Added macro named PAD_CFG_GPI_GPIO_DRIVER_SCI, for pads that need to be configured as GPI, GPIO Driver mode, and SCI interrupt.
Also remove PAD_IRQ_CFG_DUAL_ROUTE macro (subsumed by PAD_CFG_GPI_IRQ_WAKE).
BUG=none BRANCH=none TEST=Compiles
Change-Id: I0332c64e2fa62ce29c772444606adbfdf9c9afc4 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 1 file changed, 15 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/32485/4
Hello Patrick Rudolph, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32485
to look at the new patch set (#5).
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
soc/intel/common: Add new PAD_CFG macro.
Added macro named PAD_CFG_GPI_GPIO_DRIVER_SCI, for pads that need to be configured as GPI, GPIO Driver mode, and SCI interrupt.
Also remove PAD_IRQ_CFG_DUAL_ROUTE macro (subsumed by PAD_CFG_GPI_IRQ_WAKE).
BUG=none BRANCH=none TEST=Compiles
Change-Id: I0332c64e2fa62ce29c772444606adbfdf9c9afc4 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/32485/5
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32485 )
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/32485/5/src/soc/intel/common/block/include/i... File src/soc/intel/common/block/include/intelblocks/gpio_defs.h:
https://review.coreboot.org/#/c/32485/5/src/soc/intel/common/block/include/i... PS5, Line 248: PAD_CFG_GPI_GPIO_DRIVER_SCI Let's still put this under SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT in line 394.
Hello Patrick Rudolph, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32485
to look at the new patch set (#6).
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
soc/intel/common: Add new PAD_CFG macro.
Added macro named PAD_CFG_GPI_GPIO_DRIVER_SCI, for pads that need to be configured as GPI, GPIO Driver mode, and SCI interrupt.
Also remove PAD_IRQ_CFG_DUAL_ROUTE macro (subsumed by PAD_CFG_GPI_IRQ_WAKE).
BUG=none BRANCH=none TEST=Compiles
Change-Id: I0332c64e2fa62ce29c772444606adbfdf9c9afc4 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 1 file changed, 7 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/32485/6
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32485 )
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
Patch Set 6: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32485 )
Change subject: soc/intel/common: Add new PAD_CFG macro. ......................................................................
soc/intel/common: Add new PAD_CFG macro.
Added macro named PAD_CFG_GPI_GPIO_DRIVER_SCI, for pads that need to be configured as GPI, GPIO Driver mode, and SCI interrupt.
Also remove PAD_IRQ_CFG_DUAL_ROUTE macro (subsumed by PAD_CFG_GPI_IRQ_WAKE).
BUG=none BRANCH=none TEST=Compiles
Change-Id: I0332c64e2fa62ce29c772444606adbfdf9c9afc4 Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/32485 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 1 file changed, 7 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h index 91d8f00..e1ddd4b 100644 --- a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h +++ b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h @@ -391,6 +391,13 @@ PAD_IOSSTATE(TxDRxE))
#if CONFIG(SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT) +/* GPI, GPIO Driver, SCI interrupt */ +#define PAD_CFG_GPI_GPIO_DRIVER_SCI(pad, pull, rst, trig, inv) \ + _PAD_CFG_STRUCT(pad, \ + PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \ + PAD_IRQ_CFG(SCI, trig, inv), \ + PAD_PULL(pull) | PAD_CFG1_GPIO_DRIVER | PAD_IOSSTATE(TxDRxE)) + #define PAD_CFG_GPI_DUAL_ROUTE(pad, pull, rst, trig, inv, route1, route2) \ _PAD_CFG_STRUCT(pad, \ PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \
-
Furquan Shaikh (Code Review) -
Patrick Georgi (Code Review)
-
Tim Wawrzynczak (Code Review)