-
Notifications
You must be signed in to change notification settings - Fork 114
Arty S7 OpenOCD Support #111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
eaf5d4e
to
b9f15ae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately this ignores the flash
argument if Vivado is chosen as the programmer. At the very least it must assert, but ideally it should actually implement that.
@cr1901 I have minimal experience with Vivado; for the script I just looked up some commands and fiddled until the script did what it should do. You should be able to do the same unless you don't have Vivado installed. |
Also apologize for being terse; I am busy with total different things ATM and I don't expect to look at this deeper in the near future. |
I already said I don't have Vivado installed. Anyways, @whitequark, can I please assert on this for now? I got someone to help test remotely while I coded a TCL script, but I just cannot get flash programming to work with Vivado. This is the closest I came up with:
which bombs with:
I have no idea what Vivado is complaining about; this is the correct flash, according to the Digilent manual. My ability to test is limited; I want someone else to add this functionality. |
Like I said, that's the bare minimum, so you can do that. |
nmigen_boards/arty_s7.py
Outdated
@@ -164,6 +164,7 @@ def toolchain_program(self, product, name, *, programmer="vivado", flash=True): | |||
assert programmer in ("vivado", "openocd") | |||
|
|||
if programmer == "vivado": | |||
assert not flash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like this would be nicer:
assert not flash | |
assert not flash, "Flash programming is not supported with Vivado yet" |
d1f7628
to
e38ea61
Compare
@whitequark I had another day where someone could help remotely; this time, I got Flash programming working with the caveat that the user needs to manually reset/power-cycle the board. This appears to be a limitation of the Vivado TCL API, as noted in a comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, and thanks for getting Vivado flashing done; this will definitely be useful for other boards in the future.
This PR does two things:
from nmigen_boards.arty_s7 import *
from working properly.openocd
programmer for the Arty S7 board.I kept in mind your comments re: OpenOCD and programmers in general when writing this PR. So perhaps consider it a litmus test to supporting more than one programmer in nmigen :):
Indeed, all the
openocd
invocations are different, and this one's no exception. One difference is that I use the upstream board config file since it's available already (I committed it in mid 2018). Another difference is that I support both JTAG programming and flashing via a proxy.I went ahead and used this suggestion as-is.
In addition, I added a
flash
keyword argument that lets the user choose between JTAG programming and flashing a bitstream. JTAG programming is noticeably faster, shortening the already long "write, compile, program" feedback loop. Hopefully at some point Vivado's time can be shaved off too :). Therefore I think this is a reasonable keyword argument to add that will cover most use cases. I don't know ifvivado
's programmer has an equivalent.