Skip to content
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

platform.add_clock_constraint does not work for instances with lattice diamond for machxo2 #546

Open
anuejn opened this issue Nov 19, 2020 · 18 comments

Comments

@anuejn
Copy link
Contributor

anuejn commented Nov 19, 2020

Similiar to #373 platform.add_clock_constraint does not work for instances in lattice diamond.
However, the same fix does not seem to be applicable. Do you have any ideas what is going wrong?

repro:

from amaranth import *
from amaranth.build import *
from amaranth_boards.tinyfpga_ax2 import *

class Test(Elaboratable):
    def elaborate(self, plat):
        m = Module()
        clk = Signal()
        m.submodules.inst = Instance("JTAGF", o_JTCK=clk)

        m.domains += ClockDomain("sync")
        m.d.comb += ClockSignal().eq(clk)

        plat.add_clock_constraint(clk, 2e6)

        plat.add_resources([Resource("gpio", 0, Pins("1", conn=("gpio", 0), dir="o"), Attrs(IOSTANDARD="LVCMOS33"))])
        gpio0 = plat.request("gpio", 0)
        m.d.sync += gpio0.o.eq(~gpio0)

        return m

TinyFPGAAX2Platform().build(Test())
@whitequark
Copy link
Member

What is the error you're getting?

@anuejn
Copy link
Contributor Author

anuejn commented Nov 19, 2020

in build/top_impl/top_impl_cck.rpt:

create_clock -name clk -period 500.0 [get_nets {clk$1}]
	@E::"/home/anuejn/tmp/repro/build/top.sdc":1:0:1:0|Source for clock clk not found in netlist.

@anuejn
Copy link
Contributor Author

anuejn commented Nov 19, 2020

build.zip

@whitequark
Copy link
Member

It probably doesn't like the $. That doesn't happen for ports because ports are named first. Try Instance("JTAGF", o_JTCK=ClockSignal()).

@anuejn
Copy link
Contributor Author

anuejn commented Nov 19, 2020

How can I constrain it then?

Sorry, something went wrong.

@whitequark
Copy link
Member

whitequark commented Nov 19, 2020

cd_sync = ClockDomain()
platform.add_clock_constraint(cd_sync.clk, freq)

Sorry, something went wrong.

@anuejn
Copy link
Contributor Author

anuejn commented Nov 19, 2020

Thanks that works. Somehow it doesnt work for my non minimized use case :(. I will try to find another (less minimized) example

Sorry, something went wrong.

@anuejn
Copy link
Contributor Author

anuejn commented Nov 19, 2020

This also doesnt work, probably due to hierarchy:

from nmigen import *
from nmigen.build import *
from nmigen_boards.tinyfpga_ax2 import *

class Test(Elaboratable):
    def elaborate(self, plat):
        m = Module()

        m.submodules.jtag = JTAG()

        return m

class JTAG(Elaboratable):
    def elaborate(self, plat):
        m = Module()

        cd_sync = ClockDomain("sync")
        m.domains += cd_sync
        plat.add_clock_constraint(cd_sync.clk, 2e6)


        plat.add_resources([Resource("gpio", 0, Pins("1", conn=("gpio", 0), dir="o"), Attrs(IOSTANDARD="LVCMOS33"))])
        gpio0 = plat.request("gpio", 0)
        m.d.sync += gpio0.eq(~gpio0)


        m.submodules.inst = Instance("JTAGF", o_JTCK=cd_sync.clk)

        return m

TinyFPGAAX2Platform().build(Test())

the error is


create_clock -name clk -period 500.0 [get_nets jtag/clk]
	@E::"/home/anuejn/tmp/repro/build/top.sdc":1:0:1:0|Source for clock clk not found in netlist.

build.zip

@whitequark
Copy link
Member

The second one is a very serious issue, and one we can already fix (the $ problem is more thorny). You should research how to specify hierarchy for Diamond.

@anuejn
Copy link
Contributor Author

anuejn commented Nov 19, 2020

Sadly I am not at all good with diamond but I will try my best.
Maybe @cr1901 can help here too?

@whitequark
Copy link
Member

I am not good with it either. I'm just persistent enough that eventually I fix this stuff when it becomes my responsibility.

@anuejn
Copy link
Contributor Author

anuejn commented Nov 19, 2020

Okay according to some microsemi guide to synplify pro you have to use "." as a hierarchy seperator jUsT iN SdC FiLeS.
And it works :).

Should i file a PR that fixes that?

@whitequark
Copy link
Member

Yep.

@whitequark
Copy link
Member

Or maybe use set_hierarchy_separator {/} ?

@anuejn
Copy link
Contributor Author

anuejn commented Nov 19, 2020

The first example really works when we escape $ with a \ in front of it as @cr1901 suggested :)

@anuejn
Copy link
Contributor Author

anuejn commented Nov 19, 2020

Or maybe use set_hierarchy_separator {/} ?

Tested. Works

@whitequark
Copy link
Member

whitequark commented May 30, 2024

Testing with 028d5d8: this is still a problem.

The issue isn't related to hierarchy. It is caused by the $ symbol in this SDC constraint:

 create_clock -name "clk" -period 500.0 [get_nets "clk\$2"]

To see how Tcl performs this escaping, consider:

% puts "clk\$2"
clk$2
% puts {clk\$2}
clk\$2

For some reason, {clk\$2} works. However, "clk$2" doesn't (crash), "clk\$2" doesn't (clock not found), "clk\\$2" doesn't (crash), and "clk\\\$2" doesn't (clock not found), and {clk$2} doesn't (clock not found).

It looks like the only option is to use the weird inconsistent escape scheme that Diamond seems to require. Sadness.

@whitequark
Copy link
Member

After further testing, Diamond also requires the clock to be specified using the canonical net, i.e. after bypassing all the aliases (wire alias = canonical;). This seems to happen early in the frontend and no attributes affect this behavior.

whitequark added a commit to whitequark/amaranth that referenced this issue May 30, 2024
Partially addresses amaranth-lang#546.
github-merge-queue bot pushed a commit that referenced this issue May 30, 2024
Partially addresses #546.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants