Kuree / kratos

:crossed_swords: Debuggable hardware generator
https://kratos-doc.readthedocs.io
BSD 2-Clause "Simplified" License
67 stars 9 forks source link

Uart example lacks reset values #155

Closed philipaxer closed 4 years ago

philipaxer commented 4 years ago

Hi,

It is good practice to have reset values for all registers in the design. The uart one is lacking this yet. Normally i would create a PR myself, but i would like to get an idea how you would do this.

E.g. is it possible to associate a reset value with a var and automagically create a lengthy if(rst) then ... end block based on available information? Is there a way to abstract the reset semantics (async/sync, active high/low)?

regards Philip

Kuree commented 4 years ago

Good catch. I didn't pay much attention when I was translating the Verilog code from the example website.

E.g. is it possible to associate a reset value with a var and automagically create a lengthy if(rst) then ... end block based on available information? Is there a way to abstract the reset semantics (async/sync, active high/low)?

I'd probably use reg_init: https://github.com/Kuree/kratos/blob/master/tests/test_generator.py#L737. I think it's hard coded into reset high.

However, these some experiments I had long time ago to mimic Chisel's reg_next interface, as indicated here. In retrospect, these functions of mine are poorly written and not able to handle the cases you just mentioned in an elegant way.

I will refactor these functions and unify the interface into the same function.

EDIT: I took closer look at the example code and it seems to use 1-block FSM style, which makes the modification difficult. I would just add lengthy if (rst) etc in this case. In general if the next state value is computed via an always_comb block, using reg_next is probably the best option.

Kuree commented 4 years ago

Closing this issue for now since the reset logic has been updated. Please let me know if you have any questions.