sdnellen / open-register-design-tool

Tool to generate register RTL, models, and docs using SystemRDL or JSpec input
Apache License 2.0
15 stars 3 forks source link

External Reset signals Causing RTL generation to fail #18

Closed sdnellen closed 4 years ago

sdnellen commented 4 years ago

From open-register-design-tool created by neenuprince: Juniper/open-register-design-tool#78

Hi, Recently we noticed that the external reset registers, ( registers that gets reset using an external reset signal) that has resetsignal attribute causes an Error and hence cannot generate RTL.

We had generated the RTL with these resetsignal attribute earlier, but even older releases seems to fail. the error we get is as below. ERROR : Unable to resolve rhs assignment reference test_rst_sig Below I have mentioned the Reg defns and addr map. Register Definition

signal test_rst_sig {  
   async;
} test_rst_sig;

regfile other_regs_defns_rf { reg attr_resetsignal_t { name = "Reg resetsignal"; desc = "A register with resetsignal to verify resetsignal works correctly"; default hw = r; //Register default hw field type to read/write default sw = rw; //Register default sw field type to read/write field { name = "rstsignal check"; desc = "Verifying that rstsignal works"; resetsignal = test_rst_sig; //signal used to reset field's register and hardware logic fieldwidth = 32; reset = 32'h0; } fieldrstextsig; }; attr_resetsignal_t attr_rstsignal
};

Top RDL address map

addrmap ordtreg_regs { test_rst_sig test_rst_sig; other_regs_defns_rf other_rf @ 0x000100; other_rf.attr_rstsignal.fieldrstextsig->resetsignal = test_rst_sig;

};

sdnellen commented 4 years ago

All versions of ordt until today do not support access of signals defined in parent hierarchy levels directly in the child, eg resetsignal = test_rst_sig; within fieldrstextsig throws an unable to resolve error. Using a dynamic assign in the parent, eg other_rf.attr_rstsignal.fieldrstextsig->resetsignal = test_rst_sig; does work correctly, however. As of 191222.01, both forms are supported.

See example in rdl_examples/issue_18.rdl

neenuprince commented 4 years ago

Hi , Pulling from this fork gives me a .gitignore merge conflict, since I clone from the master first. Was that intentional , that these are not updated in to the master?

neenuprince commented 4 years ago

Also one more clarification, can just one of the fields of a multifield register be reset by external reset? is that supported? Getting the below error whn I try to do that. VIO#1> [CHK3/error] Missing definition: One or more field in a register has the resetsignal definition and the other fields do not.

neenuprince commented 4 years ago

Just another request for enhancement. I noticed that currently , if a register has multiple fields, each of them has to be explicitly addressed with the resetsignal in the address map. is it possible that we can have other_rf.attr_packedfields->resetsignal = test_rst_sig; instead of other_rf.attr_packedfields.fieldrstextsig->resetsignal = test_rst_sig; other_rf.attr_packedfields.fieldsticky->resetsignal = test_rst_sig;
other_rf.attr_packedfields.fieldstickybit->resetsignal = test_rst_sig;

neenuprince commented 4 years ago

So I have ben able to generate the RTL now, I don't see the error any more.

sdnellen commented 4 years ago

Responses to your questions....

  1. assume that the merge conflict seen was relative to Juniper fork master? This wasn't intentional, but some files went out of sync relative to sdnellen when I left. Would just clone from sdnellen/master going forward - this is where enhancements/fixes will be done (nothing critical in Juniper fork is lost by moving)
  2. Yes, just one of the fields of a multifield register can be reset by external reset. The resetsignal property is applied at field level, so this is allowed per the standard. Not sure where the "VIO#1> [CHK3/error] Missing definition..." error is coming from, but this isn't an ordt error message.
  3. It is possible to apply a single reset signal to all fields in a register in a single statement. There are 2 ways to do this - the first below uses ordt field wildcard form, which is not part of the rdl standard....

    other_rf.attr_packedfields.*->resetsignal = test_rst_sig;

The other, which is part of the standard, is to add a default property assign statement in the enclosing register (or regfile) definition, ie...

    default resetsignal = test_rst_sig;