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

An option to reject inputs where some of the clock nets have no defined or derived constraints #1335

Open
whitequark opened this issue Jun 29, 2024 · 4 comments

Comments

@whitequark
Copy link
Member

Due to a combination of factors, I've realized recently that I've been building Glasgow bitstreams with the default 12 MHz constraint, which of course did not work at 80 MHz post-PLL.

We have --pcf-allow-unconstrained, I propose --clk-allow-unconstrained and similarly switching the default (or at the very least, --clk-deny-unconstrained to be available as an option).

@rowanG077
Copy link
Contributor

rowanG077 commented Jun 29, 2024

I would rather propose making an unconstrained clock a warning. And then you can enable warning as error.

@whitequark
Copy link
Member Author

whitequark commented Jun 30, 2024

I would rather propose making an unconstrained clock a warning.

When would you ever want an unconstrained clock in a real design?

Unconstrained pins are reasonably common, like if you haven't done PCB layout yet. Unconstrained clocks essentially result in garbage-in, garbage-out, unless your clock is 12 MHz or less, which is fairly uncommon in amaranth-boards at least (which I think is representative of what people use).

Also, I'm not sure if the default 12 MHz frequency of unconstrained clocks propagates through PLLs, which can be even worse.

@rowanG077
Copy link
Contributor

rowanG077 commented Jun 30, 2024

I agree that you never want an unconstrained clock in a real design. But breaking peoples builds for relying on something that is bad practice but works is kinda heavy handed. A warning on the short term, maybe with a notice the warning will be upgraded to an error in the future, will at least give people time to move.

@whitequark
Copy link
Member Author

A warning on the short term, maybe with a notice the warning will be upgraded to an error in the future will at least give people time to move.

That's fine by me. What I want to avoid is any situation where a substring is matched in the warning to elevate it to an error, like Yosys does. It should be a dedicated option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants