soc/intel/common/gpio: clean up logical to chipset mapping

1. Explicitly add LOGICAL to the reset macro name to make it explicit
   that the values are logical.
2. Reword some of the comments and combine them into single comment
   instead of scattering the comments throughout.
3. Use c99 struct initializers for the reset mapping array.
4. For the chipset specific values use literals that match the hardware.
5. Use 'U' suffixes on the literals so we don't trip up compiler being
   over zealous on undefined behavior.
6. Use unsigned and fixed-width types for the reset mapping structure
   since the code is reliant on matching up with a register definition.
7. Fix formatting that can fit < 80 cols.

Change-Id: Iaa23a319832c05b8a023f6e45c4ee5ac06dd7066
Signed-off-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-on: https://review.coreboot.org/20589
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-by: Furquan Shaikh <furquan@google.com>

4 files changed, 15 insertions, 18 deletions

diff --git a/src/soc/intel/apollolake/gpio_apl.c b/src/soc/intel/apollolake/gpio_apl.c
index d7d3fe01f2..a774470fc5 100644
--- a/src/soc/intel/apollolake/gpio_apl.c
+++ b/src/soc/intel/apollolake/gpio_apl.c
@@ -22,9 +22,9 @@
 #include <soc/pm.h>
 
 static const struct reset_mapping rst_map[] = {
-	{ PAD_CFG0_RESET_PWROK, PAD_CFG0_RESET_PWROK },
-	{ PAD_CFG0_RESET_DEEP, PAD_CFG0_RESET_DEEP },
-	{ PAD_CFG0_RESET_PLTRST, PAD_CFG0_RESET_PLTRST },
+	{ .logical = PAD_CFG0_LOGICAL_RESET_PWROK, .chipset = 0U << 30 },
+	{ .logical = PAD_CFG0_LOGICAL_RESET_DEEP, .chipset = 1U << 30 },
+	{ .logical = PAD_CFG0_LOGICAL_RESET_PLTRST, .chipset = 2U << 30 },
 };
 
 static const struct pad_community apl_gpio_communities[] = {
diff --git a/src/soc/intel/common/block/gpio/gpio.c b/src/soc/intel/common/block/gpio/gpio.c
index 58bc6d9a81..6e9ae72c58 100644
--- a/src/soc/intel/common/block/gpio/gpio.c
+++ b/src/soc/intel/common/block/gpio/gpio.c
@@ -174,8 +174,7 @@ static uint32_t gpio_pad_reset_config_override(const struct pad_community *comm,
 		return config_value;/* Logical reset values equal chipset
 					values */
 	for (i = 0; i < comm->num_reset_vals; i++, rst_map++) {
-		if ((config_value & PAD_CFG0_RESET_MASK) ==
-				rst_map->logical) {
+		if ((config_value & PAD_CFG0_RESET_MASK) == rst_map->logical) {
 			config_value &= ~PAD_CFG0_RESET_MASK;
 			config_value |= rst_map->chipset;
 			return config_value;
diff --git a/src/soc/intel/common/block/include/intelblocks/gpio.h b/src/soc/intel/common/block/include/intelblocks/gpio.h
index c1542be6e7..879d30b469 100644
--- a/src/soc/intel/common/block/include/intelblocks/gpio.h
+++ b/src/soc/intel/common/block/include/intelblocks/gpio.h
@@ -45,12 +45,14 @@ struct pad_config {
 };
 
 /*
- * Structure provides the logical to actual value for PADRSTCFG in DW0
+ * Structure provides the logical to actual value for PADRSTCFG in DW0. Note
+ * that the values are expected to be within the field placement of the register
+ * itself. i.e. if the reset field is at 31:30 then the values within logical
+ * and chipset should occupy 31:30.
  */
 struct reset_mapping {
-	int	logical;/* logical value defined in
-	include/intelblocks/gpio_defs.h - PAD_CFG0_RESET_xxx */
-	int	chipset;/* translation of logical to SOC PADRSTCFG */
+	uint32_t logical;
+	uint32_t chipset;
 };
 
 /* This structure will be used to describe a community or each group within a
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 5085af462d..244c680158 100644
--- a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h
+++ b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h
@@ -50,14 +50,10 @@
 #define PAD_CFG0_RXRAW1_MASK		(1 << 28)
 #define PAD_CFG0_RXPADSTSEL_MASK	(1 << 29)
 #define PAD_CFG0_RESET_MASK		(3 << 30)
-#define  PAD_CFG0_RESET_PWROK		(0 << 30) /* Logical PADRSTCFG value */
-#define  PAD_CFG0_RESET_DEEP		(1 << 30) /* Logical PADRSTCFG value */
-#define  PAD_CFG0_RESET_PLTRST		(2 << 30) /* Logical PADRSTCFG value */
-#define  PAD_CFG0_RESET_RSMRST		(3 << 30) /* Logical PADRSTCFG value */
-/* The PAD_CFG0_RESET_xxx are logical values and the actual chipset values
-   corresponding to these will be replaced by code in
-   soc/intel/common/block/gpio
- */
+#define  PAD_CFG0_LOGICAL_RESET_PWROK		(0U << 30)
+#define  PAD_CFG0_LOGICAL_RESET_DEEP		(1U << 30)
+#define  PAD_CFG0_LOGICAL_RESET_PLTRST		(2U << 30)
+#define  PAD_CFG0_LOGICAL_RESET_RSMRST		(3U << 30)
 
 /* Use the fourth bit in IntSel field to indicate gpio
  * ownership. This field is RO and hence not used during
@@ -118,7 +114,7 @@
 #endif /* CONFIG_SOC_INTEL_COMMON_BLOCK_GPIO_PADCFG_PADTOL */
 
 #define PAD_FUNC(value)		PAD_CFG0_MODE_##value
-#define PAD_RESET(value)	PAD_CFG0_RESET_##value
+#define PAD_RESET(value)	PAD_CFG0_LOGICAL_RESET_##value
 #define PAD_PULL(value)		PAD_CFG1_PULL_##value
 
 #if IS_ENABLED(CONFIG_SOC_INTEL_COMMON_BLOCK_GPIO_IOSTANDBY)