Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk, Steve Markgraf.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71801 )
Change subject: programmer: Add bitbanging programmer driver for Linux libgpiod ......................................................................
Patch Set 1: Code-Review+1
(4 comments)
File include/programmer.h:
https://review.coreboot.org/c/flashrom/+/71801/comment/219830f7_ff4d44ee PS1, Line 75: extern const struct programmer_entry programmer_linux_gpiod; Looks like the list is sorted alphabetically
File linux_gpiod.c:
https://review.coreboot.org/c/flashrom/+/71801/comment/664ba19c_b6d93497 PS1, Line 97: atoi Other places use `strtol()` because it's safer
https://review.coreboot.org/c/flashrom/+/71801/comment/6d807e3e_908cdb4f PS1, Line 142: if (register_spi_bitbang_master(&bitbang_spi_master_gpiod, data)) : return 1; /* shutdown function does the cleanup */ : : return 0; How about:
``` /* shutdown function does the cleanup */ return register_spi_bitbang_master(&bitbang_spi_master_gpiod, data); ```
https://review.coreboot.org/c/flashrom/+/71801/comment/34428947_b64a0637 PS1, Line 148: if (data) { : if (gpiod_line_bulk_num_lines(&data->bulk) > 0) : gpiod_line_release_bulk(&data->bulk); : : free(data); : } : : if (chip) : gpiod_chip_close(chip);
I have few questions about the error path exit. […]
At some point we decided to register the shutdown function early so that it does the cleanup in case init fails. If we still do this, looks like we could benefit here too.