Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47655 )
Change subject: it85spi.c: Refactor singleton states into reentrant pattern ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/flashrom/+/47655/6/it85spi.c File it85spi.c:
https://review.coreboot.org/c/flashrom/+/47655/6/it85spi.c@242 PS6, Line 242: if (!flash || !flash->mst || !flash->mst->spi.data) { : msg_perr("Unable to extract data from flash context.\n"); : return SPI_GENERIC_ERROR; : } I don't know why gerrit lost this comment twice in a row.. You can delete this, none of these are NULL able upon entry into this callback.
https://review.coreboot.org/c/flashrom/+/47655/6/it85spi.c@308 PS6, Line 308: data->shm_io_base = 0; : data->ce_high = NULL; : data->ce_low = NULL; : data->it85xx_scratch_rom_reenter = 0; :
Its a bit of a style thing, but no need to explicitly set these to 0 since: […]
agree, option (a) is ideal imho.
https://review.coreboot.org/c/flashrom/+/47655/6/it85spi.c@330 PS6, Line 330: /* These pointers are not used directly. They will be send to EC's : * register for indirect access. */ : base = 0xFFFFF000; : data->ce_high = ((unsigned char *)base) + 0xE00; /* 0xFFFFFE00 */ : data->ce_low = ((unsigned char *)base) + 0xD00; /* 0xFFFFFD00 */ :
This part is very similar between the two LPC_* cases and is a good candidate for a small refactor i […]
We already discussed this Daniel however it is orthogonal to this commit so not addressed here.