Skip to content

C
Conv TTL Blocking - Gateware
Commit a06d4d47 authored by Evangelia Gousiou's avatar Evangelia Gousiou
Browse files
Options

eva_g first comments

parent 7e23550b
Hide whitespace changes
Inline Side-by-side
Showing with 7 additions and 3 deletions
doc/code-review/v4-27-01-2017/eva_g.txt
View file @ a06d4d47
......@@ -18,19 +18,23 @@ conv_dyn_burst_ctrl
- line 100: could be: th_array_lgth <= 6 when pwidth = 5 else 15;
- line 153: could use gc_sync_ffs; otherwise pulse_train_in_f_edge_p could also be initialized if (rst_n_i = '0')
and could use another dff before the edge detection for metastability
line 122: signals: test, temp_fall not used
- line 153: could use gc_sync_ffs (with rst_n_i <= rst_n_i and en_i); otherwise pulse_train_in_f_edge_p could also be initialized if (rst_n_i = '0')
and another dff could be used before the edge detection for metastability
- line 138: en_i not in sensitivity list for sim; process not very clear and the comment "Generate the
pulse on rising edge of pulse_burst_i" is not clear.
could the pulse_train_in assignments be moved in the p_thermal_fsm_outputs process or
do sth like "pulse_train_in <= '0' when burst_ctrl_rst = '1' else pulse_burst_i and en_i;"?
- line 176: "elsif (en_i = '1')" could be skipped; if en_i = '0' there will be no pulse_train_in_r_edge_p
- line 237: 6 could be replaced by a constant
- line 286: should be moved inside the "if" (after line 287)
- line 299: i think this could be skipped, as in this case the FSM will already be in the PULSE_REJECT state
- line 299: this could be skipped, as in this case the FSM will already be in the PULSE_REJECT state
- line 315: should be moved inside an "else"
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment