StanfordVLSI / dragonphy2

Open Source PHY v2
Apache License 2.0
21 stars 3 forks source link

Remove combo logic loop in qr_4t1_mux_top #140

Closed sgherbst closed 3 years ago

sgherbst commented 3 years ago

There is currently a combo logic loop in qr_4t1_mux_top, where data appears on both the RHS and LHS of an assign statement: https://github.com/StanfordVLSI/dragonphy2/blob/1f1c97ba5f52445fe06aa39ac60585e053916806/vlog/chip_src/tx_16t1/qr_4t1_mux_top.sv#L47

I believe this was added to prevent glitches if clocks are non-overlapping, but there may be other ways to deal with this, for example given the 4-bit select signal {Q, I, QB, IB}:

However, because this is such a critical block, it would be helpful to think about what the intended digital circuit is, so that we can make sure that is what will be produced by the Verilog.

sgherbst commented 3 years ago

Feedback from Mark was to implement as the OR of 3-input ANDs, i.e.

assign data = ((clk_Q  & clk_I  & D1MQ ) |
               (clk_I  & clk_QB & D1MI ) |
               (clk_QB & clk_IB & D2MQB) |
               (clk_IB & clk_Q  & D2MIB)); 

This removes the combo loop issue and shows the synthesis intent more explicitly. There is still a potential for glitches, but he suggested we evaluate its impact to see whether we need to do anything specific to address it.

CansWang commented 3 years ago

Feedback from Mark was to implement as the OR of 3-input ANDs, i.e.

assign data = ((clk_Q  & clk_I  & D1MQ ) |
               (clk_I  & clk_QB & D1MI ) |
               (clk_QB & clk_IB & D2MQB) |
               (clk_IB & clk_Q  & D2MIB)); 

This removes the combo loop issue and shows the synthesis intent more explicitly. There is still a potential for glitches, but he suggested we evaluate its impact to see whether we need to do anything specific to address it.

The glitches are expected to happen, however, it is normally 1-2ps since otherwise we would adjust the PI control code to minimize it. A pulse with 1-2 ps width would be negligible as it is too sharp to propagate through parasitics.

Thanks for checking with Mark. @sgherbst