From 21a4e5a41c39bc20ffe08d141235be1ed1c83131 Mon Sep 17 00:00:00 2001 From: Alex Mykyta Date: Mon, 3 Oct 2022 21:36:49 -0700 Subject: [PATCH] Add double-buffer UDP definitions --- docs/index.rst | 8 +- setup.py | 2 +- src/peakrdl_regblock/__peakrdl__.py | 3 + src/peakrdl_regblock/scan_design.py | 22 +++++ src/peakrdl_regblock/udps.py | 120 ++++++++++++++++++++++++++++ tests/lib/base_testcase.py | 5 ++ tests/pylint.rc | 3 +- 7 files changed, 160 insertions(+), 3 deletions(-) create mode 100644 src/peakrdl_regblock/udps.py diff --git a/docs/index.rst b/docs/index.rst index e519168..60b2da1 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -38,11 +38,12 @@ Below is a simple example that demonstrates how to generate a SystemVerilog implementation from SystemRDL source. .. code-block:: python - :emphasize-lines: 2-3, 23-27 + :emphasize-lines: 2-4, 29-33 from systemrdl import RDLCompiler, RDLCompileError from peakrdl_regblock import RegblockExporter from peakrdl_regblock.cpuif.apb3 import APB3_Cpuif + from peakrdl_regblock.udps import ALL_UDPS input_files = [ "PATH/TO/my_register_block.rdl" @@ -50,6 +51,11 @@ implementation from SystemRDL source. # Create an instance of the compiler rdlc = RDLCompiler() + + # Register all UDPs that 'regblock' requires + for udp in ALL_UDPS: + rdlc.register_udp(udp) + try: # Compile your RDL files for input_file in input_files: diff --git a/setup.py b/setup.py index c182186..eaec68a 100644 --- a/setup.py +++ b/setup.py @@ -24,7 +24,7 @@ setuptools.setup( include_package_data=True, python_requires='>=3.6', install_requires=[ - "systemrdl-compiler>=1.23.0", + "systemrdl-compiler>=1.25.0", "Jinja2>=2.11", ], entry_points = { diff --git a/src/peakrdl_regblock/__peakrdl__.py b/src/peakrdl_regblock/__peakrdl__.py index 36771c3..8a7d308 100644 --- a/src/peakrdl_regblock/__peakrdl__.py +++ b/src/peakrdl_regblock/__peakrdl__.py @@ -2,6 +2,7 @@ from typing import TYPE_CHECKING from .exporter import RegblockExporter from .cpuif import apb3, apb4, axi4lite, passthrough +from .udps import ALL_UDPS if TYPE_CHECKING: import argparse @@ -23,6 +24,8 @@ CPUIF_DICT = { class Exporter: short_desc = "Generate a SystemVerilog control/status register (CSR) block" + udp_definitions = ALL_UDPS + def add_exporter_arguments(self, arg_group: 'argparse.ArgumentParser') -> None: arg_group.add_argument( "--cpuif", diff --git a/src/peakrdl_regblock/scan_design.py b/src/peakrdl_regblock/scan_design.py index d48db88..047453a 100644 --- a/src/peakrdl_regblock/scan_design.py +++ b/src/peakrdl_regblock/scan_design.py @@ -146,3 +146,25 @@ class DesignScanner(RDLListener): self.out_of_hier_signals[path] = value else: self.in_hier_signal_paths.add(path) + + + # 10.6.1-f: Any field that is software-writable or clear on read shall + # not span multiple software accessible sub-words (e.g., a 64-bit + # register with a 32-bit access width may not have a writable field with + # bits in both the upper and lower half of the register). + # + # Interpreting this further - this rule applies any time a field is + # software-modifiable by any means, including rclr, rset, ruser + # TODO: suppress this check for registers that have the appropriate + # buffer_writes/buffer_reads UDP set + parent_accesswidth = node.parent.get_property('accesswidth') + parent_regwidth = node.parent.get_property('regwidth') + if ((parent_accesswidth < parent_regwidth) + and (node.lsb // parent_accesswidth) != (node.msb // parent_accesswidth) + and (node.is_sw_writable or node.get_property('onread') is not None)): + # Field spans across sub-words + self.msg.error( + "Software-modifiable field '%s' shall not span multiple software-accessible subwords." + % node.inst_name, + node.inst.inst_src_ref + ) diff --git a/src/peakrdl_regblock/udps.py b/src/peakrdl_regblock/udps.py new file mode 100644 index 0000000..e06de80 --- /dev/null +++ b/src/peakrdl_regblock/udps.py @@ -0,0 +1,120 @@ +from typing import Any + +from systemrdl.udp import UDPDefinition +from systemrdl.component import Reg +from systemrdl.rdltypes.references import RefType, PropertyReference +from systemrdl.node import Node, RegNode, VectorNode + + +class xBufferTrigger(UDPDefinition): + valid_components = {Reg} + valid_type = RefType + + def validate(self, node: Node, value: Any) -> None: + # TODO: Reference shall not cross an internal/external boundary + + if isinstance(value, VectorNode): + # Trigger can reference a vector, but only if it is a single-bit + if value.width != 1: + self.msg.error( + "%s '%s' references %s '%s' but it's width is not 1" + % ( + type(node.inst).__name__.lower(), node.inst_name, + type(value.inst).__name__.lower(), value.inst_name + ), + self.get_src_ref(node) + ) + elif isinstance(value, PropertyReference) and value.width is not None: + # Trigger can reference a property, but only if it is a single-bit + if value.width != 1: + self.msg.error( + "%s '%s' references property '%s->%s' but it's width is not 1" + % ( + type(node.inst).__name__.lower(), node.inst_name, + value.node.inst_name, value.name, + ), + self.get_src_ref(node) + ) + elif isinstance(value, RegNode): + # Trigger can reference a register, which implies access of the + # 'correct half' of the register is the trigger. + # For buffered writes, it is the upper-half. + # For buffered reads, it is the lower-half. + pass + else: + # All other reference types are invalid + self.msg.error( + "Reference to a %s component is incompatible with the '%s' property." + % (type(node.inst).__name__.lower(), self.name), + self.get_src_ref(node) + ) + +#------------------------------------------------------------------------------- +class BufferWrites(UDPDefinition): + name = "buffer_writes" + valid_components = {Reg} + valid_type = bool + default_assignment = True + + def validate(self, node: 'Node', value: Any) -> None: + assert isinstance(node, RegNode) + if value: + if not node.has_sw_writable: + self.msg.error( + "'buffer_writes' is set to true, but this register does not contain any writable fields.", + self.get_src_ref(node) + ) + # TODO: Should I limit the use of other properties on double-buffered registers? + + def get_unassigned_default(self, node: 'Node') -> Any: + return False + + +class WBufferTrigger(xBufferTrigger): + name = "wbuffer_trigger" + + def get_unassigned_default(self, node: 'Node') -> Any: + # If buffering is enabled, trigger is the register itself + if node.get_property('buffer_writes'): + return node + return None + + +class BufferReads(UDPDefinition): + name = "buffer_reads" + valid_components = {Reg} + valid_type = bool + default_assignment = True + + def validate(self, node: 'Node', value: Any) -> None: + assert isinstance(node, RegNode) + if value: + if not node.has_sw_readable: + self.msg.error( + "'buffer_reads' is set to true, but this register does not contain any readable fields.", + self.get_src_ref(node) + ) + + # TODO: Should I limit the use of other properties on double-buffered registers? + + def get_unassigned_default(self, node: 'Node') -> Any: + return False + + +class RBufferTrigger(xBufferTrigger): + name = "rbuffer_trigger" + + def get_unassigned_default(self, node: 'Node') -> Any: + # If buffering is enabled, trigger is the register itself + if node.get_property('buffer_reads'): + return node + return None + + + +ALL_UDPS = [ + BufferWrites, + WBufferTrigger, + BufferReads, + RBufferTrigger, +] diff --git a/tests/lib/base_testcase.py b/tests/lib/base_testcase.py index c4c77ef..97582e8 100644 --- a/tests/lib/base_testcase.py +++ b/tests/lib/base_testcase.py @@ -10,6 +10,8 @@ import pytest from systemrdl import RDLCompiler from peakrdl_regblock import RegblockExporter +from peakrdl_regblock.udps import ALL_UDPS + from .cpuifs.base import CpuifTestMode from .cpuifs.apb4 import APB4 @@ -84,6 +86,9 @@ class BaseTestCase(unittest.TestCase): rdl_file = glob.glob(os.path.join(this_dir, "*.rdl"))[0] rdlc = RDLCompiler() + for udp in ALL_UDPS: + rdlc.register_udp(udp) + rdlc.compile_file(rdl_file) root = rdlc.elaborate(cls.rdl_elab_target, "regblock", cls.rdl_elab_params) diff --git a/tests/pylint.rc b/tests/pylint.rc index d90eaf4..beaee5c 100644 --- a/tests/pylint.rc +++ b/tests/pylint.rc @@ -98,7 +98,8 @@ disable= abstract-method, protected-access, duplicate-code, - unused-argument + unused-argument, + consider-using-f-string # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option