From b350da3e7c2de644c7a7b8da46a52305cdd7ed78 Mon Sep 17 00:00:00 2001 From: Alex Mykyta Date: Sat, 13 May 2023 17:15:31 -0700 Subject: [PATCH] Add ability to control default reset style. #34 --- docs/configuring.rst | 14 +++ src/peakrdl_regblock/__peakrdl__.py | 98 ++++++++++++++----- src/peakrdl_regblock/dereferencer.py | 34 ++++++- src/peakrdl_regblock/exporter.py | 15 ++- src/peakrdl_regblock/module_tmpl.sv | 2 +- src/peakrdl_regblock/readback/__init__.py | 1 + .../readback/templates/readback.sv | 4 +- tests/lib/base_testcase.py | 4 + tests/lib/tb_base.sv | 5 + tests/test_structural_sw_rw/testcase.py | 13 ++- 10 files changed, 151 insertions(+), 39 deletions(-) diff --git a/docs/configuring.rst b/docs/configuring.rst index 56b4287..4f1dd39 100644 --- a/docs/configuring.rst +++ b/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`` diff --git a/src/peakrdl_regblock/__peakrdl__.py b/src/peakrdl_regblock/__peakrdl__.py index 30dadc2..b08131a 100644 --- a/src/peakrdl_regblock/__peakrdl__.py +++ b/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 @@ if TYPE_CHECKING: 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,6 +102,34 @@ class Exporter(ExporterSubcommandPlugin): help="Override the SystemVerilog package name" ) + arg_group.add_argument( + "--type-style", + dest="type_style", + choices=['lexical', 'hier'], + default="lexical", + help="""Choose how HWIF struct type names are generated. + The 'lexical' style will use RDL lexical scope & type names where + possible and attempt to re-use equivalent type definitions. + The 'hier' style uses component's hierarchy as the struct type name. [lexical] + """ + ) + + arg_group.add_argument( + "--hwif-report", + action="store_true", + default=False, + help="Generate a HWIF report file" + ) + + arg_group.add_argument( + "--addr-width", + type=int, + default=None, + help="""Override the CPU interface's address width. By default, + address width is sized to the contents of the regblock. + """ + ) + arg_group.add_argument( "--rt-read-fanin", action="store_true", @@ -99,31 +146,14 @@ class Exporter(ExporterSubcommandPlugin): "--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", - choices=['lexical', 'hier'], - default="lexical", - help="""Choose how HWIF struct type names are generated. - The 'lexical' style will use RDL lexical scope & type names where - possible and attempt to re-use equivalent type definitions. - The 'hier' style uses component's hierarchy as the struct type name. [lexical] - """ - ) - arg_group.add_argument( - "--hwif-report", - action="store_true", - default=False, - help="Generate a HWIF report file" - ) arg_group.add_argument( - "--addr-width", - type=int, + "--default-reset", + choices=["rst", "rst_n", "arst", "arst_n"], default=None, - help="""Override the CPU interface's address width. By default, - address width is sized to the contents of the regblock. - """ + 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]""" ) @@ -153,6 +183,24 @@ class Exporter(ExporterSubcommandPlugin): 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 @@ class Exporter(ExporterSubcommandPlugin): 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, ) diff --git a/src/peakrdl_regblock/dereferencer.py b/src/peakrdl_regblock/dereferencer.py index d3ce1b6..6198fea 100644 --- a/src/peakrdl_regblock/dereferencer.py +++ b/src/peakrdl_regblock/dereferencer.py @@ -3,7 +3,7 @@ from systemrdl.node import AddrmapNode, FieldNode, SignalNode, RegNode, Addressa 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 @@ class Dereferencer: 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 @@ class Dereferencer: """ 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 @@ class Dereferencer: 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)})" diff --git a/src/peakrdl_regblock/exporter.py b/src/peakrdl_regblock/exporter.py index 47fa74b..b19d283 100644 --- a/src/peakrdl_regblock/exporter.py +++ b/src/peakrdl_regblock/exporter.py @@ -113,6 +113,10 @@ class RegblockExporter: 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 @@ class RegblockExporter: # 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 @@ class RegblockExporter: "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 @@ class DesignState: 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 @@ class DesignState: 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 #------------------------ diff --git a/src/peakrdl_regblock/module_tmpl.sv b/src/peakrdl_regblock/module_tmpl.sv index ca22b20..c7e4527 100644 --- a/src/peakrdl_regblock/module_tmpl.sv +++ b/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 %} diff --git a/src/peakrdl_regblock/readback/__init__.py b/src/peakrdl_regblock/readback/__init__.py index ef27fec..da441b1 100644 --- a/src/peakrdl_regblock/readback/__init__.py +++ b/src/peakrdl_regblock/readback/__init__.py @@ -34,6 +34,7 @@ class Readback: "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, diff --git a/src/peakrdl_regblock/readback/templates/readback.sv b/src/peakrdl_regblock/readback/templates/readback.sv index 564acbe..b98d3e6 100644 --- a/src/peakrdl_regblock/readback/templates/readback.sv +++ b/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 diff --git a/tests/lib/base_testcase.py b/tests/lib/base_testcase.py index d0579ea..ca016d0 100644 --- a/tests/lib/base_testcase.py +++ b/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 @@ class BaseTestCase(unittest.TestCase): 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 diff --git a/tests/lib/tb_base.sv b/tests/lib/tb_base.sv index db71b47..a0b7442 100644 --- a/tests/lib/tb_base.sv +++ b/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 //-------------------------------------------------------------------------- diff --git a/tests/test_structural_sw_rw/testcase.py b/tests/test_structural_sw_rw/testcase.py index fa9db57..d6599a6 100644 --- a/tests/test_structural_sw_rw/testcase.py +++ b/tests/test_structural_sw_rw/testcase.py @@ -4,15 +4,24 @@ from parameterized import parameterized_class 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):