Anastasia Klimchuk has uploaded this change for review.

View Change

Simplifying the initialisation flow for it85spi

1) Inlining it85xx_spi_common_init since it's only used once in
it85xx_spi_init, after inlining ret value is not needed.

2) Creating it86_init_error section to ensure that data is freed
if error happened in initialisaton flow.

BUG=b:172876667
TEST=builds

Change-Id: Iac1b4ae9c6f34c5265e729ae0a80024800c3c272
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
---
M it85spi.c
1 file changed, 32 insertions(+), 39 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/48196/1

diff --git a/it85spi.c b/it85spi.c
index 6215515..52b09b1 100644
--- a/it85spi.c
+++ b/it85spi.c
@@ -289,11 +289,18 @@
         .write_aai      = default_spi_write_aai,
 };
 
-static int it85xx_spi_common_init(struct superio s)
+int it85xx_spi_init(struct superio s)
 {
 	chipaddr base;
+	struct it85spi_data *data;
+	msg_pdbg("%s():%d superio.vendor=0x%02x\n", __func__, __LINE__, s.vendor);
 
-	struct it85spi_data *data = calloc(1, sizeof(struct it85spi_data));
+	if (!(internal_buses_supported & BUS_FWH)) {
+		msg_pdbg("%s():%d buses not support FWH\n", __func__, __LINE__);
+		return 1;
+	}
+
+	data = calloc(1, sizeof(struct it85spi_data));
 	if (!data) {
 		msg_perr("Unable to allocate space for extra SPI master data.\n");
 		return SPI_GENERIC_ERROR;
@@ -301,12 +308,8 @@
 
 	spi_master_it85xx.data = data;
 
-	msg_pdbg("%s():%d superio.vendor=0x%02x\n", __func__, __LINE__,
-	         s.vendor);
-
 	if (register_shutdown(it85xx_shutdown, data)) {
-		free(data);
-		return 1;
+		goto it85spi_init_error;
 	}
 
 #ifdef LPC_IO
@@ -334,9 +337,9 @@
 	 * Major TODO here, and it will be a lot of work.
 	 */
 	base = (chipaddr)physmap("it85 communication", 0xFFFFF000, 0x1000);
-	if (base == (chipaddr)ERROR_PTR)
-		free(data);
-		return 1;
+	if (base == (chipaddr)ERROR_PTR) {
+		goto it85spi_init_error;
+	}
 
 	msg_pdbg("%s():%d base=0x%08x\n", __func__, __LINE__,
 	         (unsigned int)base);
@@ -344,37 +347,27 @@
 	data->ce_low = (unsigned char *)(base + 0xD00);  /* 0xFFFFFD00 */
 #endif
 
+	msg_pdbg("%s: internal_buses_supported=0x%x\n", __func__,
+		  internal_buses_supported);
+	/* Check for FWH because IT85 listens to FWH cycles.
+	 * FIXME: The big question is whether FWH cycles are necessary
+	 * for communication even if LPC_IO is defined.
+	 */
+	if (internal_buses_supported & BUS_FWH)
+		msg_pdbg("Registering IT85 SPI.\n");
+	/* FIXME: Really leave FWH enabled? We can't use this region
+	 * anymore since accessing it would mess up IT85 communication.
+	 * If we decide to disable FWH for this region, we should print
+	 * a debug message about it.
+	 */
+	/* Set this as SPI controller. */
+	register_spi_master(&spi_master_it85xx);
+
 	return 0;
-}
 
-int it85xx_spi_init(struct superio s)
-{
-	int ret;
-
-	if (!(internal_buses_supported & BUS_FWH)) {
-		msg_pdbg("%s():%d buses not support FWH\n", __func__, __LINE__);
-		return 1;
-	}
-	ret = it85xx_spi_common_init(s);
-	msg_pdbg("FWH: %s():%d ret=%d\n", __func__, __LINE__, ret);
-	if (!ret) {
-		msg_pdbg("%s: internal_buses_supported=0x%x\n", __func__,
-		          internal_buses_supported);
-		/* Check for FWH because IT85 listens to FWH cycles.
-		 * FIXME: The big question is whether FWH cycles are necessary
-		 * for communication even if LPC_IO is defined.
-		 */
-		if (internal_buses_supported & BUS_FWH)
-			msg_pdbg("Registering IT85 SPI.\n");
-		/* FIXME: Really leave FWH enabled? We can't use this region
-		 * anymore since accessing it would mess up IT85 communication.
-		 * If we decide to disable FWH for this region, we should print
-		 * a debug message about it.
-		 */
-		/* Set this as SPI controller. */
-		register_spi_master(&spi_master_it85xx);
-	}
-	return ret;
+it85spi_init_error:
+	free(data);
+	return 1;
 }
 
 #endif

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