Initial support for Diligent CModA7 (15T + 35T), Expanded Zybo Definition, Fix Vivado building on windows, Fixed VHDL conversion via rhea.build.toolflow.convert, added Clock.ticks property #40
Conversation
… ticks to simulation ticks
NickShaffner
changed the title
NickShaffner
changed the title
…f ticks of user clock signals
NickShaffner
changed the title
|
@cfelton I'm a bit confused as to why Clock.gen() can override hticks via a passed in parameter - ignoring the frequency and period. I see several of the tests seem to make use of this, but I don't understand why? It seems to me that the 'official' and only way to set hticks should be through the Clock() constructor. either via the frequency parameter, or perhaps an alternate optional hticks parameter that computes frequency. Would you be amiable to me doing such a refactoring? |
In myhdl the simulation step is not tied to an absolute timescale (well there is in some cases, like trace signals). To determine what the tick/htick should be either need all the possibly clocks (to set them relative to each other), know that absolute time you want sim step to represent, or let the user set it. In the code the absolute (1) and user (3) are currently supported but the absolute is currently tied to 1ns the user (modifying hticks) lets the user override this - cases where frequency greater than 500 MHz is desired. When the user sets the htick, it doesn't change the frequency of the clock but rather the absolute time unit that a simulation step will represent. |
| """ | ||
| return self._ticks | ||
|
|
||
| def delay(self, count=1): |
There was a problem hiding this comment.
We discussed this briefly in the issue, I don't like the name delay for this generator. In myhdl signals currently have a posedge and negedge events, what might read well is to use the plural of these:
yield clock.posedges(10)
yield clock.negedges(5)or the "edge" could be an optional argument
yield clock.edges(10) # default is posedge, edge=True
yield clock.edges(5, False) # edge=FalseThere was a problem hiding this comment.
@cfelton Thanks for the feedback.
I was originally thinking of (and would have preferred to) implement delay() as yield delay(2 * hticks). This may have better matched the myhdl delay(), but I was concerned about the fact that 2 * hticks can be off by one or two relative to simulation ticks due to their rounding. This could be corrected via separate _hticks_high and _hticks_low values but that would of course slightly impact the duty cycle in such situations.
Anyways, after your good suggestions it occurs to me that 'posedges' and 'negedges' or 'edges' really belong in Signal() and not Clock() - and may crowd Signals() currently (intentional) minimalist interface. I'll remove delay(), but leave ticks in.
NickShaffner
changed the title
…hs with spaces in them will also work
NickShaffner
changed the title
|
Sorry this pull request keeps going :) I suppose I need to figure out how to create separate pull requests in github. |
…to specify full binary 'vivado.bat' for windows to find it)
| tcl += ["# create: {}".format(date_time)] | ||
| tcl += ["# by: {}".format(os.path.basename(sys.argv[0]))] | ||
| tcl += ["#\n#\n"] | ||
| tcl += ['#\n#\n# Vivado implementation script'] |
There was a problem hiding this comment.
What change the string string character from " to '?
| tcl += ['route_design'] | ||
| tcl += ['report_timing_summary -file "{}"'.format( | ||
| self.escape_path(os.path.join(self.path, self.name+'_timing.rpt')))] | ||
| tcl += ['write_bitstream -force "{}"'.format( |
There was a problem hiding this comment.
Instead of changing all the strings in this file, just use the escapes in this case \". Otherwise this module will be inconsistent with the rest of the python modules.
|
Because the build tools are not tested by travis-ci (FPGA tools not available) I will have to pull from your branch to my local computer and tests the FPGA board builds. Some of the Vivado build changes I don't understand, I want to make sure they work with multiple versions etc. This might take me a couple days because I have MyHDL GSoC items I need to finish in this week. |
|
@cfelton No problem or rush, and sorry that things are confusing. I've just been fixing things as I go along to get my basic flow working. The changes were related to:
These changes would likely also benefit the other toolflows as well, but I only have vivado installed, so can only test it there. I'll make the changes you requested and submit another changelist later today. |
NickShaffner
changed the title
|
Apologies for cramming so much stuff into this one pull request. In future, I'll separate changes into branches so they are more topical. |
NickShaffner
changed the title
No description provided.