Edward O'Callaghan has uploaded this change for review.

View Change

sb600spi.c: Remove 'amd_gen' out of global state.

Have 'determine_generation()' explicitly return 'amd_gen'
and then pass the state into what requires it. Thus making
the code more pure, easier to read and more unit-testable.

Change-Id: I99fbad9486123c6b921eab83756de54a53ddfa7a
Signed-off-by: Edward O'Callaghan <quasisec@chromium.org>
---
M sb600spi.c
1 file changed, 12 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/36432/1

diff --git a/sb600spi.c b/sb600spi.c
index 8daaf03..23b36ee 100644
--- a/sb600spi.c
+++ b/sb600spi.c
@@ -51,7 +51,6 @@
 	CHIPSET_YANGTZE,
 	CHIPSET_PROMONTORY,
 };
-static enum amd_chipset amd_gen = CHIPSET_AMD_UNKNOWN;
 
 #define FIFO_SIZE_OLD		8
 #define FIFO_SIZE_YANGTZE	71
@@ -110,7 +109,7 @@
 /* Determine the chipset's version and identify the respective SMBUS device. */
 static int determine_generation(struct pci_dev *dev)
 {
-	amd_gen = CHIPSET_AMD_UNKNOWN;
+	enum amd_chipset amd_gen = CHIPSET_AMD_UNKNOWN;
 	msg_pdbg2("Trying to determine the generation of the SPI interface... ");
 	if (dev->device_id == 0x438d) {
 		amd_gen = CHIPSET_SB6XX;
@@ -173,11 +172,10 @@
 			  "Please report this to flashrom@flashrom.org and include this log and\n"
 			  "the output of lspci -nnvx, thanks!\n",
 			  __func__, dev->vendor_id, dev->device_id);
-	if (amd_gen == CHIPSET_AMD_UNKNOWN) {
+	if (amd_gen == CHIPSET_AMD_UNKNOWN)
 		msg_perr("Could not determine chipset generation.");
-		return -1;
-	}
-	return 0;
+
+	return amd_gen;
 }
 
 static void reset_internal_fifo_pointer(void)
@@ -373,7 +371,7 @@
 	{ "800 kHz",	0x07 },
 };
 
-static int set_speed(struct pci_dev *dev, const struct spispeed *spispeed)
+static int set_speed(struct pci_dev *dev, enum amd_chipset amd_gen, const struct spispeed *spispeed)
 {
 	bool success = false;
 	uint8_t speed = spispeed->speed;
@@ -407,7 +405,7 @@
 	return 0;
 }
 
-static int handle_speed(struct pci_dev *dev)
+static int handle_speed(struct pci_dev *dev, enum amd_chipset amd_gen)
 {
 	uint32_t tmp;
 	uint8_t spispeed_idx = 3; /* Default to 16.5 MHz */
@@ -496,10 +494,10 @@
 		tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3;
 		msg_pdbg("NormSpeed is %s\n", spispeeds[tmp].name);
 	}
-	return set_speed(dev, &spispeeds[spispeed_idx]);
+	return set_speed(dev, amd_gen, &spispeeds[spispeed_idx]);
 }
 
-static int handle_imc(struct pci_dev *dev)
+static int handle_imc(struct pci_dev *dev, enum amd_chipset amd_gen)
 {
 	/* Handle IMC everywhere but sb600 which does not have one. */
 	if (amd_gen == CHIPSET_SB6XX)
@@ -592,7 +590,8 @@
 	 */
 	sb600_spibar += tmp & 0xfff;
 
-	if (determine_generation(dev) < 0)
+	int amd_gen = determine_generation(dev);
+	if (amd_gen < 0)
 		return ERROR_NONFATAL;
 
 	/* How to read the following table and similar ones in this file:
@@ -725,10 +724,10 @@
 		return 0;
 	}
 
-	if (handle_speed(dev) != 0)
+	if (handle_speed(dev, amd_gen) != 0)
 		return ERROR_FATAL;
 
-	if (handle_imc(dev) != 0)
+	if (handle_imc(dev, amd_gen) != 0)
 		return ERROR_FATAL;
 
 	/* Starting with Yangtze the SPI controller got a different interface with a much bigger buffer. */

To view, visit change 36432. To unsubscribe, or for help writing mail filters, visit settings.