Daniel Kurtz has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48196 )
Change subject: it85spi.c: Inline it85xx_spi_common_init() ......................................................................
Patch Set 6:
(4 comments)
https://review.coreboot.org/c/flashrom/+/48196/6/it85spi.c File it85spi.c:
https://review.coreboot.org/c/flashrom/+/48196/6/it85spi.c@299 PS6, Line 299: %s Be consistent with %s(). Probably don't need __LINE__. Better to just combine both of these into a single debug string.
https://review.coreboot.org/c/flashrom/+/48196/6/it85spi.c@309 PS6, Line 309: else else not needed.
https://review.coreboot.org/c/flashrom/+/48196/6/it85spi.c@348 PS6, Line 348: data->shm_io_base = shm_io_base; Does compiler complain that `shm_io_base` is used while undefined if `!LPC_IO`?
Perhaps move this assignment into the `#if LPC_IO` block if it is only relevant in that case.
https://review.coreboot.org/c/flashrom/+/48196/6/it85spi.c@350 PS6, Line 350: base + 0xD00 The value here depends on exact type of `chipaddr`. Is chipaddr a pointer type or an integer type?
The old 'LPC_IO' code path `((unsigned char *)base) + 0xE00;` was a bit more explicit about what exactly was happening by first casting base to `unsigned char *`. In this way, the addition is always counting bytes and not accidentally counting 32-bit words or some other pointer size.