bluecmd / fejkon

Fibre Channel / FICON HBA implemented on FPGA
GNU General Public License v3.0
38 stars 2 forks source link

Investigate asynchronous and active-low reset #63

Open bluecmd opened 4 years ago

bluecmd commented 4 years ago

From the LowRISC style guide it is apparent that they are using asynchronous reset and active-low. Thinking about it, that does make a whole lot of sense, and might mean that reset in Qsys do not need to be associated with clocks anymore? That would help the design to be less complex.

They use sections like these:

  always_ff @(posedge clk_i or negedge rst_ni) begin : p_regs
    if (!rst_ni) begin
      dio_attr_q <= '0;
      mio_attr_q <= '0;
    end else begin
    end
  end

We should:

bluecmd commented 4 years ago

https://www.embedded.com/asynchronous-reset-synchronization-and-distribution-challenges-and-solutions talks a bit about the down-sides of async reset, like the metastability when reset is deasserted.

bluecmd commented 4 years ago

Alex uses synchronous.

The more I read the less I think we should change this right now. It would likely help timing as some combinatorial elements would be removed from the data path, but unsure to what cost.

bluecmd commented 4 years ago

Reading more, what Intel suggests is a synchronized synchronous reset scheme where the dessert is synchronized to the clock.

See "Example 12–11. Verilog Code for Synchronized Asynchronous Reset" from the Quartus handbook.

Also see Zip's discussion on proving the reset behavior here: https://zipcpu.com/formal/2018/04/12/areset.html

bluecmd commented 4 years ago

Looking at lowRISC it seems their reset manager emits dual-rank synchronized resets, but the IPs are made to consume asynchronous resets (without any synchronizations done).

I guess that's fine if you're sure the reset that is connected is a good one and actually synchronized (which you know if you make the reset controller), then you could do this trick to move the reset logic from the data path.

bluecmd commented 4 years ago

Anyway, I feel that insanity might be down this path and this is not currently an issue - keeping this bug open for potential future work but right now I'll stick to synchronous resets, if for nothing else because they are metastability safer.

bluecmd commented 4 years ago

@olofk "helpfully" provided me with even more reading material. https://olofkindgren.blogspot.com/2017/11/resetting-reset-handling.html

To his defense that's a really cool observation that as he mentions I have never seen talked about before. Might be worth investigating how the reset network is actually synthesized... some day.

bluecmd commented 4 years ago

Spent some time (sigh, for reals - this is the final thinking I am doing on this fascinating topic :D) on re-creating Olof's design above, and while the suggestion does lessen the load of the reset network as said - it is really annoying to get Quartus to give me an RTL that uses SCLR and ENA, and basically impossible from what I have seen to get it pre-fitting.

Post-mapping (notice muxes): image

Post-fitting (muxes gone and replaced by sclr and ena): image

That is with "Force Use of Synchronous Clear Signals" enabled for assignment and I think I had to use High-effort placement strategy as the example is too small to otherwise be considered for optimization. Both of these toggles are just by-product of the example being small and shouldn't be needed (and actually probably harmful for timing) for larger designs.