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.

Sign up for GitHub

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

Jump to bottom

AttributeError when requesting an output pin with a clock constraint #646

Open
jfng opened this issue Nov 12, 2021 · 3 comments
Open

AttributeError when requesting an output pin with a clock constraint #646

jfng opened this issue Nov 12, 2021 · 3 comments
Labels
bug platform:all

Comments

@jfng
Copy link
Member

jfng commented Nov 12, 2021

Repro:

from nmigen import *
from nmigen_boards.arty_a7 import ArtyA7_35Platform

# Resource("eth_clk50", 0, Pins("G18", dir="o"),
#          Clock(50e6), Attrs(IOSTANDARD="LVCMOS33")),

platform = ArtyA7_35Platform()
eth_clk50 = platform.request("eth_clk50", 0)

m = Module()
m.d.comb += eth_clk50.o.eq(1)

platform.build(m, do_build=False)

Output:

Traceback (most recent call last):
  File "/tmp/repro.py", line 8, in <module>
    eth_clk50 = platform.request("eth_clk50", 0)
  File "/home/jf/src/nmigen/nmigen/build/res.py", line 164, in request
    value = resolve(resource,
  File "/home/jf/src/nmigen/nmigen/build/res.py", line 157, in resolve
    self.add_clock_constraint(pin.i, resource.clock.frequency)
  File "/home/jf/src/nmigen/nmigen/hdl/rec.py", line 146, in __getattr__
    return self[name]
  File "/home/jf/src/nmigen/nmigen/hdl/rec.py", line 157, in __getitem__
    raise AttributeError("{} does not have a field '{}'. Did you mean one of: {}?"
AttributeError: Record 'eth_clk50_0' does not have a field 'i'. Did you mean one of: o?

Currently, only two nmigen-boards platforms are affected (arty_a7 and nexys4ddr).

I'm not sure how to approach this:

  1. we could skip output clock constraints for now, and throw a warning when ResourceManager.request() encounters one. While not ideal, I think it's still better than preventing the design from building.
  2. we could shave this yak by implementing Support for generated clock constraints #498 and possibly Support for PLL primitives #425.
@whitequark
Copy link
Member

Why not add the constraint to the appropriate signal depending on the direction?

@jfng
Copy link
Member Author

jfng commented Nov 12, 2021

Why not add the constraint to the appropriate signal depending on the direction?

That is actually the workaround I used when I encountered this.

The issue is that on e.g. Xilinx platforms, this ends up as a create_clock constraint, which is inaccurate according to UG835 (p. 239):
ug835_clock

I don't know whether although inaccurate, it would still be better than no constraint at all.

@whitequark
Copy link
Member

I see. I'd need to take a closer look for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug platform:all
Milestone
No milestone
Development

No branches or pull requests
2 participants
@whitequark @jfng