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

Constraint file needs some love and attention #761

Open
MJoergen opened this issue Dec 25, 2023 · 2 comments
Open

Constraint file needs some love and attention #761

MJoergen opened this issue Dec 25, 2023 · 2 comments
Labels
bug Confirmed bug.

Comments

@MJoergen
Copy link
Contributor

When building the bitstream for R5 (using make bin/mega65r5.bit) I notice in Vivado's synthesis log:

CRITICAL WARNING: [Common 17-165] Too many positional options when parsing 'eth_clock', please type 'create_clock -help' for usage info. [/home/mike/git/MEGA65/mega65-core/src/vhdl/mega65r5.xdc:544]
WARNING: [Vivado 12-627] No clocks matched 'eth_rx_clock'. [/home/mike/git/MEGA65/mega65-core/src/vhdl/mega65r5.xdc:545]

The reason seems to be the use of an incorrect character for the dash/hyphen in the constraint file.
The line in the constraint file is

create_clock –name eth_rx_clock –period  20 –waveform  {0 10} [get_ports {eth_clock}]

However, it should instead be:

create_clock -name eth_rx_clock -period  20 -waveform  {0 10} [get_ports {eth_clock}]

Note the subtle change of the glyph used for the dash/hyphen!

When trying this change, I see the critical warning goes away as expected. However, instead I get a large number of timing violations. The biggest timing violation is a Clock Domain Crossing from eth_rx_clock to clock200, where the slack is -9 ns.

I suggest reviewing the various clocks used in the system, and the associated constraints. The clock summary from Vivado shows the following table:

Clock                 Waveform(ns)         Period(ns)      Frequency(MHz)
-----                 ------------         ----------      --------------
CLK_IN                {0.000 5.000}        10.000          100.000
  CLKFBOUT            {0.000 5.000}        10.000          100.000
  CLKOUT0             {0.000 6.736}        13.472          74.227
  CLKOUT1             {0.000 7.222}        14.444          69.231
    clk_fb_adjust1    {0.000 7.222}        14.444          69.231
    u_clock124mhz     {0.000 4.012}        8.025           124.615
      clk_fb_adjust2  {0.000 4.012}        8.025           124.615
      u_clock9969mhz  {0.000 5.015}        10.031          99.692
        clk_fb        {0.000 5.015}        10.031          99.692
        clk_fb_sdram  {0.000 5.015}        10.031          99.692
        clock163      {0.000 3.086}        6.173           162.000
        clock27       {0.000 18.519}       37.037          27.000
        clock270      {0.000 1.852}        3.704           270.000
        clock325      {0.000 1.543}        3.086           324.000
          hr_clk      {0.000 6.173}        12.346          81.000
        clock41       {0.000 12.346}       24.691          40.500
        clock81p      {0.000 6.173}        12.346          81.000
        u_clock163m   {3.086 6.173}        6.173           162.000
  clk_fb_eth          {0.000 5.000}        10.000          100.000
  clko_fb             {0.000 5.000}        10.000          100.000
  clock200            {0.000 2.500}        5.000           200.000
  clock50             {0.000 10.000}       20.000          50.000
  clock60             {0.000 8.333}        16.667          60.000
eth_rx_clock          {0.000 10.000}       20.000          50.000

This shows that the clock eth_rx_clock is independent from all the other clocks. However, that I think is wrong. The clock eth_rx_clock is connected to the top level port eth_clock, which is an output, i.e. generated by the FPGA. It therefore seems more reasonable that eth_rx_clock should be constrained using a create_generated_clock command.

@MJoergen MJoergen added the new New report, not classified yet label Dec 25, 2023
@lydon42 lydon42 added bug Confirmed bug. and removed new New report, not classified yet labels Dec 25, 2023
@lydon42
Copy link
Member

lydon42 commented Dec 25, 2023

Same is true for mega65r4.xdc

@lydon42
Copy link
Member

lydon42 commented Jul 28, 2024

Some changes:

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

No branches or pull requests

2 participants