Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46140 )
Change subject: WIP: s25f.c: import file from cros flashrom ......................................................................
Patch Set 2:
(7 comments)
https://review.coreboot.org/c/flashrom/+/46140/1/Makefile File Makefile:
https://review.coreboot.org/c/flashrom/+/46140/1/Makefile@641 PS1, Line 641: s25f.o
Make adding the file and putting it into the makefile its own patch. […]
I think this is done now that chip restore has been extracted to a separate patch - or do you want me to separate out the addition of s25f.c from the header changes in this patch?
https://review.coreboot.org/c/flashrom/+/46140/1/chipdrivers.h File chipdrivers.h:
https://review.coreboot.org/c/flashrom/+/46140/1/chipdrivers.h@24 PS1, Line 24: #include "writeprotect.h" /* for modifier_bits */
unrelated.
It provides the `struct modifier_bits` type used in the s25f prototypes.
https://review.coreboot.org/c/flashrom/+/46140/1/flash.h File flash.h:
https://review.coreboot.org/c/flashrom/+/46140/1/flash.h@52 PS1, Line 52: struct flashrom_flashctx; : #define flashctx flashrom_flashctx /* TODO: Agree on a name and convert all occurences. */
can you make this move its own patch, nice and simple one.
Done - I think it's actually better to leave the typedef where it was since it was close to the struct definition. I have moved the function prototype that used it instead (in the chip restore patch).
https://review.coreboot.org/c/flashrom/+/46140/1/flash.h@60 PS1, Line 60: #define CHIP_RESTORE_CALLBACK int (*func) (struct flashctx *flash, uint8_t status) : : int register_chip_restore(CHIP_RESTORE_CALLBACK, struct flashctx *flash, uint8_t status);
add this chip_restore support into its own patch.
Done
https://review.coreboot.org/c/flashrom/+/46140/1/flashrom.c File flashrom.c:
https://review.coreboot.org/c/flashrom/+/46140/1/flashrom.c@539 PS1, Line 539: #define CHIP_RESTORE_MAXFN 4 : static int chip_restore_fn_count = 0; : static struct chip_restore_func_data { : CHIP_RESTORE_CALLBACK; : struct flashctx *flash; : uint8_t status; : } chip_restore_fn[CHIP_RESTORE_MAXFN];
chip restore patch
Done
https://review.coreboot.org/c/flashrom/+/46140/1/flashrom.c@598 PS1, Line 598: //int register_chip_restore(int (*function) (void *data), void *data) : int register_chip_restore(CHIP_RESTORE_CALLBACK, : struct flashctx *flash, uint8_t status) : { : if (chip_restore_fn_count >= CHIP_RESTORE_MAXFN) { : msg_perr("Tried to register more than %i chip restore" : " functions.\n", CHIP_RESTORE_MAXFN); : return 1; : } : chip_restore_fn[chip_restore_fn_count].func = func; /* from macro */ : chip_restore_fn[chip_restore_fn_count].flash = flash; : chip_restore_fn[chip_restore_fn_count].status = status; : chip_restore_fn_count++; : : return 0; : }
chip restore patch
Done
https://review.coreboot.org/c/flashrom/+/46140/1/flashrom.c@2319 PS1, Line 2319: {
probably need to call chip restore here?
Done