View Change

18 comments:

• File Makefile:

  • Patch Set #8, Line 967: NI845X_LIBS += -L'C:\Program Files\National Instruments\NI-845x\MS Visual C'

    Please try the following:

      NI845X_LIBS += -L'${ProgramFiles}\National Instruments\NI-845x\MS Visual C'
      NI845X_LIBS += -L'${ProgramFiles(x86)}\National Instruments\NI-845x\MS Visual C'

• File flashrom.8.tmpl:

  • Patch Set #8, Line 1094:

    it seems you dropped this from the digilent_spi description

• File ni845x_spi.c:

  • Patch Set #8:

    I'm only through half the file yet, will have to continue another time... hopefully soon.

  • Patch Set #8, Line 15:

     *
     * You should have received a copy of the GNU General Public License
     * along with this program; if not, write to the Free Software
     * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
    

    Please remove this paragraph.

  • Patch Set #8, Line 20: #if CONFIG_NI845X_SPI == 1

    remove, please. Makefile already takes care of that.

  • Patch Set #8, Line 34: USB845xType

    No CamelCase please, same for all other new identifiers.

  • Patch Set #8, Line 43: static char *serial_number = NULL; // by default open the first connected device

    This is not supposed to be global, as you added it as an
    argument to ni845x_spi_open().

  • Patch Set #8, Line 50: // forward declaration

    obviously

  • Patch Set #8, Line 52:

    no space after asterisk

  • Patch Set #8, Line 55: static uint8_t usb8452_io_voltages_in_100mV[5] = {

    should be `static const`

  • Patch Set #8, Line 63: /* Copied from dediprog.c */

    If it's really a verbatim copy, please place it into its own file
    (e.g. `parameters.c`).

    If not, the original code is GPL v2 only licensed...

  • Patch Set #8, Line 117:

    no space after asterisk

  • Patch Set #8, Line 117: char *serial

    should be `const char`

  • Patch Set #8, Line 119: resource_handle

    Why is it called `handle`? isn't it rather `resource_value`?

  • Patch Set #8, Line 136: serial_number

    should use the `serial` function parameter instead

  • Patch Set #8, Line 160: }

    else (if != 0)?

  • Patch Set #8, Line 166: ARRAY_SIZE

    You can use `sizeof()` instead, as we know it's a char array.

  • Patch Set #8, Line 169: }

    This whole block is very confusing. I don't see bigger problems,
    but think we can save many indentation levels and redundant code
    if we use a control flow as follows:

      tmp = ni845xFindDevice(...);
      if (tmp != 0) {
              ...
              return -1;
      }

      for (; serial && found_devices_count; --found_devices_count) {
              sscanf(resource_handle, ...);
              if (strtol(serial, ...) == serial_as_number)
                      break;
              if (found_devices_count > 1) {
                      if (ni845xFindDeviceNext(...) != 0)
                              goto _close_ret;
              }
      }

      if (found_devices_count)
              ret = ni845x_spi_open_resource(...);

      _close_ret:

    Just a sketch, I guess we should check the return value of
    sscanf() too and (as I didn't look at the library API) I don't
    know if we should call ni845xCloseFindDeviceHandle() at all if
    ni845xFindDeviceNext() failed?

To view, visit change 25683. To unsubscribe, or for help writing mail filters, visit settings.