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 3: Code-Review+2
(2 comments)
AFAIR it's non-trivial to reset the proper CS bit while keeping the code clear. Also, i'm using the new parameter to choose between different chips while the CS pin is left floating.
Well, all the modifications of `cs_bits / pindir` happen earlier in the code flow, so I would have thought it's enough to clear bit3 in `pindir`. But it doesn't matter much. And I've also noticed now that the code isn't consistent (arm-usb-ocd* seems to be confused about the meaning of `cs_bits`; maybe `cs_idle` would describe it better). So maybe we should clean it up later.
Noticed another nit, but it's good to go as is.
https://review.coreboot.org/#/c/28911/3/ft2232_spi.c File ft2232_spi.c:
https://review.coreboot.org/#/c/28911/3/ft2232_spi.c@333 PS3, Line 333: unsigned Not sure about flashrom, but in other projects (e.g. coreboot) we have a rule now to spell it in full `unsigned int`. I have no strong preference, though.
https://review.coreboot.org/#/c/28911/3/ft2232_spi.c@340 PS3, Line 340: unsigned same here
-
Nico Huber (Code Review)