Attention is currently required from: Richard Hughes, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Patrick Rudolph. Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 )
Change subject: libflashrom: Return progress state to the library user ......................................................................
Patch Set 11:
(16 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/49643/comment/10ec2b73_825cd270 PS11, Line 15:
It would be realy good to test it on at least one real programmer, not dummy. […]
I am not completely familiar with the different programmers, but I think the programmer here is spi25 ???
File cli_output.c:
https://review.coreboot.org/c/flashrom/+/49643/comment/51c4d99a_4b1b2e52 PS11, Line 87:
Space between function name and arguments list not needed.
Done
File flash.h:
https://review.coreboot.org/c/flashrom/+/49643/comment/9ac059f2_ade1976b PS11, Line 481: set_progress
Looking into the usage of this function, maybe `update_progress` is better name? It is called repeat […]
Done
https://review.coreboot.org/c/flashrom/+/49643/comment/852a23df_50b31cc4 PS11, Line 481: void set_progress(struct flashctx *flash, enum flashrom_progress_stage stage, size_t current, size_t total);
Lets move it into the /* cli_output.c */ section, together with other output stuff. […]
This is to support the API exposed via libflashrom and independent from the CLI.
File libflashrom.h:
https://review.coreboot.org/c/flashrom/+/49643/comment/6cb7a25b_0930f07b PS11, Line 106: flashrom_set_progress_callback
If set_progress becomes update_progress, this one becomes flashrom_update_progress_callback
Done
File libflashrom.c:
https://review.coreboot.org/c/flashrom/+/49643/comment/574c5e32_13fe10da PS11, Line 102: chaned
chaned -> changed (typo)
Done
https://review.coreboot.org/c/flashrom/+/49643/comment/01c8e569_c5ccee22 PS11, Line 106: userdata
This is called progress_userdata not userdata?
Done
File spi.c:
https://review.coreboot.org/c/flashrom/+/49643/comment/63a1f172_cec5592c PS11, Line 104: size_t progress_start = start; : size_t progress_total = len - start;
I think the variable naming can be improved, I spent a while trying to read this small piece. […]
Done
File spi25.c:
https://review.coreboot.org/c/flashrom/+/49643/comment/a29094da_cb4f2f8e PS11, Line 677: size_t progress_start = start; : size_t progress_total = len - start;
This looks the same as previous, let do […]
Done
https://review.coreboot.org/c/flashrom/+/49643/comment/8f8f794e_91ac2cb5 PS11, Line 704: size_t progress_start = start; : size_t progress_total = len - start;
And one more here :)
Done
File tests/spi25.c:
https://review.coreboot.org/c/flashrom/+/49643/comment/959fd74b_5467d951 PS11, Line 93: assert_true(0); : break;
This probably can be `fail()` , if it's the intention
Done
https://review.coreboot.org/c/flashrom/+/49643/comment/02d09a9d_d33e5c3a PS11, Line 106:
new line
Done
https://review.coreboot.org/c/flashrom/+/49643/comment/3f8e6cce_d86df9a3 PS11, Line 107:
new line
Done
https://review.coreboot.org/c/flashrom/+/49643/comment/86628ce5_824f7254 PS11, Line 110:
new line
Done
https://review.coreboot.org/c/flashrom/+/49643/comment/419bda1e_c048218a PS11, Line 110:
new line
Done
https://review.coreboot.org/c/flashrom/+/49643/comment/bc30e2b3_e66499b8 PS11, Line 110:
new line
Done
-
Daniel Campello (Code Review)