Closed DigitalBrains1 closed 1 year ago
Review comments on the user documentation are particularly welcome.
There's a circumstance I thought of that I found no good way to put into the documentation. In unsafeResetGlitchFilter
, about the period when it is still unasserted. I don't know how realistic it is, but during power-on the clock might not have stabilised. It might violate timing requirements if it is pulsing faster than the stable clock rate, which might mess up the Mealy machine in the reset glitch filter, potentially increasing the number of clock cycles spent unasserted. Then again, once the clock is stable, the amount of cycles will not exceed 2 * glitchlessPeriod. Perhaps it is understood that you don't start counting until the clock is stable. Or I'm just thinking too much about it.
Finally, this PR should probably wait for the discussed changes in #2539. That's the only reason why it is Draft.
The build failure due to a dependency loop will be solved by adding the constraint from #2539, which gets rid of clashCompileError
in resetGlitchFilter
, which is responsible for the dependency loop.
[edit]
I temporarily changed the clashCompileError
to error
so we can look at, for example, the generated Haddock. This term-level treatment will be replaced by a type-level treatment introduced in #2539 anyway.
[/edit]
The existing code for
resetGlitchFilter
was wrong: theerror
on domains with unknown initial values rendered as an X value in HDL instead of throwing an error.The new
unsafeResetGlitchFilter
allows to sidestep the restriction if the user is sure this works for them.The new
resetGlitchFilterWithReset
is the better solution for domains with unknown initial values, relying on an additional power-on reset for which assertion glitches don't occur.Both
resetSynchronizer
andresetGlitchFilter
no longer carry aNOINLINE
/OPAQUE
annotation. The only reason they had one was for nicer port/signal names, there was no technical reason to put them in a separate HDL file. Furthermore, somehow the names forresetSynchronizer
were not nice anyway, it would need-fno-do-lambda-eta-expansion
to work correctly.Still TODO: