Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: m-labs/nmigen
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 21f2f8c46efe
Choose a base ref
...
head repository: m-labs/nmigen
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 99d205494a89
Choose a head ref
  • 4 commits
  • 9 files changed
  • 1 contributor

Commits on Aug 3, 2019

  1. hdl.ir: don't expose as ports missing domains added via elaboratables.

    The elaboratable is already likely driving the clk/rst signals in
    some way appropriate for the platform; if we expose them as ports
    nevertheless it will cause problems downstream.
    whitequark committed Aug 3, 2019
    Copy the full SHA
    9c28b61 View commit details
  2. hdl.ir: allow adding more than one domain in missing domain callback.

    This is useful for injecting complex power-on reset logic.
    whitequark committed Aug 3, 2019
    Copy the full SHA
    e0b54b4 View commit details
  3. build.plat,vendor: automatically create sync domain from default_clk.

    But only if it is not defined by the programmer.
    
    Closes #57.
    whitequark committed Aug 3, 2019
    Copy the full SHA
    8854ca0 View commit details
  4. hdl.dsl: reword m.If(~True) warning to be more clear.

    Before this commit, it only suggested one thing (silencing it) and
    that's wrong almost all of the time, so suggest the right thing
    instead.
    whitequark committed Aug 3, 2019
    Copy the full SHA
    99d2054 View commit details
22 changes: 19 additions & 3 deletions nmigen/build/plat.py
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@

from .. import __version__
from ..hdl.ast import *
from ..hdl.cd import *
from ..hdl.dsl import *
from ..hdl.ir import *
from ..back import rtlil, verilog
@@ -71,11 +72,27 @@ def build(self, fragment, name="top",

self.toolchain_program(products, name, **(program_opts or {}))

@abstractmethod
def create_missing_domain(self, name):
if name == "sync" and self.default_clk is not None:
clk_i = self.request(self.default_clk).i
if self.default_rst is not None:
rst_i = self.request(self.default_rst).i

m = Module()
m.domains += ClockDomain("sync", reset_less=self.default_rst is None)
m.d.comb += ClockSignal("sync").eq(clk_i)
if self.default_rst is not None:
m.d.comb += ResetSignal("sync").eq(rst_i)
return m

def prepare(self, fragment, name="top", **kwargs):
assert not self._prepared
self._prepared = True

fragment = Fragment.get(fragment, self)
fragment = fragment.prepare(ports=list(self.iter_ports()),
missing_domain=self.create_missing_domain)

def add_pin_fragment(pin, pin_fragment):
pin_fragment = Fragment.get(pin_fragment, self)
@@ -226,9 +243,8 @@ def toolchain_prepare(self, fragment, name, **kwargs):
autogenerated = "Automatically generated by nMigen {}. Do not edit.".format(__version__)

def emit_design(backend):
return {"rtlil": rtlil, "verilog": verilog}[backend].convert(
fragment, name=name, platform=self, ports=list(self.iter_ports()),
missing_domain=lambda name: None)
return {"rtlil": rtlil, "verilog": verilog}[backend].convert(fragment, name=name,
ports=list(self.iter_ports()), missing_domain=lambda name: None)

def emit_commands(format):
commands = []
5 changes: 3 additions & 2 deletions nmigen/hdl/dsl.py
Original file line number Diff line number Diff line change
@@ -171,8 +171,9 @@ def _check_signed_cond(self, cond):
if sign:
warnings.warn("Signed values in If/Elif conditions usually result from inverting "
"Python booleans with ~, which leads to unexpected results: ~True is "
"-2, which is truthful. "
"(Silence this warning with `m.If(x)` → `m.If(x.bool())`.)",
"-2, which is truthful. Replace `~flag` with `not flag`. (If this is "
"a false positive, silence this warning with "
"`m.If(x)` → `m.If(x.bool())`.)",
SyntaxWarning, stacklevel=4)
return cond

20 changes: 10 additions & 10 deletions nmigen/hdl/ir.py
Original file line number Diff line number Diff line change
@@ -363,21 +363,21 @@ def _create_missing_domains(self, missing_domain):
if value is None:
raise DomainError("Domain '{}' is used but not defined".format(domain_name))
if type(value) is ClockDomain:
domain = value
self.add_domains(value)
# And expose ports on the newly added clock domain, since it is added directly
# and there was no chance to add any logic driving it.
new_domains.append(value)
else:
new_fragment = Fragment.get(value, platform=None)
if new_fragment.domains.keys() != {domain_name}:
if domain_name not in new_fragment.domains:
defined = new_fragment.domains.keys()
raise DomainError(
"Fragment returned by missing domain callback should define exactly "
"one domain '{}', but defines domain(s) {}."
.format(domain_name,
", ".join("'{}'".format(n)
for n in new_fragment.domains.keys())))
"Fragment returned by missing domain callback does not define "
"requested domain '{}' (defines {})."
.format(domain_name, ", ".join("'{}'".format(n) for n in defined)))
new_fragment.flatten = True
self.add_subfragment(new_fragment)
domain = new_fragment.domains[domain_name]
self.add_domains(domain)
new_domains.append(domain)
self.add_domains(new_fragment.domains.values())
return new_domains

