Change in ...coreboot[master]: soc/intel/common: Set RX_DISABLE for pads configured as NC
Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32514
Change subject: soc/intel/common: Set RX_DISABLE for pads configured as NC ......................................................................
soc/intel/common: Set RX_DISABLE for pads configured as NC
For GPIO pads that are configured as no-connect (PAD_NC), setting it as GPI (with Rx enabled) leads to GPE0_STS being set incorrectly. Though this is not an issue in practice (GPE0_EN is not set, so no events triggered), it can confuse users when debugging GPE related issues.
This change configures PAD_NC to have Rx disabled along with Tx to ensure that it does not end up setting GPE0_STS bits for unwanted GPIO pads.
P.S.: IOSSTATE config does not have a TxDRxD setting, so leaving that configuration as is.
BUG=b:129235068 TEST=Verified that GPE0_STS bits are not set for pads that are marked as PAD_NC.
Change-Id: I726cc7b86a94e7449352cd8a8806d4d775c593dc Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 1 file changed, 9 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/32514/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..91d8f00 100644 --- a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h +++ b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h @@ -264,10 +264,16 @@ PAD_CFG0_TRIG_##trig | PAD_CFG0_RX_POL_NONE, \ PAD_PULL(pull) | PAD_CFG1_GPIO_DRIVER | PAD_IOSSTATE(TxDRxE))
-/* No Connect configuration for unused pad. - * NC should be GPI with Term as PU20K, PD20K, NONE depending upon default Term +/* + * No Connect configuration for unused pad. + * Both TX and RX are disabled. RX disabling is done to avoid unnecessary + * setting of GPI_STS. */ -#define PAD_NC(pad, pull) PAD_CFG_GPI(pad, pull, DEEP) +#define PAD_NC(pad, pull) \ + _PAD_CFG_STRUCT(pad, \ + PAD_FUNC(GPIO) | PAD_RESET(DEEP) | \ + PAD_CFG0_TX_DISABLE | PAD_CFG0_RX_DISABLE, \ + PAD_PULL(pull) | PAD_IOSSTATE(TxDRxE))
#if CONFIG(SOC_INTEL_COMMON_BLOCK_GPIO_LEGACY_MACROS)
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32514 )
Change subject: soc/intel/common: Set RX_DISABLE for pads configured as NC ......................................................................
Patch Set 1: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32514 )
Change subject: soc/intel/common: Set RX_DISABLE for pads configured as NC ......................................................................
Patch Set 1: Code-Review+2
Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32514 )
Change subject: soc/intel/common: Set RX_DISABLE for pads configured as NC ......................................................................
Patch Set 1: Code-Review+1
Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32514 )
Change subject: soc/intel/common: Set RX_DISABLE for pads configured as NC ......................................................................
soc/intel/common: Set RX_DISABLE for pads configured as NC
For GPIO pads that are configured as no-connect (PAD_NC), setting it as GPI (with Rx enabled) leads to GPE0_STS being set incorrectly. Though this is not an issue in practice (GPE0_EN is not set, so no events triggered), it can confuse users when debugging GPE related issues.
This change configures PAD_NC to have Rx disabled along with Tx to ensure that it does not end up setting GPE0_STS bits for unwanted GPIO pads.
P.S.: IOSSTATE config does not have a TxDRxD setting, so leaving that configuration as is.
BUG=b:129235068 TEST=Verified that GPE0_STS bits are not set for pads that are marked as PAD_NC.
Change-Id: I726cc7b86a94e7449352cd8a8806d4d775c593dc Signed-off-by: Furquan Shaikh furquan@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32514 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Duncan Laurie dlaurie@chromium.org Reviewed-by: Subrata Banik subrata.banik@intel.com Reviewed-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 1 file changed, 9 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Duncan Laurie: Looks good to me, approved Subrata Banik: Looks good to me, approved Ronak Kanabar: Looks good to me, but someone else must approve
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..91d8f00 100644 --- a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h +++ b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h @@ -264,10 +264,16 @@ PAD_CFG0_TRIG_##trig | PAD_CFG0_RX_POL_NONE, \ PAD_PULL(pull) | PAD_CFG1_GPIO_DRIVER | PAD_IOSSTATE(TxDRxE))
-/* No Connect configuration for unused pad. - * NC should be GPI with Term as PU20K, PD20K, NONE depending upon default Term +/* + * No Connect configuration for unused pad. + * Both TX and RX are disabled. RX disabling is done to avoid unnecessary + * setting of GPI_STS. */ -#define PAD_NC(pad, pull) PAD_CFG_GPI(pad, pull, DEEP) +#define PAD_NC(pad, pull) \ + _PAD_CFG_STRUCT(pad, \ + PAD_FUNC(GPIO) | PAD_RESET(DEEP) | \ + PAD_CFG0_TX_DISABLE | PAD_CFG0_RX_DISABLE, \ + PAD_PULL(pull) | PAD_IOSSTATE(TxDRxE))
#if CONFIG(SOC_INTEL_COMMON_BLOCK_GPIO_LEGACY_MACROS)
-
Duncan Laurie (Code Review) -
Furquan Shaikh (Code Review)
-
Ronak Kanabar (Code Review)
-
Subrata Banik (Code Review)