Miklós Márton has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/25683 )
Change subject: Add support for National Instruments USB-845x devices ......................................................................
Patch Set 26:
(4 comments)
https://review.coreboot.org/c/flashrom/+/25683/25/ni845x_spi.c File ni845x_spi.c:
https://review.coreboot.org/c/flashrom/+/25683/25/ni845x_spi.c@253 PS25, Line 253: if (io_voltage_100mV < usb8452_io_voltages_in_100mV[0]) {
Nit, if called with `use_smaller = false` we might not want to take this path? […]
Ack
https://review.coreboot.org/c/flashrom/+/25683/25/ni845x_spi.c@443 PS25, Line 443: if (device_pid == USB8451) {
Nit, taking this path will result in no warning printed, even if […]
Ack
https://review.coreboot.org/c/flashrom/+/25683/25/ni845x_spi.c@504 PS25, Line 504: return -1;
This path is a little troubling. What if the user has a voltage translator […]
Ack
https://review.coreboot.org/c/flashrom/+/25683/25/ni845x_spi.c@542 PS25, Line 542: }
Might look nicer with the whole `if` block in a separate fucntion.
Ack
-
Miklós Márton (Code Review)