Add ability to control default reset style. #34

docs/configuring.rst

@@ -20,3 +20,17 @@ All regblock-specific options are defined under the ``[regblock]`` TOML heading.
 
         [regblock]
         cpuifs.my-cpuif-name = "my_cpuif_module:MyCPUInterfaceClass"
+
+
+.. data:: default_reset
+
+    Choose the default style of reset signal if not explicitly
+    specified by the SystemRDL design. If unspecified, the default reset
+    is active-high and synchronous.
+
+    Choice of:
+
+        * ``rst`` (default)
+        * ``rst_n``
+        * ``arst``
+        * ``arst_n``

src/peakrdl_regblock/__peakrdl__.py

@@ -1,4 +1,4 @@
-from typing import TYPE_CHECKING, Dict, Type
+from typing import TYPE_CHECKING, Dict, Type, List, Any
 import functools
 import sys
 
@@ -15,13 +15,32 @@
     from systemrdl.node import AddrmapNode
 
 
+class Choice(schema.String):
+    """
+    Schema that matches against a specific set of allowed strings
+
+    Base PeakRDL does not have this schema yet. Polyfill here for now until it
+    is added and widely available.
+    """
+    def __init__(self, choices: List[str]) -> None:
+        super().__init__()
+        self.choices = choices
+
+    def extract(self, data: Any, path: str, err_ctx: str) -> Any:
+        s = super().extract(data, path, err_ctx)
+        if s not in self.choices:
+            raise schema.SchemaException(f"{err_ctx}: Value '{s}' is not a valid choice. Must be one of: {','.join(self.choices)}")
+        return s
+
+
 class Exporter(ExporterSubcommandPlugin):
     short_desc = "Generate a SystemVerilog control/status register (CSR) block"
 
     udp_definitions = ALL_UDPS
 
     cfg_schema = {
-        "cpuifs": {"*": schema.PythonObjectImport()}
+        "cpuifs": {"*": schema.PythonObjectImport()},
+        "default_reset": Choice(["rst", "rst_n", "arst", "arst_n"]),
     }
 
     @functools.lru_cache()
@@ -83,22 +102,6 @@ def add_exporter_arguments(self, arg_group: 'argparse.ArgumentParser') -> None:
             help="Override the SystemVerilog package name"
         )
 
-        arg_group.add_argument(
-            "--rt-read-fanin",
-            action="store_true",
-            default=False,
-            help="Enable additional read path retiming. Good for register blocks with large readback fan-in"
-        )
-        arg_group.add_argument(
-            "--rt-read-response",
-            action="store_true",
-            default=False,
-            help="Enable additional retiming stage between readback fan-in and cpu interface"
-        )
-        arg_group.add_argument(
-            "--rt-external",
-            help="Retime outputs to external components. Specify a comma-separated list of: reg,regfile,mem,addrmap,all"
-        )
         arg_group.add_argument(
             "--type-style",
             dest="type_style",
@@ -110,6 +113,7 @@ def add_exporter_arguments(self, arg_group: 'argparse.ArgumentParser') -> None:
             The 'hier' style uses component's hierarchy as the struct type name. [lexical]
             """
         )
+
         arg_group.add_argument(
             "--hwif-report",
             action="store_true",
@@ -126,6 +130,32 @@ def add_exporter_arguments(self, arg_group: 'argparse.ArgumentParser') -> None:
             """
         )
 
+        arg_group.add_argument(
+            "--rt-read-fanin",
+            action="store_true",
+            default=False,
+            help="Enable additional read path retiming. Good for register blocks with large readback fan-in"
+        )
+        arg_group.add_argument(
+            "--rt-read-response",
+            action="store_true",
+            default=False,
+            help="Enable additional retiming stage between readback fan-in and cpu interface"
+        )
+        arg_group.add_argument(
+            "--rt-external",
+            help="Retime outputs to external components. Specify a comma-separated list of: reg,regfile,mem,addrmap,all"
+        )
+
+        arg_group.add_argument(
+            "--default-reset",
+            choices=["rst", "rst_n", "arst", "arst_n"],
+            default=None,
+            help="""Choose the default style of reset signal if not explicitly
+            specified by the SystemRDL design. If unspecified, the default reset
+            is active-high and synchronous [rst]"""
+        )
+
 
     def do_export(self, top_node: 'AddrmapNode', options: 'argparse.Namespace') -> None:
         cpuifs = self.get_cpuifs()
@@ -153,6 +183,24 @@ def do_export(self, top_node: 'AddrmapNode', options: 'argparse.Namespace') -> N
                 else:
                     print("error: invalid option for --rt-external: '%s'" % key, file=sys.stderr)
 
