Recent changes break ECP5 platform examples

[bjonnh]

https://github.com/enjoy-digital/litex/commit/fb0c9e846d72fc4fe99ca329cc84a8d2537d3d8d

This seems to break building even simple SoCs or just block programs on ECP5: for example just trying to build the target of colorlight_i5.py (arguments --build)

Traceback (most recent call last):
  File "colorlight_i5.py", line 236, in <module>
    main()
  File "colorlight_i5.py", line 229, in main
    builder.build(**parser.toolchain_argdict)
  File "/home/fpga/oss-cad-suite/litex/litex/litex/soc/integration/builder.py", line 367, in build
    vns = self.soc.build(build_dir=self.gateware_dir, **kwargs)
  File "/home/fpga/oss-cad-suite/litex/litex/litex/soc/integration/soc.py", line 1322, in build
    return self.platform.build(self, *args, **kwargs)
  File "/home/fpga/oss-cad-suite/litex/litex/litex/build/lattice/platform.py", line 48, in build
    return self.toolchain.build(self, *args, **kwargs)
  File "/home/fpga/oss-cad-suite/litex/litex/litex/build/lattice/trellis.py", line 67, in build
    return YosysNextPNRToolchain.build(self, platform, fragment, **kwargs)
  File "/home/fpga/oss-cad-suite/litex/litex/litex/build/yosys_nextpnr_toolchain.py", line 126, in build
    return GenericToolchain.build(self, platform, fragment, **kwargs)
  File "/home/fpga/oss-cad-suite/litex/litex/litex/build/generic_toolchain.py", line 90, in build
    self.named_sc, self.named_pc = platform.resolve_signals(self._vns)
  File "/home/fpga/oss-cad-suite/litex/litex/litex/build/generic_platform.py", line 457, in resolve_signals
    name_dict = dict((k, vns.get_name(sig)) for k, sig in args.items())
  File "/home/fpga/oss-cad-suite/litex/litex/litex/build/generic_platform.py", line 457, in <genexpr>
    name_dict = dict((k, vns.get_name(sig)) for k, sig in args.items())
  File "/home/fpga/oss-cad-suite/litex/litex/litex/gen/fhdl/namer.py", line 252, in get_name
    if sig.name_override is not None:
AttributeError: 'NoneType' object has no attribute 'name_override'

It does the same on Linux and Windows and was working last week.

[bjonnh]

I confirm that going back to commit 53a0bc92e459ad440ae1a9fb9f6f24c600f658d6 solves this issue.

[zyp]

I just debugged the same issue. The problem is that fb0c9e8 and 53a0bc9 moved some checks from the individual platforms to GenericToolchain.add_period_constraint, but since LatticeTrellisToolchain is overriding add_period_constraint, those checks are now omitted.

Adding them to LatticeTrellisToolchain solves the problem:

diff --git a/litex/build/lattice/trellis.py b/litex/build/lattice/trellis.py
index 0f365c83..36786423 100644
--- a/litex/build/lattice/trellis.py
+++ b/litex/build/lattice/trellis.py
@@ -146,6 +146,10 @@ class LatticeTrellisToolchain(YosysNextPNRToolchain):
     }

     def add_period_constraint(self, platform, clk, period):
+        if clk is None:
+            return
+        if hasattr(clk, "p"):
+            clk = clk.p
         platform.add_platform_command("""FREQUENCY PORT "{clk}" {freq} MHz;""".format(
             freq=str(float(1/period)*1000), clk="{clk}"), clk=clk)

It looks like this might be a problem for some of the other toolchains as well.

[enjoy-digital]

Thanks @bjonnh and @zyp and sorry for this. This should be fixed with https://github.com/enjoy-digital/litex/commit/f5a9efd8ba42da36cd1ab6a63bc685dfe7ce317b. Will check LiteX-Boards CI with it.

[trabucayre]

@enjoy-digital too fast... I have written this but didn't have time to open the PR :)

[enjoy-digital]

@trabucayre: I just tried to fix what I broke :) we could still revisit this and improve in the future.

[bjonnh]

Thank you so much for the quick fix!

[enjoy-digital]

@bjonnh thanks for reporting!

[CarlFK]

fix confirmed by running https://github.com/bjonnh/fpga_class_psone/blob/main/setup_linux.sh

Thoughts on using this or a version of it for CI?

[enjoy-digital]

Thanks @CarlFK, the issue was also raised with current CI on LiteX-Boards but I would need to revisit CI trigger to catch such issues faster.