Change in ...coreboot[master]: soc/intel/common/gpio_defs: Enable configuring GPIO_DW2 pad register

[Karthik Ramasubramanian (Code Review)]

Hello Karthikeyan Ramasubramanian,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/30449

to review the following change.

Change subject: soc/intel/common/gpio_defs: Enable configuring GPIO_DW2 pad register
......................................................................

soc/intel/common/gpio_defs: Enable configuring GPIO_DW2 pad register

Currently all the helpers support configuring GPIO_DW0/1 registers. In
some architectures there is an additional configuration GPIO_DW2 register
that can be used to configure debounce duration etc. Update the helpers
to enable configuring GPIO_DW2 pad register.

BRANCH=octopus
BUG=b:117953118
TEST=Ensure that the system boots to ChromeOS. Ensure that the current
configuration is not disturbed by turning on the GPIO_DEBUG option and
verifying the debug output before and after the change.

Change-Id: I3e5d259d007fdc83940a43cc4cd4a2b8a547d334
Signed-off-by: Karthikeyan Ramasubramanian 
---
M src/soc/intel/common/block/include/intelblocks/gpio_defs.h
1 file changed, 42 insertions(+), 25 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/49/30449/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 418b6ab..153a9da 100644
--- a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h
+++ b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h
@@ -107,6 +107,8 @@
 #define PAD_CFG1_IOSSTATE_MASK		0
 #endif /* CONFIG_SOC_INTEL_COMMON_BLOCK_GPIO_IOSTANDBY */

+#define PAD_CFG2_NONE			0
+
 /* voltage tolerance  0=3.3V default 1=1.8V tolerant */
 #if IS_ENABLED(CONFIG_SOC_INTEL_COMMON_BLOCK_GPIO_PADCFG_PADTOL)
 #define PAD_CFG1_TOL_MASK		(0x1 << 25)
@@ -138,17 +140,27 @@
    			PAD_CFG0_RX_POL_##inv)
 #endif /* CONFIG_SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT */

-#define _PAD_CFG_STRUCT(__pad, __config0, __config1)	\
+#if GPIO_NUM_PAD_CFG_REGS > 2
+#define _PAD_CFG_STRUCT(__pad, __config0, __config1, __config2)	\
+	{					\
+		.pad = __pad,			\
+		.pad_config[0] = __config0,	\
+		.pad_config[1] = __config1,	\
+		.pad_config[2] = __config2,	\
+	}
+#else
+#define _PAD_CFG_STRUCT(__pad, __config0, __config1, __config2)	\
    {					\
    	.pad = __pad,			\
    	.pad_config[0] = __config0,	\
    	.pad_config[1] = __config1,	\
    }
+#endif

/* Native function configuration */
 #define PAD_CFG_NF(pad, pull, rst, func) \
    _PAD_CFG_STRUCT(pad, PAD_RESET(rst) | PAD_FUNC(func), PAD_PULL(pull) | \
-		PAD_IOSSTATE(TxLASTRxE))
+		PAD_IOSSTATE(TxLASTRxE), PAD_CFG2_NONE)

#if IS_ENABLED(CONFIG_SOC_INTEL_COMMON_BLOCK_GPIO_PADCFG_PADTOL)
 /* Native 1.8V tolerant pad, only applies to some pads like I2C/I2S
@@ -156,55 +168,57 @@
  */
 #define PAD_CFG_NF_1V8(pad, pull, rst, func) \
    _PAD_CFG_STRUCT(pad, PAD_RESET(rst) | PAD_FUNC(func), PAD_PULL(pull) |\
-		PAD_IOSSTATE(TxLASTRxE) | PAD_CFG1_TOL_1V8)
+		PAD_IOSSTATE(TxLASTRxE) | PAD_CFG1_TOL_1V8, PAD_CFG2_NONE)
 #endif

/* Native function configuration for standby state */
 #define PAD_CFG_NF_IOSSTATE(pad, pull, rst, func, iosstate) \
    _PAD_CFG_STRUCT(pad, PAD_RESET(rst) | PAD_FUNC(func), PAD_PULL(pull) | \
-		PAD_IOSSTATE(iosstate))
+		PAD_IOSSTATE(iosstate), PAD_CFG2_NONE)

/* Native function configuration for standby state, also configuring
    iostandby as masked */
 #define PAD_CFG_NF_IOSTANDBY_IGNORE(pad, pull, rst, func) \
    _PAD_CFG_STRUCT(pad, PAD_RESET(rst) | PAD_FUNC(func), PAD_PULL(pull) | \
-		PAD_IOSSTATE(IGNORE))
+		PAD_IOSSTATE(IGNORE), PAD_CFG2_NONE)

/* Native function configuration for standby state, also configuring
    iosstate and iosterm */
 #define PAD_CFG_NF_IOSSTATE_IOSTERM(pad, pull, rst, func, iosstate, iosterm) \
    _PAD_CFG_STRUCT(pad, PAD_RESET(rst) | PAD_FUNC(func), PAD_PULL(pull) | \
-		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm))
+		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm), PAD_CFG2_NONE)

