Nico Huber has posted comments on this change. ( https://review.coreboot.org/28087 )
Change subject: Add initial J-Link SPI programmer ......................................................................
Patch Set 2:
(4 comments)
https://review.coreboot.org/#/c/28087/1/jlink_spi.c File jlink_spi.c:
https://review.coreboot.org/#/c/28087/1/jlink_spi.c@130 PS1, Line 130:
Do not free it here or set `buffer = NULL;`. Otherwise we'd […]
Ping?
Also, `buffer_size` should be set to zero too if we keep the free(). Btw., it would all get less complex if we'd just malloc()/free() on every call, w/o the global buffer.
https://review.coreboot.org/#/c/28087/2/jlink_spi.c File jlink_spi.c:
https://review.coreboot.org/#/c/28087/2/jlink_spi.c@191 PS2, Line 191: /* TODO: Use kHz for speed and update manual! */ already obsolete?
https://review.coreboot.org/#/c/28087/2/jlink_spi.c@195 PS2, Line 195: && strlen(arg) > 0 Why check the strlen? if it's 0, the user literally typed `:spispeed=` without a value. Why allow that?
https://review.coreboot.org/#/c/28087/2/jlink_spi.c@344 PS2, Line 344: dev Isn't this `dev` (as part of `devs`) free'd yet?
-
Nico Huber (Code Review)