-
Notifications
You must be signed in to change notification settings - Fork 20
adapted lattice_bscan_spi.py for LFE5UM5G-85F #7
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
base: master
Are you sure you want to change the base?
Conversation
@HarryHo90sHK could you please test that it still works for LFE5UM-45F ? |
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.
@ilya-epifanov Thanks for the PR! I've left some comments but I haven't got a device to test your code yet. Please feel free to share any ideas here.
@@ -149,6 +149,7 @@ def elaborate(self, platform): | |||
# JTAG adapter: sample TDO | |||
with m.FSM() as fsm: | |||
with m.State("IDLE"): | |||
m.d.sync += self.cs_n.o.eq(1) |
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.
Since the Output Enable for the tri-stated CS_N pin is already set to 1 whenever the JTAG chain 1 has been selected (lines 97 and 130), and the Output value of this pin is automatically assigned to 0 whenever the SPI transfer is ongoing (line 167), I don't think these changes to the CS_N pin (this line and your line 163) are necessary.
@@ -159,12 +160,15 @@ def elaborate(self, platform): | |||
with m.If(head == 0): | |||
m.next = "XFER" | |||
with m.State("XFER"): | |||
m.d.sync += self.cs_n.o.eq(0) |
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.
See my comment on line 152.
m.next = "IDLE" | ||
m.next = "XFER_FIN" | ||
with m.State("XFER_FIN"): | ||
m.d.sync += self.jtag.sel.eq(0) |
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.
I believe driving jtag.sel
by the JTAG TAP controller itself is more appropriate than our custom SPI transfer cycle FSM.
@@ -125,7 +126,6 @@ def elaborate(self, platform): | |||
] | |||
|
|||
m.d.comb += [ | |||
cd_sync.rst.eq(self.jtag_sel1_capture), |
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.
This line was intended to adopt the same clock domain reset logic as the Xilinx counterpart (line 87). Has there been any negative effect brought by this line?
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.
Update: another user has recently adapted the proxy bitstream for their MachXO2 FPGA, and they also reported that the removal of this sync clock reset is necessary to deassert CSn. @ilya-epifanov You don't need to add this line back 😃
@@ -89,12 +90,12 @@ def _detect_jtag_state(self, module): | |||
# it is implied chain 1 is selected until TAP enters TLR state again | |||
with module.FSM(): | |||
with module.State("IDLE"): | |||
with module.If(~jtag_rst_n): | |||
with module.If(jtag_rti1): |
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.
This seems to assume that the JTAG TAP will never be reset as long as the FPGA doesn't get re-initialised. Is this safe?
There is apparently an (undocumented) IR command to connect the SPI bus to DR (according to @adamgreig). That should make this PR and https://github.com/quartiq/bscan_spi_bitstreams/blob/master/lattice_bscan_spi.py obsolete. |
@jordens Thanks for the comment. Is the said IR command different from the one I stated on README, i.e. 0x32? |
@HarryMakes is there still an need/interest for this, given that ecpprog etc use the ecp5 features directly? |
@jordens Yes, openocd supports more programmers. I'm trying to figure out if an svf that will just use this custom command will work. |
It turned out that adding support in directly openocd was easier. |
Had to spend some time debugging the thing, moved some logic to sync domain and added a 1 clock cycle wait at the end of the transaction.
Also, cleaned up handling of multiple devices and parallel build.