def _propagate_domains(self, missing_domain):
6 changes: 4 additions & 2 deletions nmigen/test/test_hdl_dsl.py
Original file line number Diff line number Diff line change
@@ -273,7 +273,8 @@ def test_If_signed_suspicious(self):
with self.assertWarns(SyntaxWarning,
msg="Signed values in If/Elif conditions usually result from inverting Python "
"booleans with ~, which leads to unexpected results: ~True is -2, which is "
"truthful. (Silence this warning with `m.If(x)` → `m.If(x.bool())`.)"):
"truthful. Replace `~flag` with `not flag`. (If this is a false positive, "
"silence this warning with `m.If(x)` → `m.If(x.bool())`.)"):
with m.If(~True):
pass

@@ -284,7 +285,8 @@ def test_Elif_signed_suspicious(self):
with self.assertWarns(SyntaxWarning,
msg="Signed values in If/Elif conditions usually result from inverting Python "
"booleans with ~, which leads to unexpected results: ~True is -2, which is "
"truthful. (Silence this warning with `m.If(x)` → `m.If(x.bool())`.)"):
"truthful. Replace `~flag` with `not flag`. (If this is a false positive, "
"silence this warning with `m.If(x)` → `m.If(x.bool())`.)"):
with m.Elif(~True):
pass

25 changes: 22 additions & 3 deletions nmigen/test/test_hdl_ir.py
Original file line number Diff line number Diff line change
@@ -415,12 +415,31 @@ def test_propagate_create_missing_fragment(self):
new_domains = f1._propagate_domains(missing_domain=lambda name: f2)
self.assertEqual(f1.domains.keys(), {"sync"})
self.assertEqual(f1.domains["sync"], f2.domains["sync"])
self.assertEqual(new_domains, [f1.domains["sync"]])
self.assertEqual(new_domains, [])
self.assertEqual(f1.subfragments, [
(f2, None)
])
self.assertTrue(f2.flatten)

def test_propagate_create_missing_fragment_many_domains(self):
s1 = Signal()
f1 = Fragment()
f1.add_driver(s1, "sync")

cd_por = ClockDomain("por")
cd_sync = ClockDomain("sync")
f2 = Fragment()
f2.add_domains(cd_por, cd_sync)

new_domains = f1._propagate_domains(missing_domain=lambda name: f2)
self.assertEqual(f1.domains.keys(), {"sync", "por"})
self.assertEqual(f2.domains.keys(), {"sync", "por"})
self.assertEqual(f1.domains["sync"], f2.domains["sync"])
self.assertEqual(new_domains, [])
self.assertEqual(f1.subfragments, [
(f2, None)
])

def test_propagate_create_missing_fragment_wrong(self):
s1 = Signal()
f1 = Fragment()
@@ -430,8 +449,8 @@ def test_propagate_create_missing_fragment_wrong(self):
f2.add_domains(ClockDomain("foo"))

with self.assertRaises(DomainError,
msg="Fragment returned by missing domain callback should define exactly "
"one domain 'sync', but defines domain(s) 'foo'."):
msg="Fragment returned by missing domain callback does not define requested "
"domain 'sync' (defines 'foo')."):
f1._propagate_domains(missing_domain=lambda name: f2)


4 changes: 4 additions & 0 deletions nmigen/vendor/lattice_ecp5.py
Original file line number Diff line number Diff line change
@@ -127,6 +127,10 @@ class LatticeECP5Platform(TemplatedPlatform):
"""
]

def create_missing_domain(self, name):
# No additional reset logic needed.
return super().create_missing_domain(name)

_single_ended_io_types = [
"HSUL12", "LVCMOS12", "LVCMOS15", "LVCMOS18", "LVCMOS25", "LVCMOS33", "LVTTL33",
"SSTL135_I", "SSTL135_II", "SSTL15_I", "SSTL15_II", "SSTL18_I", "SSTL18_II",
40 changes: 40 additions & 0 deletions nmigen/vendor/lattice_ice40.py
Original file line number Diff line number Diff line change
@@ -119,6 +119,46 @@ class LatticeICE40Platform(TemplatedPlatform):
"""
]

