[rtl] Fix non-DSP reset in ibex_counter

[nasahlpa]

When targeting Xilinx FPGAs, we utilize a DSP for counters with a width of less than 49-bit. In this case, a sync. reset is needed. However, currently, there is a bug in the RTL where also a sync. reset is used for the non-DSP counters on the FPGA.

[nasahlpa]

Apologies, I should have re-generated the bitstream after addressing this review comment. I assumed this is purely cosmetically but it actually led to a Vivado synthesis failure :/

[vogelpi]

Thanks for fixing this @nasahlpa .

To me it looks like this PR adds a lot more code and makes things much less readable and more complex. It negatively impacts maintenance. I think what we should do:

1. Revert the previous PR which introduced the issue.
2. Do a PR that maybe doesn't address the comment you mentioned but that implements the change without making it much more complex.

[nasahlpa]

As discussed offline:

• I've reverted the previous commit in PR Revert "[rtl] Fix counter reset value on FPGA" #2229
• Tried to create a submodule for the Xilinx specific flop. However, with this Vivado decided to ignore the use_dsp pragma. The solution now in this PR is the only implementation that (i) uses DSPs when it should and (ii) does not trigger a Vivado warning.

[vogelpi]

Thanks @nasahlpa , this is pretty elegant :-)

[vogelpi]

I think it would be better to use an int type here as well for consistency. Thanks for adding the comment on Vivado requiring int instead of string.

[nasahlpa]

Fixed, thanks.

[vogelpi]

Please don't use generate regions, some tools don't support this and this is why our style guide doesn't permit it: https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md#generate-constructs

[nasahlpa]

Fixed, thanks.

[vogelpi]

Thanks Pascal for addressing the remaining comments, I think we can merge this today.