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 19:
(4 comments)
File spi25_statusreg.c:
https://review.coreboot.org/c/flashrom/+/58475/comment/8e41c1f8_d7b6e6da PS14, Line 51: int result = spi_send_multicommand(flash, cmds);
I think you're referring to the fact that it declares a variable in the middle of the function? Ther […]
Ack
File spi25_statusreg.c:
https://review.coreboot.org/c/flashrom/+/58475/comment/00d231f3_77c17f09 PS15, Line 63: */
Changed in latest patch set.
Well, it's one thing to avoid a kludge for new use cases but something different to change behavior for existing use cases. Unless we want to test a lot of 15+ years old chips, please keep the behavior for SR1.
I'd be ok to risk to break things, but that would have to be a commit on its own, e.g. after this series, a patch that removes the kludge even for SR1.
https://review.coreboot.org/c/flashrom/+/58475/comment/c06493a7_69fdab9d PS15, Line 84: *
Done
Doesn't look done.
I'm not sure but I guess checkpatch would see this. Please just try. See [1] for Linus' reasoning and [2] for the style choices in coreboot (which avoids C++ comments but should be compatible with the style used here).
[1] https://lkml.org/lkml/2016/7/8/625 [2] https://doc.coreboot.org/contributing/coding_style.html#commenting
https://review.coreboot.org/c/flashrom/+/58475/comment/74beeeae_e612a757 PS15, Line 89: }
The next commit adds nested ifs for handling chips with different ways of writing the same register, […]
Are you saying one can't use an if inside switch/case? I've looked ahead and I don't see any real problem.