Change in ...coreboot[master]: soc/intel/cannonlake: Fix return values for get_param_value

[Furquan Shaikh (Code Review)]

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

Change subject: soc/intel/cannonlake: Fix return values for get_param_value
......................................................................

soc/intel/cannonlake: Fix return values for get_param_value

Commit 41483c9 (soc/intel/cannonlake: Add required FSP UPD changes for
CML) changed the enum values for PCH_SERIAL_IO_MODE so that 0 is
invalid and valid values start from 1. However, get_param_value was
not updated to correctly subtract 1 before returning any value. This
change adds a macro PCH_SERIAL_IO_INDEX to apply the subtract 1
operation on any value that get_param_value needs to return.

BUG=b:128946016
TEST=Verified that hatch boots successfully.

Change-Id: I4e32fcd1efe4a535251f0ec58662a2dc5f70e8b0
Signed-off-by: Furquan Shaikh 
---
M src/soc/intel/cannonlake/fsp_params.c
1 file changed, 9 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/31974/1

diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c
index e3a2310..77d82d6 100644
--- a/src/soc/intel/cannonlake/fsp_params.c
+++ b/src/soc/intel/cannonlake/fsp_params.c
@@ -40,22 +40,28 @@
    PCH_DEVFN_UART2
 };

+/*
+ * Given an enum for PCH_SERIAL_IO_MODE, 1 needs to be subtracted to get the FSP
+ * UPD expected value for Serial IO since valid enum index starts from 1.
+ */
+#define PCH_SERIAL_IO_INDEX(x)		((x) - 1)
+
 static uint8_t get_param_value(const config_t *config, uint32_t dev_offset)
 {
    struct device *dev;

dev = dev_find_slot(0, serial_io_dev[dev_offset]);
    if (!dev || !dev->enabled)
-		return PchSerialIoDisabled;
+		return PCH_SERIAL_IO_INDEX(PchSerialIoDisabled);

if ((config->SerialIoDevMode[dev_offset] >= PchSerialIoMax) ||
        (config->SerialIoDevMode[dev_offset] == PchSerialIoNotInitialized))
-		return PchSerialIoPci;
+		return PCH_SERIAL_IO_INDEX(PchSerialIoPci);

/*
     * Correct Enum index starts from 1, so subtract 1 while returning value
     */
-	return config->SerialIoDevMode[dev_offset] - 1;
+	return PCH_SERIAL_IO_INDEX(config->SerialIoDevMode[dev_offset]);
 }

#if IS_ENABLED(CONFIG_SOC_INTEL_COMETLAKE)

[Duncan Laurie (Code Review)]

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31974 )

Change subject: soc/intel/cannonlake: Fix return values for get_param_value
......................................................................

Patch Set 1: Code-Review+2

[Rizwan Qureshi (Code Review)]

Rizwan Qureshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31974 )

Change subject: soc/intel/cannonlake: Fix return values for get_param_value
......................................................................

Patch Set 1: Code-Review+2

[Maulik V Vaghela (Code Review)]

Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31974 )

Change subject: soc/intel/cannonlake: Fix return values for get_param_value
......................................................................

Patch Set 1: Code-Review+1

[Aaron Durbin (Code Review)]

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

Change subject: soc/intel/cannonlake: Fix return values for get_param_value
......................................................................

Patch Set 1: Code-Review+2

[Furquan Shaikh (Code Review)]

Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31974 )

Change subject: soc/intel/cannonlake: Fix return values for get_param_value
......................................................................

soc/intel/cannonlake: Fix return values for get_param_value

Commit 41483c9 (soc/intel/cannonlake: Add required FSP UPD changes for
CML) changed the enum values for PCH_SERIAL_IO_MODE so that 0 is
invalid and valid values start from 1. However, get_param_value was
not updated to correctly subtract 1 before returning any value. This
change adds a macro PCH_SERIAL_IO_INDEX to apply the subtract 1
operation on any value that get_param_value needs to return.

BUG=b:128946016
TEST=Verified that hatch boots successfully.

Change-Id: I4e32fcd1efe4a535251f0ec58662a2dc5f70e8b0
Signed-off-by: Furquan Shaikh 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31974
Tested-by: build bot (Jenkins) 
Reviewed-by: Duncan Laurie 
Reviewed-by: Rizwan Qureshi 
Reviewed-by: Maulik V Vaghela 
Reviewed-by: Aaron Durbin 
---
M src/soc/intel/cannonlake/fsp_params.c
1 file changed, 9 insertions(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Duncan Laurie: Looks good to me, approved
  Aaron Durbin: Looks good to me, approved
  Rizwan Qureshi: Looks good to me, approved
  Maulik V Vaghela: Looks good to me, but someone else must approve

diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c
index e3a2310..77d82d6 100644
--- a/src/soc/intel/cannonlake/fsp_params.c
+++ b/src/soc/intel/cannonlake/fsp_params.c
@@ -40,22 +40,28 @@
    PCH_DEVFN_UART2
 };

+/*
+ * Given an enum for PCH_SERIAL_IO_MODE, 1 needs to be subtracted to get the FSP
+ * UPD expected value for Serial IO since valid enum index starts from 1.
+ */
+#define PCH_SERIAL_IO_INDEX(x)		((x) - 1)
+
 static uint8_t get_param_value(const config_t *config, uint32_t dev_offset)
 {
    struct device *dev;

dev = dev_find_slot(0, serial_io_dev[dev_offset]);
    if (!dev || !dev->enabled)
-		return PchSerialIoDisabled;
+		return PCH_SERIAL_IO_INDEX(PchSerialIoDisabled);

if ((config->SerialIoDevMode[dev_offset] >= PchSerialIoMax) ||
        (config->SerialIoDevMode[dev_offset] == PchSerialIoNotInitialized))
-		return PchSerialIoPci;
+		return PCH_SERIAL_IO_INDEX(PchSerialIoPci);

/*
     * Correct Enum index starts from 1, so subtract 1 while returning value
     */
-	return config->SerialIoDevMode[dev_offset] - 1;
+	return PCH_SERIAL_IO_INDEX(config->SerialIoDevMode[dev_offset]);
 }

#if IS_ENABLED(CONFIG_SOC_INTEL_COMETLAKE)