Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/32034 )
Change subject: Add support for security register and SBULK command on Macronix chips ......................................................................
Patch Set 2: Code-Review+2
(3 comments)
Thanks for you patch, and sorry for the delay. It somehow went under my radar.
I've commented inline on some nits, if you don't mind to fix them.
https://review.coreboot.org/#/c/32034/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32034/2//COMMIT_MSG@8 PS2, Line 8: Would be nice to have one or two sentences about WPSEL and DPB here. Not everybody is an expert in macronix chips :)
https://review.coreboot.org/#/c/32034/2/spi25_statusreg.c File spi25_statusreg.c:
https://review.coreboot.org/#/c/32034/2/spi25_statusreg.c@242 PS2, Line 242: static const unsigned char cmd[JEDEC_GBULK_OUTSIZE] = { JEDEC_GBULK }; : int ret; : : ret = spi_write_enable(flash); : if (ret) { : msg_cerr("Set WREN failed!\n"); : return -1; : } : : /* Send GBULK cmd to device. */ : ret = spi_send_command(flash, sizeof(cmd), 0, cmd, NULL); There is spi_simple_write_cmd() (currently static in spi25.c) that serves this pattern. I wouldn't mind to export it for this purpose.
https://review.coreboot.org/#/c/32034/2/spi25_statusreg.c@788 PS2, Line 788: return 0; please return a negative error code and check for it below