Change in coreboot[master]: soc/amd/common/gpio: Add helper macros for setting fields of soc_amd_...

[Furquan Shaikh (Code Review)]

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42872 )

Change subject: soc/amd/common/gpio: Add helper macros for setting fields of soc_amd_gpio
......................................................................

soc/amd/common/gpio: Add helper macros for setting fields of soc_amd_gpio

This change adds helper macro PAD_CFG_STRUCT for setting the fields
of `soc_amd_gpio`. Additionally, macros are added for different
operations i.e. pull, output, trigger, int_enable, event_trigger,
wake_enable, debounce, etc. All GPIO configuration macros are updated
to use PAD_CFG_STRUCT instead of setting the fields directly.

BUG=b:159944426

Change-Id: I03535d2da0c05f72c4163fa30d72f9c6df44908b
Signed-off-by: Furquan Shaikh 
---
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
1 file changed, 47 insertions(+), 47 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/42872/1

diff --git a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
index 027238d..96dcc5b 100644
--- a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
+++ b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
@@ -245,61 +245,61 @@
  *	debounce_time	the debounce time
  */

+#define PAD_CFG_STRUCT(__pin, __function, __control, __flags)	\
+	{							\
+		.gpio = __pin,					\
+		.function = __function,			\
+		.control = __control,				\
+		.flags = __flags,				\
+	}
+
+#define PAD_PULL(__pull)		GPIO_PULL_ ## __pull
+#define PAD_OUTPUT(__dir)		GPIO_OUTPUT_OUT_ ## __dir
+#define PAD_TRIGGER(__trig)		GPIO_TRIGGER_ ## __trig
+#define PAD_INT_ENABLE(__action)	GPIO_INT_ENABLE_ ## __action
+#define PAD_FLAG_EVENT_TRIGGER(__trig)	GPIO_FLAG_EVENT_TRIGGER_ ## __trig
+#define PAD_WAKE_ENABLE(__wake)	GPIO_WAKE_ ## __wake
+#define PAD_DEBOUNCE_CONFIG(__deb)	GPIO_DEB_ ## __deb
+
 /* Native function pad configuration */
-#define PAD_NF(pin, func, pull) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## func, \
-		.control = GPIO_PULL ## _ ## pull, \
-		.flags = 0 }
+#define PAD_NF(pin, func, pull)						\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_ ## func, PAD_PULL(pull), 0)
+
 /* General purpose input pad configuration */
-#define PAD_GPI(pin, pull) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = GPIO_PULL ## _ ## pull, \
-		.flags = 0 }
+#define PAD_GPI(pin, pull)							\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx, PAD_PULL(pull), 0)
+
 /* General purpose output pad configuration */
-#define PAD_GPO(pin, direction) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = GPIO_OUTPUT ## _OUT_ ## direction, \
-		.flags = 0 }
-/* Auxiliary macro for legacy interrupt and wake */
-#define PAD_AUX1(pull, trigger) (GPIO_PULL ## _ ## pull | \
-				 GPIO_TRIGGER ## _ ## trigger)
+#define PAD_GPO(pin, direction)			\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx, PAD_OUTPUT(direction), 0)
+
 /* Legacy interrupt pad configuration */
-#define PAD_INT(pin, pull, trigger, action) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = (PAD_AUX1(pull, trigger) | \
-			    GPIO_INT_ENABLE ## _ ## action), \
-		.flags = GPIO_FLAG_INT }
-/* Auxiliary macro for SCI and SMI */
-#define PAD_AUX2(trigger, flag) (GPIO_FLAG_EVENT_TRIGGER ## _ ## trigger | flag)
+#define PAD_INT(pin, pull, trigger, action)				\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx,			\
+		PAD_PULL(pull) | PAD_TRIGGER(trigger) | PAD_INT_ENABLE(action), \
+		GPIO_FLAG_INT)
+
 /* SCI pad configuration */
-#define PAD_SCI(pin, pull, trigger) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = GPIO_PULL ## _ ## pull, \
-		.flags = PAD_AUX2(trigger, GPIO_FLAG_SCI) }
+#define PAD_SCI(pin, pull, trigger)					\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx, PAD_PULL(pull),	\
+		PAD_FLAG_EVENT_TRIGGER(trigger) | GPIO_FLAG_SCI)
+
 /* SMI pad configuration */
-#define PAD_SMI(pin, pull, trigger) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = GPIO_PULL ## _ ## pull, \
-		.flags = PAD_AUX2(trigger, GPIO_FLAG_SMI) }
+#define PAD_SMI(pin, pull, trigger)					\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx, PAD_PULL(pull),	\
+		PAD_FLAG_EVENT_TRIGGER(trigger) | GPIO_FLAG_SMI)
+
 /* WAKE pad configuration */
