Attention is currently required from: Nico Huber, Nikolai Artemiev, Sergii Dmytruk. Angel Pons 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 14:
(8 comments)
File flash.h:
https://review.coreboot.org/c/flashrom/+/58475/comment/8eb6e671_1eebe37f PS14, Line 169: enum flash_reg { : INVALID_REG = 0, : STATUS1, : STATUS2, : CONFIG1, : }; : #define MAX_REGISTERS 4 Does this need to be public, or can it remain private inside `spi25_statusreg.c`?
File spi25_statusreg.c:
https://review.coreboot.org/c/flashrom/+/58475/comment/d2825c1b_b1a5c042 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. DJGPP).
https://review.coreboot.org/c/flashrom/+/58475/comment/62c990d2_3cb2aef2 PS14, Line 126: should be a tab
https://review.coreboot.org/c/flashrom/+/58475/comment/79cf7549_f8bfb225 PS14, Line 134: nit: double blank line
https://review.coreboot.org/c/flashrom/+/58475/comment/b196aedc_5366d628 PS14, Line 138: /* FIXME: We should propagate the error. */ Ah, this is why you want to drop `spi_read_status_register()`
https://review.coreboot.org/c/flashrom/+/58475/comment/0fcb07c5_f39ff12c PS14, Line 149: nit: double blank line
https://review.coreboot.org/c/flashrom/+/58475/comment/87fb8ad1_df5ffccd PS14, Line 194: spi_write_register failed. Would be good to indicate which register failed:
Could not write status register 1.
Or similar.
https://review.coreboot.org/c/flashrom/+/58475/comment/3868e53e_88d303bc PS14, Line 207: spi_write_register failed. Ditto