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

Clock propagation enhancement #54

Merged

Conversation

tmichalak
Copy link
Collaborator

This PR aims at solving issue #53.

Added a distinction between three types of clocks:

  • explicit - added with create_clock command
  • generated - propagated clocks that have different clock parameters than the master clock (like PLLs or MMCMs)
  • propagated clocks - clock wires with the same clock parameters as their master clock

@mithro
Copy link
Collaborator

mithro commented Nov 24, 2020

I haven't actually looked at this pull request, but do please make sure we have a bunch of documentation around this.

@mithro
Copy link
Collaborator

mithro commented Nov 24, 2020

Also, if there is a conflict, which one wins?

file << "create_clock -period " << Clock::Period(clock_wire);
file << " -waveform {" << Clock::RisingEdge(clock_wire) << " "
<< Clock::FallingEdge(clock_wire) << "}";
file << " " << Clock::WireName(clock_wire);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why this is called SourcePinName, rather than SourceWireName? Does Yosys have a first class cell pin object?

Copy link
Collaborator Author

@tmichalak tmichalak Dec 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SOURCE_PIN was the attribute in Vivado, but we do use wires instead of pins.
Hence, I changed the attribute and method name according to what it really is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can yosys return the wire attached to a cell pin? If so, it might make more sense to stick with SOURCE_PIN, and change this code to get the wire from the source pin?

Copy link
Contributor

@litghost litghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good.

@litghost
Copy link
Contributor

Conflicts?

Signed-off-by: Tomasz Michalak <tmichalak@antmicro.com>
Signed-off-by: Tomasz Michalak <tmichalak@antmicro.com>
Signed-off-by: Tomasz Michalak <tmichalak@antmicro.com>
Signed-off-by: Tomasz Michalak <tmichalak@antmicro.com>
Signed-off-by: Tomasz Michalak <tmichalak@antmicro.com>
Signed-off-by: Tomasz Michalak <tmichalak@antmicro.com>
Signed-off-by: Tomasz Michalak <tmichalak@antmicro.com>
@tmichalak tmichalak force-pushed the clock_propagation_enhancement branch from 19619b4 to 352aea3 Compare December 1, 2020 16:09
@tmichalak
Copy link
Collaborator Author

Conflicts?

Resolved

@litghost litghost merged commit e8018aa into chipsalliance:master Dec 1, 2020
@tmichalak tmichalak deleted the clock_propagation_enhancement branch December 15, 2020 08:50
mglb pushed a commit to antmicro/yosys-f4pga-plugins that referenced this pull request Apr 3, 2023
…es/sv2v-dd95174

Bump sv2v from `9bc946c` to `dd95174`
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

Successfully merging this pull request may close these issues.

None yet

3 participants