/* General purpose output, no pullup/down. */
 #define PAD_CFG_GPO(pad, val, rst)	\
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_RX_DISABLE | !!val, \
-		PAD_PULL(NONE) | PAD_IOSSTATE(TxLASTRxE))
+		PAD_PULL(NONE) | PAD_IOSSTATE(TxLASTRxE), PAD_CFG2_NONE)

/* General purpose output, with termination specified */
 #define PAD_CFG_TERM_GPO(pad, val, pull, rst)	\
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_RX_DISABLE | !!val, \
-		PAD_PULL(pull) | PAD_IOSSTATE(TxLASTRxE))
+		PAD_PULL(pull) | PAD_IOSSTATE(TxLASTRxE), PAD_CFG2_NONE)

/* General purpose output, no pullup/down. */
 #define PAD_CFG_GPO_GPIO_DRIVER(pad, val, rst, pull)	\
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_RX_DISABLE | !!val, \
-		PAD_PULL(pull) | PAD_IOSSTATE(TxLASTRxE) | PAD_CFG1_GPIO_DRIVER)
+		PAD_PULL(pull) | PAD_IOSSTATE(TxLASTRxE) |	\
+		PAD_CFG1_GPIO_DRIVER, PAD_CFG2_NONE)

/* General purpose output. */
 #define PAD_CFG_GPO_IOSSTATE_IOSTERM(pad, val, rst, pull, iosstate, ioterm)	\
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_RX_DISABLE | !!val, \
-		PAD_PULL(pull) | PAD_IOSSTATE(iosstate) | PAD_IOSTERM(ioterm))
+		PAD_PULL(pull) | PAD_IOSSTATE(iosstate) | PAD_IOSTERM(ioterm), \
+		PAD_CFG2_NONE)

/* General purpose input */
 #define PAD_CFG_GPI(pad, pull, rst) \
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE, \
-		PAD_PULL(pull) | PAD_IOSSTATE(TxDRxE))
+		PAD_PULL(pull) | PAD_IOSSTATE(TxDRxE), PAD_CFG2_NONE)

/* General purpose input. The following macro sets the
  * Host Software Pad Ownership to GPIO Driver mode.
@@ -212,27 +226,29 @@
 #define PAD_CFG_GPI_GPIO_DRIVER(pad, pull, rst) \
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE, \
-		PAD_PULL(pull) | PAD_CFG1_GPIO_DRIVER | PAD_IOSSTATE(TxDRxE))
+		PAD_PULL(pull) | PAD_CFG1_GPIO_DRIVER | PAD_IOSSTATE(TxDRxE), \
+		PAD_CFG2_NONE)

#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 |	\
    	PAD_CFG0_RX_DISABLE,					\
    	PAD_PULL(pull) | PAD_CFG1_GPIO_DRIVER |			\
-		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm))
+		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm), PAD_CFG2_NONE)

#define PAD_CFG_GPIO_HI_Z(pad, pull, rst, iosstate, iosterm) \
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE |	\
    	PAD_CFG0_RX_DISABLE, PAD_PULL(pull) |			\
-		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm))
+		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm), PAD_CFG2_NONE)

/* GPIO Interrupt */
 #define PAD_CFG_GPI_INT(pad, pull, rst, trig) \
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE |	\
    		PAD_CFG0_TRIG_##trig | PAD_CFG0_RX_POL_NONE,	\
-		PAD_PULL(pull) | PAD_CFG1_GPIO_DRIVER | PAD_IOSSTATE(TxDRxE))
+		PAD_PULL(pull) | PAD_CFG1_GPIO_DRIVER | PAD_IOSSTATE(TxDRxE), \
+		PAD_CFG2_NONE)

/* No Connect configuration for unused pad.
  * NC should be GPI with Term as PU20K, PD20K, NONE depending upon default Term
@@ -244,12 +260,13 @@
 #define PAD_CFG_GPI_APIC(pad, pull, rst) \
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \
-		PAD_IRQ_CFG(IOAPIC, LEVEL, NONE), PAD_PULL(pull))
+		PAD_IRQ_CFG(IOAPIC, LEVEL, NONE), PAD_PULL(pull), PAD_CFG2_NONE)

#define PAD_CFG_GPI_APIC_INVERT(pad, pull, rst) \
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \
-		PAD_IRQ_CFG(IOAPIC, LEVEL, INVERT), PAD_PULL(pull))
+		PAD_IRQ_CFG(IOAPIC, LEVEL, INVERT), PAD_PULL(pull),	\
+		PAD_CFG2_NONE)

#define PAD_CFG_GPI_ACPI_SCI(pad, pull, rst, inv)	\
    PAD_CFG_GPI_SCI(pad, pull, rst, EDGE_SINGLE, inv)
@@ -271,7 +288,7 @@
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \
    	PAD_IRQ_CFG(IOAPIC, trig, inv), PAD_PULL(pull) | \
-		PAD_IOSSTATE(TxDRxE))
+		PAD_IOSSTATE(TxDRxE), PAD_CFG2_NONE)
 #endif

/* General purpose input, routed to APIC - with IOStandby Config*/
@@ -279,7 +296,7 @@
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \
    	PAD_IRQ_CFG(IOAPIC, trig, inv), PAD_PULL(pull) | \
-		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm))
+		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm), PAD_CFG2_NONE)

/*
  * The following APIC macros assume the APIC will handle the filtering
@@ -300,14 +317,14 @@
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \
    	PAD_IRQ_CFG(SMI, trig, inv), PAD_PULL(pull) | \
-		PAD_IOSSTATE(TxDRxE))
+		PAD_IOSSTATE(TxDRxE), PAD_CFG2_NONE)

/* General purpose input, routed to SMI */
 #define PAD_CFG_GPI_SMI_IOS(pad, pull, rst, trig, inv, iosstate, iosterm) \
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \
    	PAD_IRQ_CFG(SMI, trig, inv), PAD_PULL(pull) | \
-		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm))
+		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm), PAD_CFG2_NONE)

#define PAD_CFG_GPI_SMI_LOW(pad, pull, rst, trig) \
    PAD_CFG_GPI_SMI(pad, pull, rst, trig, INVERT)
@@ -320,14 +337,14 @@
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \
    	PAD_IRQ_CFG(SCI, trig, inv), PAD_PULL(pull) | \
-		PAD_IOSSTATE(TxDRxE))
+		PAD_IOSSTATE(TxDRxE), PAD_CFG2_NONE)

/* General purpose input, routed to SCI */
 #define PAD_CFG_GPI_SCI_IOS(pad, pull, rst, trig, inv, iosstate, iosterm) \
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \
    	PAD_IRQ_CFG(SCI, trig, inv), PAD_PULL(pull) | \
-		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm))
+		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm), PAD_CFG2_NONE)

#define PAD_CFG_GPI_SCI_LOW(pad, pull, rst, trig) \
    PAD_CFG_GPI_SCI(pad, pull, rst, trig, INVERT)
@@ -340,14 +357,14 @@
    _PAD_CFG_STRUCT(pad,		\
    	PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TX_DISABLE | \
    	PAD_IRQ_CFG(NMI, trig, inv), PAD_PULL(pull) | \
-		PAD_IOSSTATE(TxDRxE))
+		PAD_IOSSTATE(TxDRxE), PAD_CFG2_NONE)

#if IS_ENABLED(CONFIG_SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT)
 #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 | \
    	PAD_IRQ_CFG_DUAL_ROUTE(route1, route2,  trig, inv), \
-		PAD_PULL(pull) | PAD_IOSSTATE(TxDRxE))
+		PAD_PULL(pull) | PAD_IOSSTATE(TxDRxE), PAD_CFG2_NONE)

#define PAD_CFG_GPI_IRQ_WAKE(pad, pull, rst, trig, inv)	\
    PAD_CFG_GPI_DUAL_ROUTE(pad, pull, rst, trig, inv, IOAPIC, SCI)

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30449 )

Change subject: soc/intel/common/gpio_defs: Enable configuring GPIO_DW2 pad register
......................................................................

Patch Set 2: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30449 )

Change subject: soc/intel/common/gpio_defs: Enable configuring GPIO_DW2 pad register
......................................................................

soc/intel/common/gpio_defs: Enable configuring GPIO_DW2 pad register

Currently all the helpers support configuring GPIO_DW0/1 registers. In
some architectures there is an additional configuration GPIO_DW2 register
that can be used to configure debounce duration etc. Add a helper macro
to enable configuring GPIO_DW2 pad register.

BRANCH=octopus
BUG=b:117953118
TEST=Ensure that the system boots to ChromeOS. Ensure that the current
configuration is not disturbed by turning on the GPIO_DEBUG option and
verifying the debug output before and after the change.

Change-Id: I3e5d259d007fdc83940a43cc4cd4a2b8a547d334
Signed-off-by: Karthikeyan Ramasubramanian 
Reviewed-on: https://review.coreboot.org/c/30449
Tested-by: build bot (Jenkins) 
Reviewed-by: Furquan Shaikh 
---
M src/soc/intel/common/block/include/intelblocks/gpio_defs.h
1 file changed, 13 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 418b6ab..6aeef04 100644
--- a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h
+++ b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h
@@ -145,6 +145,19 @@
    	.pad_config[1] = __config1,	\
    }

+#if GPIO_NUM_PAD_CFG_REGS > 2
+#define _PAD_CFG_STRUCT_3(__pad, __config0, __config1, __config2)	\
+	{					\
+		.pad = __pad,			\
+		.pad_config[0] = __config0,	\
+		.pad_config[1] = __config1,	\
+		.pad_config[2] = __config2,	\
+	}
+#else
+#define _PAD_CFG_STRUCT_3(__pad, __config0, __config1, __config2)	\
+	_PAD_CFG_STRUCT(__pad, __config0, __config1)
+#endif
+
 /* Native function configuration */
 #define PAD_CFG_NF(pad, pull, rst, func) \
    _PAD_CFG_STRUCT(pad, PAD_RESET(rst) | PAD_FUNC(func), PAD_PULL(pull) | \