Attention is currently required from: Miklós Márton, Alexander Goncharov.
4 comments:
Patchset:
This programmer requires a special environment to run, and unfortunately, it only works on Windows. […]
Just a small note from me: maybe we can do the code review first (for all the patches in the chain) and then when all comments resolved, do the testing?
Also as it's typically happens, testing is done at the end of the chain (not for each individual patch).
Let's maybe keep this comment unresolved, and do the rest of code reviews.
Miklós thank you so much for help!
File ni845x_spi.c:
static int32 ni845x_spi_open_resource(char *resource_handle, uInt32 *opened_handle,
enum USB845x_type device_pid)
I am thinking, this could fit in one line: the hard limit for line length is 112 chars. Of course not all the lines need to be that long, but for the case like this (function arguments list) we can use one line, mainly because it makes it easier to grep.
static int ni845x_spi_open(const char *serial, uInt32 *return_handle,
enum USB845x_type *device_pid)
this can be one line too
Patch Set #2, Line 233: set_io_voltage_mV
Not sure you meant it: there is `set_io_voltage_mV` which is an existing second argument in the function, and there is also `io_voltage_in_mV` which is global.
Why do you replace one with the other?
To follow what the patch is doing, you would need to add one more argument to this function, `io_voltage_in_mV` and use it instead of a global var?
To view, visit change 72154. To unsubscribe, or for help writing mail filters, visit settings.