WIP: bmp: Implement. Requires firmware from https://github.com/UweBonnes/b…#111
WIP: bmp: Implement. Requires firmware from https://github.com/UweBonnes/b…#111UweBonnes wants to merge 15 commits into
Conversation
|
Thanks for this PR. Will study this one and add feedback |
|
Sorry, fixes isnot enough after yesterdays changes. Stand By. |
|
Use https://github.com/UweBonnes/blackmagic/tree/fixes1 or better avr. |
UweBonnes
force-pushed
the
bmp_remote
branch
from
d1e3e3c to
f2a7354
Compare
UweBonnes
force-pushed
the
bmp_remote
branch
from
f2a7354 to
814541c
Compare
UweBonnes
force-pushed
the
bmp_remote
branch
from
814541c to
bb008c3
Compare
|
BMP has code to work on windows. Somebody with windows needs to port and test. |
|
Maybe in a first time it make sense to add this driver only for linux devices, and add a note to request for someone with a windows machine to check/add/fix support. |
trabucayre
left a comment
trabucayre
left a comment
There was a problem hiding this comment.
src/cable.hpp: it's okay. But maybe it's requires it's own PR since it's not related to bmp.
src/part.hpp: I'm not really convinced, some devices are already present in this file, others are not (currently) supported. It's error prone, since openFPGALoader don't really know if there are supported or not (or consider is it).
| printError("FAIL"); | ||
| return false; |
trabucayre
left a comment
trabucayre
left a comment
There was a problem hiding this comment.
Some white space are presents in bmp.cpp
| void Bmp::DEBUG_WARN(const char *format, ...) | ||
| { | ||
| va_list ap; | ||
| va_start(ap, format); | ||
| vfprintf(stderr, format, ap); | ||
| va_end(ap); | ||
| } | ||
|
|
||
| void Bmp::DEBUG_WIRE(const char *format, ...) | ||
| { | ||
| if (!_verbose) | ||
| return; | ||
| va_list ap; | ||
| va_start(ap, format); | ||
| vfprintf(stderr, format, ap); | ||
| va_end(ap); | ||
| } | ||
|
|
||
| void Bmp::DEBUG_PROBE(const char *format, ...) | ||
| { | ||
| if (!_verbose) | ||
| return; | ||
| va_list ap; | ||
| va_start(ap, format); | ||
| vfprintf(stderr, format, ap); | ||
| va_end(ap); | ||
| } |
There was a problem hiding this comment.
There is printWarn, printInfo and printError (with color) in display.hpp
|
|
||
| int Bmp::setClkFreq(uint32_t clkHZ) | ||
| { | ||
| char construct[REMOTE_MAX_MSG_SIZE]; |
There was a problem hiding this comment.
char construct[REMOTE_MAX_MSG_SIZE], is used in most of methods. Maybe a privtate variable in bmp.hpp to reduce that.
| s = platform_buffer_read(construct, REMOTE_MAX_MSG_SIZE); | ||
|
|
||
| if ((!s) || (construct[0] == REMOTE_RESP_ERR)) { | ||
| fprintf(stderr,"Update Firmware to allow to set max SWJ frequency\n"); |
There was a problem hiding this comment.
It's an error. It seems logical to stop no ?
UweBonnes
force-pushed
the
bmp_remote
branch
2 times, most recently
from
c0c5d4a to
6be1e00
Compare
| option(BUILD_STATIC "Whether or not to build with static libraries" OFF) | ||
| option(ENABLE_UDEV "use udev to search JTAG adapter from /dev/xx" ON) | ||
| if (${CMAKE_SYSTEM_NAME} MATCHES "Windows") | ||
| option(ENABLE_UDEV "use udev to search JTAG adapter from /dev/xx" OFF) |
There was a problem hiding this comment.
This default configuration makes it impossible to use this on Windows because searching for the device isn't implemented. Might want to change the --device parameter to not depend on USE_UDEV
There was a problem hiding this comment.
Thanks for this remark
It make sense: original idea with --device was to use it for FTDI devices on Linux (/dev/ttyUSBx) but if this option may be used for another things it make sense to split that between UDEV support (ftdi devices) and search for a device base on a path (like here).
There was a problem hiding this comment.
I have updated repository: now --device has specific USE_DEVICE_ARG. This allows to enable/disable this arg at CMakeLists.txt level according to options (udev, libgpiod, ...).
Other cable type may provide a unique serial number to
Checks for version 3, maybe version 2 will also work.
Fix dangling escapes
| src/xilinxMapParser.hpp | ||
| src/colognechip.hpp | ||
| src/colognechipCfgParser.hpp | ||
| src/bmd.hpp |
|
LGTM. |
3 participants
…lackmagic/tree/fixes
Hello,
if you find time, please have a look at this PR. It works only for Linux, maybe for Mac, but windows device detection is not implemented. Code from BMP needs porting and testing. C++ Coding style and general coding style need rework. Speed is probably not good, as shiftIR|DR need up to 5 usb round trips. Please comment.