Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52774 )
Change subject: pickit2_spi.c: Refactor singleton states into reentrant pattern ......................................................................
Patch Set 1:
(3 comments)
File pickit2_spi.c:
https://review.coreboot.org/c/flashrom/+/52774/comment/39a5cbe4_1e92716f PS1, Line 57: }; Do we need this struct? I guess it would make it easier to extend in the future, but this seems hard to predict. We could also pass the libusb handle directly as `data`.
https://review.coreboot.org/c/flashrom/+/52774/comment/e1d4b973_7f01382a PS1, Line 343: static struct spi_master spi_master_pickit2 = { This will bug me with every single patch. I think the API is just broken and maybe we should fix it before making more use of it. The per-instance `data` pointer should simply be passed to register_spi_master(), IMHO. This might actually be easier to address right now as there are not that many programmer drivers that use the `data` pointer yet.
https://review.coreboot.org/c/flashrom/+/52774/comment/0b21cbcc_982ab4fb PS1, Line 472: pickit2_data = calloc(1, sizeof(*pickit2_data)); No NULL check? (Could be avoided by not using a struct.)