Attention is currently required from: Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58475 )
Change subject: spi25_statusreg: make register read/write functions generic ......................................................................
Patch Set 15:
(5 comments)
File flash.h:
https://review.coreboot.org/c/flashrom/+/58475/comment/7a0ce1e9_eb7236e1 PS15, Line 170: INVALID_REG = 0, : STATUS1, : STATUS2, : CONFIG1, Please add entries in the commits that make use of them. Took me rather long to realize that I can't find a commit for CONFIG1 probably because it's not used?
File flash.h:
https://review.coreboot.org/c/flashrom/+/58475/comment/5e844940_7a58ff2f PS14, Line 169: enum flash_reg { : INVALID_REG = 0, : STATUS1, : STATUS2, : CONFIG1, : }; : #define MAX_REGISTERS 4
Also, this is specific to SPI flash chips. I'm not sure if it should be here.
`MAX_REGISTERS` is only used in `writeprotect.c` AFAICS, so it would belong there. Or maybe as a last enum element? It would automatically have the correct value if starting at 0.
IMO, the enum belongs to the API functions.
File spi25_statusreg.c:
https://review.coreboot.org/c/flashrom/+/58475/comment/07f7a6d7_10339085 PS14, Line 51: int result = spi_send_multicommand(flash, cmds);
Would be good to check with manibuilder if this causes problems with unusual toolchains (e.g. […]
What do you mean? What could cause problems?
File spi25_statusreg.c:
https://review.coreboot.org/c/flashrom/+/58475/comment/a1757286_129a092c PS15, Line 63: */ I have doubts that this is true for any modern chip. "allow running RDSR only once"? when? only once after an erase/write command? how should one keep track of it?
So I would pretty much prefer to start testing writes to registers != SR1 without this delay. Otherwise we'd have to keep assuming forever that it's necessary.
https://review.coreboot.org/c/flashrom/+/58475/comment/262245b3_98401a39 PS15, Line 84: * Please drop this dangling asterisk or add a line break after the first / before the last.