bl0x / learn-fpga-amaranth

Code for Bruno Levy's learn-fpga tutorial written in Amaranth HDL
BSD 3-Clause "New" or "Revised" License
89 stars 12 forks source link

adding clockwork to top module gives error #8

Open chrisjohgorman opened 1 month ago

chrisjohgorman commented 1 month ago

Hello @bl0x,

First off let me thank you for your project. It is interesting and I am going to try and follow it to gain an understanding of amaranth.

I am trying to build with Vivado and have had success with the first step. I now have lighted leds on my cmod a7. When trying to build step 2, I get the following error.

step = 2
Traceback (most recent call last):
  File "/home/chris/src/amaranth/learn-fpga-amaranth/boards/digilent_cmod_a7.py", line 25, in <module>
    platform.build(Top(leds, uart), do_program=True)
  File "/usr/lib/python3.12/site-packages/amaranth/build/plat.py", line 99, in build
    plan = self.prepare(elaboratable, name, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/site-packages/amaranth/build/plat.py", line 140, in prepare
    fragment._propagate_domains(self.create_missing_domain, platform=self)
  File "/usr/lib/python3.12/site-packages/amaranth/hdl/_ir.py", line 240, in _propagate_domains
    self._propagate_domains_up()
  File "/usr/lib/python3.12/site-packages/amaranth/hdl/_ir.py", line 141, in _propagate_domains_up
    subfrag._propagate_domains_up(hierarchy + (hier_name,))
  File "/usr/lib/python3.12/site-packages/amaranth/hdl/_ir.py", line 184, in _propagate_domains_up
    self.add_domains(domain)
  File "/usr/lib/python3.12/site-packages/amaranth/hdl/_ir.py", line 91, in add_domains
    assert domain.name not in self.domains
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError

Systematically looking at the commands makes the following code from 02_slower_blinky/soc.py responsible as commenting it out starts the build for me.

# Add the clockwork to the top module. If this is not done,
# the logic will not be instantiated.
m.submodules.cw = cw

Do you have any idea on how to set the domain.name in self.domains?

chrisjohgorman commented 1 month ago

hello again,

Just to update you, I have circumvented this with IMHO a horrible hack to _ir.py. The hack seems to work and I can now process step 2.

The following patch is the one I used to get this to work...

--- /home/chris/install/python-amaranth/src/amaranth/amaranth/hdl/_ir.py       2024-10-04 16:12:59.534701199 -0400
+++ /usr/lib/python3.12/site-packages/amaranth/hdl/_ir.py       2024-10-05 11:00:50.106393002 -0400
@@ -86,9 +86,12 @@
         self.domain_renames = {}

     def add_domains(self, *domains):
+        count=0
         for domain in flatten(domains):
             assert isinstance(domain, _cd.ClockDomain)
-            assert domain.name not in self.domains
+            if (count != 0):
+                assert domain.name not in self.domains
+            count=count+1
             self.domains[domain.name] = domain

     def iter_domains(self):

Apologies for the spam on a weekend.

chrisjohgorman commented 1 month ago

I've decided to move this to the amaranth github issues page. I will report back on a fix if I get one.

Chris

chrisjohgorman commented 1 month ago

I have a "fix" for this problem for me. I wonder if there is a typo in clockworks.py. When I change the assignment

module.domains += ClockDomain(clockworks_domain_name)

to

module.domains = ClockDomain(clockworks_domain_name)

this fixes the build problem for me.

chrisjohgorman commented 1 month ago

I just got a note from @whitequark saying that my solution was not proper, so please disregard it. I will post something if I find a better solution.