-#define PAD_WAKE(pin, pull, trigger, type) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = (PAD_AUX1(pull, trigger) | \
-			    GPIO_WAKE ## _ ## type), \
-		.flags = GPIO_FLAG_WAKE }
+#define PAD_WAKE(pin, pull, trigger, type)				\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx,			\
+		PAD_PULL(pull) | PAD_TRIGGER(trigger) | PAD_WAKE_ENABLE(type), \
+		GPIO_FLAG_WAKE)
+
 /* pin debounce configuration */
-#define PAD_DEBOUNCE(pin, type, time) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = (GPIO_DEB  ## _ ## type | GPIO_DEB  ## _ ## time), \
-		.flags = GPIO_FLAG_DEBOUNCE }
+#define PAD_DEBOUNCE(pin, type, time)					\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx,			\
+			PAD_DEBOUNCE_CONFIG(type) | PAD_DEBOUNCE_CONFIG(time), \
+			GPIO_FLAG_DEBOUNCE)

typedef uint32_t gpio_t;

[Furquan Shaikh (Code Review)]

Furquan Shaikh has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/42872 )

Change subject: soc/amd/common/gpio: Add helper macros for setting fields of soc_amd_gpio
......................................................................

soc/amd/common/gpio: Add helper macros for setting fields of soc_amd_gpio

This change adds helper macro PAD_CFG_STRUCT for setting the fields
of `soc_amd_gpio`. Additionally, macros are added for different
operations i.e. pull, output, trigger, int_enable, event_trigger,
wake_enable, debounce, etc. All GPIO configuration macros are updated
to use PAD_CFG_STRUCT instead of setting the fields directly.

BUG=b:159944426

Change-Id: I03535d2da0c05f72c4163fa30d72f9c6df44908b
Signed-off-by: Furquan Shaikh 
---
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
1 file changed, 47 insertions(+), 47 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/42872/2

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42872 )

Change subject: soc/amd/common/gpio: Add helper macros for setting fields of soc_amd_gpio
......................................................................

Patch Set 2: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/42872/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/42872/2//COMMIT_MSG@7 
PS2, Line 7: soc/amd/common/gpio: Add helper macros for setting fields of soc_amd_gpio
wraps, could be one character too long subject

Change in coreboot[master]: soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio

[Furquan Shaikh (Code Review)]

Hello build bot (Jenkins), Raul Rangel, Kyösti Mälkki, Aaron Durbin,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

Change subject: soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio
......................................................................

soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio

This change adds helper macro PAD_CFG_STRUCT for setting the fields
of `soc_amd_gpio`. Additionally, macros are added for different
operations i.e. pull, output, trigger, int_enable, event_trigger,
wake_enable, debounce, etc. All GPIO configuration macros are updated
to use PAD_CFG_STRUCT instead of setting the fields directly.

BUG=b:159944426

Change-Id: I03535d2da0c05f72c4163fa30d72f9c6df44908b
Signed-off-by: Furquan Shaikh 
---
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
1 file changed, 47 insertions(+), 47 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/42872/3

Change in coreboot[master]: soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio

[Furquan Shaikh (Code Review)]

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

Change subject: soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42872/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/42872/2//COMMIT_MSG@7 
PS2, Line 7: soc/amd/common/gpio: Add helper macros for setting fields of soc_amd_gpio
> wraps, could be one character too long subject

Done

Change in coreboot[master]: soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42872 )

Change subject: soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio
......................................................................

Patch Set 3: Code-Review+1

Change in coreboot[master]: soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42872 )

Change subject: soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42872 )

Change subject: soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio
......................................................................

Patch Set 4: Code-Review+2

Change in coreboot[master]: soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio

[Furquan Shaikh (Code Review)]

Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42872 )

Change subject: soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio
......................................................................

soc/amd/common/gpio: Add macros for setting fields of soc_amd_gpio

This change adds helper macro PAD_CFG_STRUCT for setting the fields
of `soc_amd_gpio`. Additionally, macros are added for different
operations i.e. pull, output, trigger, int_enable, event_trigger,
wake_enable, debounce, etc. All GPIO configuration macros are updated
to use PAD_CFG_STRUCT instead of setting the fields directly.

BUG=b:159944426

Change-Id: I03535d2da0c05f72c4163fa30d72f9c6df44908b
Signed-off-by: Furquan Shaikh 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42872
Reviewed-by: Aaron Durbin 
Reviewed-by: Kyösti Mälkki 
Reviewed-by: Raul Rangel 
Tested-by: build bot (Jenkins) 
---
M src/soc/amd/common/block/include/amdblocks/gpio_banks.h
1 file changed, 47 insertions(+), 47 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Kyösti Mälkki: Looks good to me, but someone else must approve
  Aaron Durbin: Looks good to me, approved
  Raul Rangel: Looks good to me, approved

diff --git a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
index 6857c22..70f1db3 100644
--- a/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
+++ b/src/soc/amd/common/block/include/amdblocks/gpio_banks.h
@@ -245,61 +245,61 @@
  *	debounce_time	the debounce time
  */

+#define PAD_CFG_STRUCT(__pin, __function, __control, __flags)	\
+	{							\
+		.gpio = __pin,					\
+		.function = __function,				\
+		.control = __control,				\
+		.flags = __flags,				\
+	}
+
+#define PAD_PULL(__pull)		GPIO_PULL_ ## __pull
+#define PAD_OUTPUT(__dir)		GPIO_OUTPUT_OUT_ ## __dir
+#define PAD_TRIGGER(__trig)		GPIO_TRIGGER_ ## __trig
+#define PAD_INT_ENABLE(__action)	GPIO_INT_ENABLE_ ## __action
+#define PAD_FLAG_EVENT_TRIGGER(__trig)	GPIO_FLAG_EVENT_TRIGGER_ ## __trig
+#define PAD_WAKE_ENABLE(__wake)		GPIO_WAKE_ ## __wake
+#define PAD_DEBOUNCE_CONFIG(__deb)	GPIO_DEB_ ## __deb
+
 /* Native function pad configuration */
-#define PAD_NF(pin, func, pull) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## func, \
-		.control = GPIO_PULL ## _ ## pull, \
-		.flags = 0 }
+#define PAD_NF(pin, func, pull)						\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_ ## func, PAD_PULL(pull), 0)
+
 /* General purpose input pad configuration */
-#define PAD_GPI(pin, pull) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = GPIO_PULL ## _ ## pull, \
-		.flags = 0 }
+#define PAD_GPI(pin, pull)							\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx, PAD_PULL(pull), 0)
+
 /* General purpose output pad configuration */
-#define PAD_GPO(pin, direction) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = GPIO_OUTPUT ## _OUT_ ## direction, \
-		.flags = 0 }
-/* Auxiliary macro for legacy interrupt and wake */
-#define PAD_AUX1(pull, trigger) (GPIO_PULL ## _ ## pull | \
-				 GPIO_TRIGGER ## _ ## trigger)
+#define PAD_GPO(pin, direction)			\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx, PAD_OUTPUT(direction), 0)
+
 /* Legacy interrupt pad configuration */
-#define PAD_INT(pin, pull, trigger, action) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = (PAD_AUX1(pull, trigger) | \
-			    GPIO_INT_ENABLE ## _ ## action), \
-		.flags = GPIO_FLAG_INT }
-/* Auxiliary macro for SCI and SMI */
-#define PAD_AUX2(trigger, flag) (GPIO_FLAG_EVENT_TRIGGER ## _ ## trigger | flag)
+#define PAD_INT(pin, pull, trigger, action)				\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx,			\
+		PAD_PULL(pull) | PAD_TRIGGER(trigger) | PAD_INT_ENABLE(action), \
+		GPIO_FLAG_INT)
+
 /* SCI pad configuration */
-#define PAD_SCI(pin, pull, trigger) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = GPIO_PULL ## _ ## pull, \
-		.flags = PAD_AUX2(trigger, GPIO_FLAG_SCI) }
+#define PAD_SCI(pin, pull, trigger)					\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx, PAD_PULL(pull),	\
+		PAD_FLAG_EVENT_TRIGGER(trigger) | GPIO_FLAG_SCI)
+
 /* SMI pad configuration */
-#define PAD_SMI(pin, pull, trigger) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = GPIO_PULL ## _ ## pull, \
-		.flags = PAD_AUX2(trigger, GPIO_FLAG_SMI) }
+#define PAD_SMI(pin, pull, trigger)					\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx, PAD_PULL(pull),	\
+		PAD_FLAG_EVENT_TRIGGER(trigger) | GPIO_FLAG_SMI)
+
 /* WAKE pad configuration */
-#define PAD_WAKE(pin, pull, trigger, type) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = (PAD_AUX1(pull, trigger) | \
-			    GPIO_WAKE ## _ ## type), \
-		.flags = GPIO_FLAG_WAKE }
+#define PAD_WAKE(pin, pull, trigger, type)				\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx,			\
+		PAD_PULL(pull) | PAD_TRIGGER(trigger) | PAD_WAKE_ENABLE(type), \
+		GPIO_FLAG_WAKE)
+
 /* pin debounce configuration */
-#define PAD_DEBOUNCE(pin, type, time) \
-	{ .gpio = (pin), \
-		.function = pin ## _IOMUX_ ## GPIOxx, \
-		.control = (GPIO_DEB  ## _ ## type | GPIO_DEB  ## _ ## time), \
-		.flags = GPIO_FLAG_DEBOUNCE }
+#define PAD_DEBOUNCE(pin, type, time)					\
+	PAD_CFG_STRUCT(pin, pin ## _IOMUX_GPIOxx,			\
+			PAD_DEBOUNCE_CONFIG(type) | PAD_DEBOUNCE_CONFIG(time), \
+			GPIO_FLAG_DEBOUNCE)

typedef uint32_t gpio_t;