Márton Miklós has posted comments on this change. ( https://review.coreboot.org/25683 )
Change subject: Add support for National Instruments USB-845x devices ......................................................................
Patch Set 8:
(3 comments)
https://review.coreboot.org/#/c/25683/3/flashrom.c File flashrom.c:
https://review.coreboot.org/#/c/25683/3/flashrom.c@417 PS3, Line 417: { : .name = "digilent_spi", : .type = USB, : .devs.dev = devs_digilent_spi, : .init = digilent_spi_init, : .map_flash_region = fallback_map, : .unmap_flash_region = fallback_unmap, : .delay = internal_delay, : },
Please use tabs for indentation
Done
https://review.coreboot.org/#/c/25683/8/ni845x_spi.c File ni845x_spi.c:
PS8:
I'm only through half the file yet, will have to continue another time... hopefully soon.
Thanks for the review I will fix the comments soon!
https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@63 PS8, Line 63: /* Copied from dediprog.c */
If it's really a verbatim copy, please place it into its own file […]
Actually it is copied from the pickit2 code: https://github.com/flashrom/flashrom/blob/master/pickit2_spi.c#L302
I am under the impression too that these spi clock speed calculation functions shall be uniformized & placed to a common place. What do you recommend? Should I create an another patchset which modifies dediprog, pickit and rebase this onto the top of that?
-
Márton Miklós (Code Review)