+        # Get default reset. Favor command-line over cfg. Fall back to 'rst'
+        default_rst = options.default_reset or self.cfg['default_reset'] or "rst"
+        if default_rst == "rst":
+            default_reset_activelow = False
+            default_reset_async = False
+        elif default_rst == "rst_n":
+            default_reset_activelow = True
+            default_reset_async = False
+        elif default_rst == "arst":
+            default_reset_activelow = False
+            default_reset_async = True
+        elif default_rst == "arst_n":
+            default_reset_activelow = True
+            default_reset_async = True
+        else:
+            raise RuntimeError
+
+
         x = RegblockExporter()
         x.export(
             top_node,
@@ -169,4 +217,6 @@ def do_export(self, top_node: 'AddrmapNode', options: 'argparse.Namespace') -> N
             retime_external_addrmap=retime_external_addrmap,
             generate_hwif_report=options.hwif_report,
             address_width=options.addr_width,
+            default_reset_activelow=default_reset_activelow,
+            default_reset_async=default_reset_async,
         )

src/peakrdl_regblock/dereferencer.py

@@ -3,7 +3,7 @@
 from systemrdl.rdltypes import PropertyReference
 
 if TYPE_CHECKING:
-    from .exporter import RegblockExporter
+    from .exporter import RegblockExporter, DesignState
     from .hwif import Hwif
     from .field_logic import FieldLogic
     from .addr_decode import AddressDecode
@@ -28,6 +28,10 @@ def address_decode(self) -> 'AddressDecode':
     def field_logic(self) -> 'FieldLogic':
         return self.exp.field_logic
 
+    @property
+    def ds(self) -> 'DesignState':
+        return self.exp.ds
+
     @property
     def top_node(self) -> AddrmapNode:
         return self.exp.ds.top_node
@@ -211,6 +215,16 @@ def get_external_block_access_strobe(self, obj: 'AddressableNode') -> str:
         """
         return self.address_decode.get_external_block_access_strobe(obj)
 
+    @property
+    def default_resetsignal_name(self) -> str:
+        s = "rst"
+        if self.ds.default_reset_async:
+            s = f"a{s}"
+        if self.ds.default_reset_activelow:
+            s = f"{s}_n"
+        return s
+
+
     def get_resetsignal(self, obj: Optional[SignalNode] = None) -> str:
         """
         Returns a normalized active-high reset signal
@@ -222,13 +236,23 @@ def get_resetsignal(self, obj: Optional[SignalNode] = None) -> str:
             else:
                 return f"~{s}"
 
-        # default reset signal
-        return "rst"
+        # No explicit reset signal specified. Fall back to default reset signal
+        s = self.default_resetsignal_name
+        if self.ds.default_reset_activelow:
+            s = f"~{s}"
+        return s
 
     def get_always_ff_event(self, resetsignal: Optional[SignalNode] = None) -> str:
         if resetsignal is None:
-            return "@(posedge clk)"
-        if resetsignal.get_property('async') and resetsignal.get_property('activehigh'):
+            # No explicit reset signal specified. Fall back to default reset signal
+            if self.ds.default_reset_async:
+                if self.ds.default_reset_activelow:
+                    return f"@(posedge clk or negedge {self.default_resetsignal_name})"
+                else:
+                    return f"@(posedge clk or posedge {self.default_resetsignal_name})"
+            else:
+                return "@(posedge clk)"
+        elif resetsignal.get_property('async') and resetsignal.get_property('activehigh'):
             return f"@(posedge clk or posedge {self.get_value(resetsignal)})"
         elif resetsignal.get_property('async') and not resetsignal.get_property('activehigh'):
             return f"@(posedge clk or negedge {self.get_value(resetsignal)})"

src/peakrdl_regblock/exporter.py

