Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/48196 )
Change subject: it85spi.c: Inline it85xx_spi_common_init() ......................................................................
Patch Set 7:
(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__. […]
Combined into a single string. I would keep __LINE__ maybe it was useful for someone who added it?
https://review.coreboot.org/c/flashrom/+/48196/6/it85spi.c@309 PS6, Line 309: else
else not needed.
Done
https://review.coreboot.org/c/flashrom/+/48196/6/it85spi.c@348 PS6, Line 348: data->shm_io_base = shm_io_base;
Probably reasonable to just assign it to zero at its declaration above Anastasia, `unsigned int shm_ […]
Compiler did not complain, but I wouldn't mind it complaining in this case. shm_io_base is now a member of the struct, and all members exist regardless of LPC_IO, so I want to assign it anyway even if !LPC_IO (so doing what Ed suggested).
https://review.coreboot.org/c/flashrom/+/48196/6/it85spi.c@350 PS6, Line 350: base + 0xD00
+1 to Daniel's comment here, my eye didn't even notice that :O
chipaddr is both pointer and integer... uintptr_t This was written differently between LPC_IO and LPC_MEMORY code, but worked the same because of uintptr_t. Agree to change to `((unsigned char *)base) + 0xE00`