ipbus/ipbus-firmware

Standardise example/evaluation boards clock constraints structure

Closed this issue · 6 comments

Constraint files included in the repository for example/evaluation board designs do not qdopt a standard structure nor have a standard clock constraint policy (e.g. naming convention, ipbus vs payload clock relationship).

This becomes very confusing in projects implementing the same payload firmware on multiple example designs, where the lack of constraints causes timing failures in some designs but not others.

In order to avoid that, 2 actions should be taken

  • apply the same clock naming convention to all boards, e.g. ipbus_clk
  • include the ipbus vs payload clock constraint in all designs in a separate clock constraint file. Keeping it separate has the advantage to give the option to users to exclude this additional constraints for designs where ipbus clock and payload clocks are actually relates (rare but possible case)

Examples:

  • derived clock names: defined in k800.tcl, vcu118.tcl, etc. but absent in zcu102_basex.tcl, pc053a.tcl, etc.

Some notes after a first pass on enclustra_ax3_pm3 and kc705_basex:

  • Some boards have a 'system clock' (from an on-board external oscillator) others expose a ref_clk (usually ethernet's clock through the gt). PCIe based designs probably use the axi clock. In summary, once gone through all example designs, a naming convention for infrastructure source must be established.
  • the enclustra and kc705 infra entities have an unconnected clk_aux port. This complicates a little setting up stadanard CDC tests on different board. It is also worth noting that the other clocks that infra provides are derived from the system/ethernet/axi clock via an MMCM (instantiated in the clocks_* entity used by each design). Not all the MMCMs ports are used; one of the spares can be used to generate an configurable aux clock (freq defined via generic)

Regarding clock constraints, some designs loosen the constraint on the clock reset register, some don't.

Example:

set_false_path -through [get_pins infra/clocks/rst_reg/Q]
set_false_path -through [get_nets infra/clocks/nuke_i]

Among those that do relax the timing on such registers, some use set_false_path -through others set_false_path -from -to. A common approach would be good, or where not possible, some motivation for choosing one or the other.

If I'm not mistaken, only pc053 remains to be sorted out.
Need to check the code because it seems to have a different structure from other example designs.

From Tom:

The use of set_false_path commands still appears to be slightly inconsistent between different > designs

  • kcu105_basex, pc053a: None
  • enclustra_ax3_pm3, kc705_gmii (but duplicated),
set_false_path -through [get_pins infra/clocks/rst_reg/Q]
set_false_path -through [get_nets infra/clocks/nuke_i]
  • k800, vcu118
set_false_path -from [get_pins infra/clocks/rst_reg/C] -to [get_pins infra/clocks/rst_ipb_ctrl_reg/D]
set_false_path -from [get_pins infra/clocks/rst_reg/C] -to [get_pins infra/clocks/rst_ipb_reg/D]```

Updated the constraints of board designs using the external system/oscillator clock to generate ipbus/aux clocks. The parent and the children are now declared unrelated.
this was effectively in place at the start of this ticket

set_false_path -from [get_pins infra/clocks/nuke_i_reg/C] -to [get_pins infra/clocks/nuke_d_reg/D]
set_false_path -from [get_pins infra/clocks/rst_reg/C] -to [get_pins infra/clocks/rst_ipb_ctrl_reg/D]
set_false_path -from [get_pins infra/clocks/rst_reg/C] -to [get_pins infra/clocks/rst_ipb_reg/D]

and now has been replaced by an explicit declaration

# Declare the oscillator clock, ipbus clock and aux clock as unrelated
set_clock_groups -asynchronous -group [get_clocks osc_clk] -group [get_clocks ipbus_clk] -group [get_clocks -include_generated_clocks [get_clocks clk_aux]]

Note: The example design are not able to catch this issue as no slave in ipbus_examples uses clk_aux. One or more CDC slaves must be added to ipbus_examples to provide the appropriate coverage.

Finally closed.