@@ -113,6 +113,10 @@ def export(self, node: Union[RootNode, AddrmapNode], output_dir:str, **kwargs: A
         address_width: int
             Override the CPU interface's address width. By default, address width
             is sized to the contents of the regblock.
+        default_reset_activelow: bool
+            If overriden to True, default reset is active-low instead of active-high.
+        default_reset_async: bool
+            If overriden to True, default reset is asynchronous instead of synchronous.
         """
         # If it is the root node, skip to top addrmap
         if isinstance(node, RootNode):
@@ -140,10 +144,7 @@ def export(self, node: Union[RootNode, AddrmapNode], output_dir:str, **kwargs: A
 
         # Construct exporter components
         self.cpuif = cpuif_cls(self)
-        self.hwif = Hwif(
-            self,
-            hwif_report_file=hwif_report_file,
-        )
+        self.hwif = Hwif(self, hwif_report_file=hwif_report_file)
         self.readback = Readback(self)
         self.address_decode = AddressDecode(self)
         self.field_logic = FieldLogic(self)
@@ -163,6 +164,7 @@ def export(self, node: Union[RootNode, AddrmapNode], output_dir:str, **kwargs: A
             "write_buffering": self.write_buffering,
             "read_buffering": self.read_buffering,
             "get_resetsignal": self.dereferencer.get_resetsignal,
+            "default_resetsignal_name": self.dereferencer.default_resetsignal_name,
             "address_decode": self.address_decode,
             "field_logic": self.field_logic,
             "readback": self.readback,
@@ -207,7 +209,6 @@ def __init__(self, top_node: AddrmapNode, kwargs: Any) -> None:
         self.package_name = kwargs.pop("package_name", None) or (self.module_name + "_pkg") # type: str
         user_addr_width = kwargs.pop("address_width", None) # type: Optional[int]
 
-
         # Pipelining options
         self.retime_read_fanin = kwargs.pop("retime_read_fanin", False) # type: bool
         self.retime_read_response = kwargs.pop("retime_read_response", False) # type: bool
@@ -216,6 +217,10 @@ def __init__(self, top_node: AddrmapNode, kwargs: Any) -> None:
         self.retime_external_mem = kwargs.pop("retime_external_mem", False) # type: bool
         self.retime_external_addrmap = kwargs.pop("retime_external_addrmap", False) # type: bool
 
+        # Default reset type
+        self.default_reset_activelow = kwargs.pop("default_reset_activelow", False) # type: bool
+        self.default_reset_async = kwargs.pop("default_reset_async", False) # type: bool
+
         #------------------------
         # Info about the design
         #------------------------

src/peakrdl_regblock/module_tmpl.sv

@@ -3,7 +3,7 @@
 
 module {{ds.module_name}} (
         input wire clk,
-        input wire rst,
+        input wire {{default_resetsignal_name}},
 
         {%- for signal in ds.out_of_hier_signals.values() %}
         {%- if signal.width == 1 %}

src/peakrdl_regblock/readback/__init__.py

@@ -34,6 +34,7 @@ def get_implementation(self) -> str:
             "array_assignments" : array_assignments,
             "array_size" : array_size,
             'get_always_ff_event': self.exp.dereferencer.get_always_ff_event,
+            'get_resetsignal': self.exp.dereferencer.get_resetsignal,
             "cpuif": self.exp.cpuif,
             "do_fanin_stage": self.do_fanin_stage,
             "ds": self.ds,

src/peakrdl_regblock/readback/templates/readback.sv

@@ -29,8 +29,8 @@ end
 
 logic [{{cpuif.data_width-1}}:0] readback_array_r[{{fanin_array_size}}];
 logic readback_done_r;
-always_ff @(posedge clk) begin
-    if(rst) begin
+always_ff {{get_always_ff_event(cpuif.reset)}} begin
+    if({{get_resetsignal(cpuif.reset)}}) begin
         for(int i=0; i<{{fanin_array_size}}; i++) readback_array_r[i] <= '0;
         readback_done_r <= '0;
     end else begin

tests/lib/base_testcase.py

@@ -37,6 +37,8 @@ class BaseTestCase(unittest.TestCase):
     retime_read_response = False
     reuse_hwif_typedefs = True
     retime_external = False
+    default_reset_activelow = False
+    default_reset_async = False
 
     #: this gets auto-loaded via the _load_request autouse fixture
     request = None # type: pytest.FixtureRequest
@@ -111,6 +113,8 @@ def _export_regblock(cls):
             retime_external_regfile=cls.retime_external,
             retime_external_mem=cls.retime_external,
             retime_external_addrmap=cls.retime_external,
+            default_reset_activelow=cls.default_reset_activelow,
+            default_reset_async=cls.default_reset_async,
         )
 
     @classmethod

tests/lib/tb_base.sv

@@ -10,6 +10,11 @@ module tb;
         clk = ~clk;
     end
 
+    logic rst_n, arst, arst_n;
+    assign rst_n = ~rst;
+    assign arst = rst;
+    assign arst_n = ~rst;
+
     //--------------------------------------------------------------------------
     // DUT Signal declarations
     //--------------------------------------------------------------------------

tests/test_structural_sw_rw/testcase.py

@@ -4,15 +4,24 @@
 
 from ..lib.sim_testcase import SimTestCase
 from ..lib.synth_testcase import SynthTestCase
-from ..lib.test_params import TEST_PARAMS
+from ..lib.test_params import TEST_PARAMS, get_permutations
 from ..lib.simulators.xilinx import Xilinx
 import pytest
 
 @parameterized_class(TEST_PARAMS)
-class Test(SimTestCase):
+class TestParameterizations(SimTestCase):
     def test_dut(self):
         self.run_test()
 
+@parameterized_class(get_permutations({
+    "default_reset_activelow": [True, False],
+    "default_reset_async": [True, False],
+}))
+class TestDefaultResets(SimTestCase):
+    def test_dut(self):
+        self.run_test()
+
+
 @parameterized_class(TEST_PARAMS)
 class TestSynth(SynthTestCase):
     def test_dut(self):