Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/48196 )
Change subject: Simplifying the initialisation flow for it85spi ......................................................................
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