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

[WIP] Initial implementation of ECP5 PLL instantiation #426

Closed
wants to merge 1 commit into from

Conversation

@GuzTech
Copy link

@GuzTech GuzTech commented Jul 8, 2020

This is the initial implementation where I took the liberty to be inspired by the Litex implementation.

Signed-off-by: Oguz Meteer info@guztech.nl

Signed-off-by: Oguz Meteer <info@guztech.nl>
@codecov
Copy link

@codecov codecov bot commented Jul 8, 2020

Codecov Report

Merging #426 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #426   +/-   ##
=======================================
  Coverage   81.72%   81.72%           
=======================================
  Files          40       40           
  Lines        6100     6100           
  Branches     1243     1243           
=======================================
  Hits         4985     4985           
+ Misses        935      934    -1     
- Partials      180      181    +1     
Impacted Files Coverage Δ
nmigen/build/run.py 31.25% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30e2f91...ca0fad2. Read the comment docs.

@GuzTech
Copy link
Author

@GuzTech GuzTech commented Jul 8, 2020

I have no doubt that the implementation can be improved, but I'm not entirely sure how. My Python skills aren't the best, and there are parts that I'm a bit iffy about.

Some examples:

  • In register_clkin, I don't know exactly how to check that the clkin parameter is a ClockSignal.
  • Also in register_clkin, I had an intermediate signal self.clkin that I commented out. That actually caused me to not define a new Module in elaborate, but in __init__, since I needed to assign the clkin parameter to self.clkin combinatorially. I found that an intermediate signal is unnecessary, so I now instantiate a new Module and return in it in elaborate.
  • In the Litex implementation, they always feed the last clock output back into the feedback input. Maybe we should make this optional?

@whitequark
Copy link
Member

@whitequark whitequark commented Jul 9, 2020

It's way too early to consider merging anything upstream. Let's discuss what API we want first in #425.

@GuzTech
Copy link
Author

@GuzTech GuzTech commented Jul 9, 2020

Of course. I just posted this just to have some code that we could look at.

@GuzTech GuzTech closed this Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants