Nico Huber has posted comments on this change. ( https://review.coreboot.org/28911 )
Change subject: ft2232_spi: add an ability to use GPIO for chip selection ......................................................................
Patch Set 1:
(4 comments)
Just some smaller remarks.
https://review.coreboot.org/#/c/28911/1/flashrom.8.tmpl File flashrom.8.tmpl:
https://review.coreboot.org/#/c/28911/1/flashrom.8.tmpl@725 PS1, Line 725: channel/interface/port I think "channel/interface/port" refer all to the same parameter `port`. So the whole sentence should probably be something like:
Optional parameters specify the controller, channel/interface/ port and GPIO-based chip select to use.
https://review.coreboot.org/#/c/28911/1/ft2232_spi.c File ft2232_spi.c:
https://review.coreboot.org/#/c/28911/1/ft2232_spi.c@331 PS1, Line 331: && strlen(arg) I know it's a common pattern, but doesn't it prevent an error message to be printed when somebody specifies `-p ft2232_spi:csgpiol=` without a number?
Your call.
https://review.coreboot.org/#/c/28911/1/ft2232_spi.c@332 PS1, Line 332: = 0 Unnecessary initialization.
https://review.coreboot.org/#/c/28911/1/ft2232_spi.c@342 PS1, Line 342: cs_bits |= 1 << pin; This keeps the original bit (0x08) to assert CS. Probably a rather rare use-case, but what if somebody would want to use the new parameter to choose between different chips?