Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48196 )
Change subject: Simplifying the initialisation flow for it85spi ......................................................................
Patch Set 1:
(6 comments)
https://review.coreboot.org/c/flashrom/+/48196/1/it85spi.c File it85spi.c:
https://review.coreboot.org/c/flashrom/+/48196/1/it85spi.c@303 PS1, Line 303: 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; : } : : spi_master_it85xx.data = data; : : if (register_shutdown(it85xx_shutdown, data)) { : goto it85spi_init_error; this whole block moves towards the end of the function and therefore there is no need for goto's.
https://review.coreboot.org/c/flashrom/+/48196/1/it85spi.c@339 PS1, Line 339: chipaddr these casts are not needed
https://review.coreboot.org/c/flashrom/+/48196/1/it85spi.c@341 PS1, Line 341: goto it85spi_init_error; this should be a basic early return the code was correct before.
https://review.coreboot.org/c/flashrom/+/48196/1/it85spi.c@346 PS1, Line 346: data->ce_high = (unsigned char *)(base + 0xE00); /* 0xFFFFFE00 */ : data->ce_low = (unsigned char *)(base + 0xD00); /* 0xFFFFFD00 */ this is common code on both sides of the pre-processor directive.
https://review.coreboot.org/c/flashrom/+/48196/1/it85spi.c@349 PS1, Line 349: : msg_pdbg("%s: internal_buses_supported=0x%x\n", __func__, : internal_buses_supported); this isn't needed.
https://review.coreboot.org/c/flashrom/+/48196/1/it85spi.c@362 PS1, Line 362: */ this comment block is all related the to the above code and should be immediately close to the register_spi_master() function.