search

dirtyjtag / DirtyJTAG

JTAG probe firmware
MIT License
464 stars 70 forks source link

changes for openocd. Allow queuing of CMD_XFER #102

Closed phdussud closed 1 year ago

phdussud commented 1 year ago

Hello! My openocd changes got refused because we weren't batching the small XFERS into an optimal number of USB transactions. I completely rewrote the code based on their bitq framework that does the a whole bunch of work, leaving the probe code to manage IO (batching on output and un-batching on input). The requires some changes in our code. Here are the changes.
Thanks!

jeanthom commented 1 year ago

Hi @phdussud,

Thanks for all the work you've done on fixing OpenOCD and its upstreaming. I'm a little concerned about OpenOCD reviewers reaction, because I'd really like to have at least basic DJTAG1 compatibility.

I don't mind modifying DJTAG2 though.

phdussud commented 1 year ago

Hi @jeanthom, Unfortunately I only coded support for V2 in the upstream PR. The main reason is that they rejected the non batching version and V1 does not support batching multiples XFER into one USB request. My repo on sourceforge is still available for V1/V2 probes. I could extend the upstream version to support V1 as well but the code will be definitely more complicated. Let me ask a question. What is the reason why can't everybody with an existing V1 probe upgrade to V2 firmware. I am not aware of any incompatibility issues and the same HW is supported. The pinout has changed for the bluepill but not for the stlink style HW. Some will not support SPI pin toggling as a consequence but I don't think it is a major problem. I would suggest that the best path for the future is to merge V2 into master and move on. Of course I understand you may have some concerns that I don't know about. It would help me to know them. Thanks! Patrick

zoobab commented 1 year ago

Fully agree with you. Users will have to reflash with a firmware update, that's it.

jeanthom commented 1 year ago

Hi @phdussud & @zoobab,

I initially thought that keeping the V1 support would be nice for implementing DirtyJTAG on new targets, but judging the amount of code it would take to have support for both versions in OpenOCD it would be hardly practical for the devs. So, yeah let's do OpenOCD for DJTAG2 only, it will be fine.

I would like to have an error message for DJTAG1 users, telling them that they have to upgrade their probe so that the user experience isn't confusing.

Regarding the merge on DirtyJTAG v2 onto master, I have a couple of small things I want to review (I'd like to update the USB descriptors to reflect that the project has grown up, it's not a one-man band anymore - they are in fact more active on this project than I am, I don't deserve all the credit :smile:). I will try to finalize the merge next week.

phdussud commented 1 year ago

Hi @jeanthom and @zoobab The upstream PR for openocd has an error message saying "Probe must run V2". Please suggest another message and I will update my PR accordingly. Your planned merge into master looks good. Cheers, Patrick.

jeanthom commented 1 year ago

Hi,

Maybe something in the likes of "The probe appears to be running an old version of DirtyJTAG. Please upgrade to DirtyJTAG 2.0 or newer."? What do you think?

phdussud commented 1 year ago

Maybe "The probe appears to be running version 1 of DirtyJTAG. Please upgrade to DirtyJTAG 2.0 or newer." ? It is more specific than old.

jeanthom commented 1 year ago

Yes, that's better.

phdussud commented 1 year ago

I submitted the change to review. Thanks!