Attention is currently required from: Alexander Goncharov.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72160 )
Change subject: ni845x_spi: refactor singleton states into reentrant pattern ......................................................................
Patch Set 2:
(5 comments)
File ni845x_spi.c:
https://review.coreboot.org/c/flashrom/+/72160/comment/b93cd2df_5955a65b PS2, Line 228: uInt32 device_handle I thought the purpose of the first patch in the chain CB:72154 is to do this type of work: adding arguments to functions and passing values as arguments instead of using globals? Is there a reason why this specific diff, adding `uInt32 device_handle` as an argument to `usb8452_spi_set_io_voltage` cannot be done in CB:72154 ?
https://review.coreboot.org/c/flashrom/+/72160/comment/d5aa83ad_978fcd32 PS2, Line 395: data These are actually arguments, not global data
https://review.coreboot.org/c/flashrom/+/72160/comment/cdc05f96_16ca4d87 PS2, Line 575: uInt32 device_handle `device_handle` declared as `Int32` in the `ni845x_spi_data` struct
https://review.coreboot.org/c/flashrom/+/72160/comment/52da9761_537b4bbd PS2, Line 640: You won't need previous patch CB:72159 if you do: 1) allocate data here, and use data->member_name from here and below 2) add `free(data)` in shutdown function 3) call shutdown function with `data` in `err:` label branch Also it would be consistent with other programmers, all of them which have data struct they free data in shutdown function. what do you think?
https://review.coreboot.org/c/flashrom/+/72160/comment/893aa9e6_c124bbb0 PS2, Line 663: NULL and now you need to pass `data` here, now you have data!