def create_missing_domain(self, name):
# For unknown reasons (no errata was ever published, and no documentation mentions this
# issue), iCE40 BRAMs read as zeroes for ~3 us after configuration and release of internal
# global reset. Note that this is a *time-based* delay, generated purely by the internal
# oscillator, which may not be observed nor influenced directly. For details, see links:
# * https://github.com/cliffordwolf/icestorm/issues/76#issuecomment-289270411
# * https://github.com/cliffordwolf/icotools/issues/2#issuecomment-299734673
#
# To handle this, it is necessary to have a global reset in any iCE40 design that may
# potentially instantiate BRAMs, and assert this reset for >3 us after configuration.
# (We add a margin of 5x to allow for PVT variation.) If the board includes a dedicated
# reset line, this line is ORed with the power on reset.
#
# The power-on reset timer counts up because the vendor tools do not support initialization
# of flip-flops.
if name == "sync" and self.default_clk is not None:
clk_i = self.request(self.default_clk).i
if self.default_rst is not None:
rst_i = self.request(self.default_rst).i

m = Module()
# Power-on-reset domain
m.domains += ClockDomain("ice40_por", reset_less=True)
delay = int(15e-6 * self.default_clk_frequency)
timer = Signal(max=delay)
ready = Signal()
m.d.comb += ClockSignal("ice40_por").eq(clk_i)
with m.If(timer == delay):
m.d.ice40_por += ready.eq(1)
with m.Else():
m.d.ice40_por += timer.eq(timer + 1)
# Primary domain
m.domains += ClockDomain("sync")
m.d.comb += ClockSignal("sync").eq(clk_i)
if self.default_rst is not None:
m.d.comb += ResetSignal("sync").eq(~ready | rst_i)
else:
m.d.comb += ResetSignal("sync").eq(~ready)
return m

def should_skip_port_component(self, port, attrs, component):
# On iCE40, a differential input is placed by only instantiating an SB_IO primitive for
# the pin with z=0, which is the non-inverting pin. The pinout unfortunately differs
4 changes: 4 additions & 0 deletions nmigen/vendor/xilinx_7series.py
Original file line number Diff line number Diff line change
@@ -122,6 +122,10 @@ class Xilinx7SeriesPlatform(TemplatedPlatform):
"""
]

def create_missing_domain(self, name):
# No additional reset logic needed.
csuper().create_missing_domain(name)

def _get_xdr_buffer(self, m, pin, i_invert=None, o_invert=None):
def get_dff(clk, d, q):
# SDR I/O is performed by packing a flip-flop into the pad IOB.
36 changes: 20 additions & 16 deletions nmigen/vendor/xilinx_spartan_3_6.py
Original file line number Diff line number Diff line change
@@ -59,6 +59,23 @@ class XilinxSpartan3Or6Platform(TemplatedPlatform):
package = abstractproperty()
speed = abstractproperty()

@property
def family(self):
device = self.device.upper()
if device.startswith("XC3S"):
if device.endswith("A"):
return "3A"
elif device.endswith("E"):
raise NotImplementedError("""Spartan 3E family is not supported
as a nMigen platform.""")
else:
raise NotImplementedError("""Spartan 3 family is not supported
as a nMigen platform.""")
elif device.startswith("XC6S"):
return "6"
else:
assert False

file_templates = {
**TemplatedPlatform.build_script_templates,
"{{name}}.v": r"""
@@ -142,22 +159,9 @@ class XilinxSpartan3Or6Platform(TemplatedPlatform):
"""
]

@property
def family(self):
device = self.device.upper()
if device.startswith("XC3S"):
if device.endswith("A"):
return "3A"
elif device.endswith("E"):
raise NotImplementedError("""Spartan 3E family is not supported
as a nMigen platform.""")
else:
raise NotImplementedError("""Spartan 3 family is not supported
as a nMigen platform.""")
elif device.startswith("XC6S"):
return "6"
else:
assert False
def create_missing_domain(self, name):
# No additional reset logic needed.
return super().create_missing_domain(name)

def _get_xdr_buffer(self, m, pin, i_invert=None, o_invert=None):
def get_dff(clk, d, q):