-
Notifications
You must be signed in to change notification settings - Fork 58
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
vendor.xilinx_s7: implement. #89
Conversation
Codecov Report
@@ Coverage Diff @@
## master #89 +/- ##
======================================
Coverage 80.6% 80.6%
======================================
Files 32 32
Lines 4950 4950
Branches 1068 1068
======================================
Hits 3990 3990
Misses 824 824
Partials 136 136 Continue to review full report at Codecov.
|
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.
Very nice work!
nmigen/vendor/xilinx_s7.py
Outdated
create_project -force -name {{name}} -part {{platform.device}} | ||
{% for file in platform.extra_files %} | ||
{% if file.endswith(".v") -%} | ||
add_files {{file}} |
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.
Should this also handle .sv
files?
nmigen/vendor/xilinx_s7.py
Outdated
__all__ = ["XilinxS7Platform"] | ||
|
||
|
||
class XilinxS7Platform(TemplatedPlatform): |
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.
Nit: can we call this Xilinx7SeriesPlatform
, since that's what used in all official docs? The file would be renamed as well.
nmigen/vendor/xilinx_s7.py
Outdated
* ``{{name}}.bit``: binary bitstream. | ||
""" | ||
|
||
device = abstractproperty() |
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.
Is this property going to contain combined device, package and speed grade? It would be more future-proof (and to a degree more convenient) if those were split, for several reasons. First, that makes it easier to branch on device, package, etc in nMigen code portable across a family that wants to change its behavior accordingly. Second, other tools, like nextpnr, generally use separate parameters for these.
nmigen/vendor/xilinx_s7.py
Outdated
* ``script_after_route``: inserts commands after ``route_design`` in Tcl script. | ||
* ``script_before_bitstream``: inserts commands before ``write_bitstream`` in Tcl script. | ||
* ``script_after_bitstream``: inserts commands after ``write_bitstream`` in Tcl script. | ||
* ``additional_constraints``: inserts commands in XDC file. |
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.
Tiny nit: I would prefer if this override was called add_constraints
.
No description provided.