Attention is currently required from: Stefan Reinauer, Paul Menzel, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54944 )
Change subject: Add support for TerribleFire TF530 SPI controller ......................................................................
Patch Set 1: Code-Review+1
(3 comments)
File Makefile:
https://review.coreboot.org/c/flashrom/+/54944/comment/94a81356_4e64285c PS1, Line 1211: ifeq ($(CONFIG_TF530_SPI), yes) : FEATURE_CFLAGS += -D'CONFIG_TF530_SPI=1' : PROGRAMMER_OBJS += tf530_spi.o : endif please also add to meson.
File tf530_spi.c:
https://review.coreboot.org/c/flashrom/+/54944/comment/3b4bf82b_8669b8e9 PS1, Line 62: static struct ConfigDev *cd = NULL;
I'll have a look at this again. […]
One of the key ideas of flashrom becoming reentrant is allowing for extensible testing by improving the core API in how drivers are registered as to be more akin to that of the kernel.
While it is clear the life-time of the handle is for the driver there should be not reason to not pack this into the data field of the spi master struct as is the case in the commits that Angel alludes to.
https://review.coreboot.org/c/flashrom/+/54944/comment/139b2f8a_1279e6f8 PS1, Line 71: uint8_t busy = 0; : : while (busy == 0) { : busy = port->ctrl & TF53x_CTRL_BUSY; : }
nit: I'd remove the `busy` variable, the name is confusing (it's active-low): […]
as it is a common pattern, seems like a good idea to have it in a small function with a timeout. See https://github.com/flashrom/flashrom/blob/master/realtek_mst_i2c_spi.c#L108 for the general concept.