From 95fda3abaa7c1b885858505d359abb4eb65e4863 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sun, 26 Oct 2025 18:47:11 -0700 Subject: [PATCH] Refactor cpuif classes to use Interface abstraction (#14) * Initial plan * Refactor cpuif classes to use Interface abstraction Co-authored-by: arnavsacheti <36746504+arnavsacheti@users.noreply.github.com> * Fix type annotation consistency in Interface.signal() Co-authored-by: arnavsacheti <36746504+arnavsacheti@users.noreply.github.com> * Add runtime validation and documentation for indexer types Co-authored-by: arnavsacheti <36746504+arnavsacheti@users.noreply.github.com> * Remove unused variable in SVInterface.signal() Co-authored-by: arnavsacheti <36746504+arnavsacheti@users.noreply.github.com> * Fix master port directions in APB3 and APB4 flat interfaces Co-authored-by: arnavsacheti <36746504+arnavsacheti@users.noreply.github.com> * Fix AXI4LiteCpuifFlat and apply code formatting Co-authored-by: arnavsacheti <36746504+arnavsacheti@users.noreply.github.com> * PSELx -> PSEL * cleanup marker warnings --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: arnavsacheti <36746504+arnavsacheti@users.noreply.github.com> --- pyproject.toml | 10 +- .../cpuif/apb3/apb3_cpuif.py | 37 ++-- .../cpuif/apb3/apb3_cpuif_flat.py | 65 ++---- .../cpuif/apb3/apb3_interface.py | 56 ++++++ .../cpuif/apb4/apb4_cpuif.py | 37 ++-- .../cpuif/apb4/apb4_cpuif_flat.py | 69 ++----- .../cpuif/apb4/apb4_interface.py | 60 ++++++ .../cpuif/axi4lite/axi4_lite_cpuif.py | 40 ++-- .../cpuif/axi4lite/axi4_lite_cpuif_flat.py | 136 ++++++------- .../cpuif/axi4lite/axi4lite_interface.py | 84 ++++++++ src/peakrdl_busdecoder/cpuif/interface.py | 190 ++++++++++++++++++ .../cocotb/apb3/smoke/test_register_access.py | 4 +- .../cocotb/apb4/smoke/test_register_access.py | 4 +- tests/pytest.ini | 5 - 14 files changed, 539 insertions(+), 258 deletions(-) create mode 100644 src/peakrdl_busdecoder/cpuif/apb3/apb3_interface.py create mode 100644 src/peakrdl_busdecoder/cpuif/apb4/apb4_interface.py create mode 100644 src/peakrdl_busdecoder/cpuif/axi4lite/axi4lite_interface.py create mode 100644 src/peakrdl_busdecoder/cpuif/interface.py delete mode 100644 tests/pytest.ini diff --git a/pyproject.toml b/pyproject.toml index cd244f7..2620d50 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,7 +67,7 @@ tools = ["pyrefly>=0.37.0", "ruff>=0.14.0"] [project.entry-points."peakrdl.exporters"] busdecoder = "peakrdl_busdecoder.__peakrdl__:Exporter" - +# ---------------------- RUFF ---------------------- [tool.ruff] line-length = 110 target-version = "py310" @@ -106,3 +106,11 @@ untyped-def-behavior = "check-and-infer-return-type" project-includes = ["src/**/*"] project-excludes = ["**/__pycache__", "**/*venv/**/*"] + +# ---------------------- PYTEST ---------------------- +[tool.pytest.ini_options] +python_files = ["test_*.py", "*_test.py"] +markers = [ + "simulation: marks tests as requiring cocotb simulation (deselect with '-m \"not simulation\"')", + "verilator: marks tests as requiring verilator simulator (deselect with '-m \"not verilator\"')", +] diff --git a/src/peakrdl_busdecoder/cpuif/apb3/apb3_cpuif.py b/src/peakrdl_busdecoder/cpuif/apb3/apb3_cpuif.py index db99c51..845e38e 100644 --- a/src/peakrdl_busdecoder/cpuif/apb3/apb3_cpuif.py +++ b/src/peakrdl_busdecoder/cpuif/apb3/apb3_cpuif.py @@ -1,47 +1,36 @@ -from typing import overload +from typing import TYPE_CHECKING, overload from systemrdl.node import AddressableNode from ...utils import get_indexed_path from ..base_cpuif import BaseCpuif +from .apb3_interface import APB3SVInterface + +if TYPE_CHECKING: + from ...exporter import BusDecoderExporter class APB3Cpuif(BaseCpuif): template_path = "apb3_tmpl.sv" - is_interface = True - def _port_declaration(self, child: AddressableNode) -> str: - base = f"apb3_intf.master m_apb_{child.inst_name}" + def __init__(self, exp: "BusDecoderExporter") -> None: + super().__init__(exp) + self._interface = APB3SVInterface(self) - # When unrolled, current_idx is set - append it to the name - if child.current_idx is not None: - base = f"{base}_{'_'.join(map(str, child.current_idx))}" - - # Only add array dimensions if this should be treated as an array - if self.check_is_array(child): - assert child.array_dimensions is not None - return f"{base} {''.join(f'[{dim}]' for dim in child.array_dimensions)}" - - return base + @property + def is_interface(self) -> bool: + return self._interface.is_interface @property def port_declaration(self) -> str: - slave_ports: list[str] = ["apb3_intf.slave s_apb"] - master_ports: list[str] = list(map(self._port_declaration, self.addressable_children)) - - return ",\n".join(slave_ports + master_ports) + return self._interface.get_port_declaration("s_apb", "m_apb_") @overload def signal(self, signal: str, node: None = None, indexer: None = None) -> str: ... @overload def signal(self, signal: str, node: AddressableNode, indexer: str) -> str: ... def signal(self, signal: str, node: AddressableNode | None = None, indexer: str | None = None) -> str: - if node is None or indexer is None: - # Node is none, so this is a slave signal - return f"s_apb.{signal}" - - # Master signal - return f"m_apb_{get_indexed_path(node.parent, node, indexer, skip_kw_filter=True)}.{signal}" + return self._interface.signal(signal, node, indexer) def fanout(self, node: AddressableNode) -> str: fanout: dict[str, str] = {} diff --git a/src/peakrdl_busdecoder/cpuif/apb3/apb3_cpuif_flat.py b/src/peakrdl_busdecoder/cpuif/apb3/apb3_cpuif_flat.py index 576155a..2620ccf 100644 --- a/src/peakrdl_busdecoder/cpuif/apb3/apb3_cpuif_flat.py +++ b/src/peakrdl_busdecoder/cpuif/apb3/apb3_cpuif_flat.py @@ -1,46 +1,29 @@ +from typing import TYPE_CHECKING + from systemrdl.node import AddressableNode from ...utils import get_indexed_path from ..base_cpuif import BaseCpuif +from .apb3_interface import APB3FlatInterface + +if TYPE_CHECKING: + from ...exporter import BusDecoderExporter class APB3CpuifFlat(BaseCpuif): template_path = "apb3_tmpl.sv" - is_interface = False - def _port_declaration(self, child: AddressableNode) -> list[str]: - return [ - f"input logic {self.signal('PCLK', child)}", - f"input logic {self.signal('PRESETn', child)}", - f"output logic {self.signal('PSELx', child)}", - f"output logic {self.signal('PENABLE', child)}", - f"output logic {self.signal('PWRITE', child)}", - f"output logic [{self.addr_width - 1}:0] {self.signal('PADDR', child)}", - f"output logic [{self.data_width - 1}:0] {self.signal('PWDATA', child)}", - f"input logic [{self.data_width - 1}:0] {self.signal('PRDATA', child)}", - f"input logic {self.signal('PREADY', child)}", - f"input logic {self.signal('PSLVERR', child)}", - ] + def __init__(self, exp: "BusDecoderExporter") -> None: + super().__init__(exp) + self._interface = APB3FlatInterface(self) + + @property + def is_interface(self) -> bool: + return self._interface.is_interface @property def port_declaration(self) -> str: - slave_ports: list[str] = [ - f"input logic {self.signal('PCLK')}", - f"input logic {self.signal('PRESETn')}", - f"input logic {self.signal('PSELx')}", - f"input logic {self.signal('PENABLE')}", - f"input logic {self.signal('PWRITE')}", - f"input logic [{self.addr_width - 1}:0] {self.signal('PADDR')}", - f"input logic [{self.data_width - 1}:0] {self.signal('PWDATA')}", - f"output logic [{self.data_width - 1}:0] {self.signal('PRDATA')}", - f"output logic {self.signal('PREADY')}", - f"output logic {self.signal('PSLVERR')}", - ] - master_ports: list[str] = [] - for child in self.addressable_children: - master_ports.extend(self._port_declaration(child)) - - return ",\n".join(slave_ports + master_ports) + return self._interface.get_port_declaration("s_apb_", "m_apb_") def signal( self, @@ -48,27 +31,11 @@ class APB3CpuifFlat(BaseCpuif): node: AddressableNode | None = None, idx: str | int | None = None, ) -> str: - mapped_signal = "PSELx" if signal == "PSEL" else signal - if node is None: - # Node is none, so this is a slave signal - return f"s_apb_{mapped_signal}" - - # Master signal - base = f"m_apb_{node.inst_name}" - if not self.check_is_array(node): - # Not an array or an unrolled element - if node.current_idx is not None: - # This is a specific instance of an unrolled array - return f"{base}_{mapped_signal}_{'_'.join(map(str, node.current_idx))}" - return f"{base}_{mapped_signal}" - # Is an array - if idx is not None: - return f"{base}_{mapped_signal}[{idx}]" - return f"{base}_{mapped_signal}[N_{node.inst_name.upper()}S]" + return self._interface.signal(signal, node, idx) def fanout(self, node: AddressableNode) -> str: fanout: dict[str, str] = {} - fanout[self.signal("PSELx", node)] = ( + fanout[self.signal("PSEL", node)] = ( f"cpuif_wr_sel.{get_indexed_path(self.exp.ds.top_node, node, 'i')}|cpuif_rd_sel.{get_indexed_path(self.exp.ds.top_node, node, 'i')}" ) fanout[self.signal("PENABLE", node)] = self.signal("PENABLE") diff --git a/src/peakrdl_busdecoder/cpuif/apb3/apb3_interface.py b/src/peakrdl_busdecoder/cpuif/apb3/apb3_interface.py new file mode 100644 index 0000000..04badae --- /dev/null +++ b/src/peakrdl_busdecoder/cpuif/apb3/apb3_interface.py @@ -0,0 +1,56 @@ +"""APB3-specific interface implementations.""" + +from systemrdl.node import AddressableNode + +from ..interface import FlatInterface, SVInterface + + +class APB3SVInterface(SVInterface): + """APB3 SystemVerilog interface.""" + + def get_interface_type(self) -> str: + return "apb3_intf" + + def get_slave_name(self) -> str: + return "s_apb" + + def get_master_prefix(self) -> str: + return "m_apb_" + + +class APB3FlatInterface(FlatInterface): + """APB3 flat signal interface.""" + + def get_slave_prefix(self) -> str: + return "s_apb_" + + def get_master_prefix(self) -> str: + return "m_apb_" + + def _get_slave_port_declarations(self, slave_prefix: str) -> list[str]: + return [ + f"input logic {slave_prefix}PCLK", + f"input logic {slave_prefix}PRESETn", + f"input logic {slave_prefix}PSEL", + f"input logic {slave_prefix}PENABLE", + f"input logic {slave_prefix}PWRITE", + f"input logic [{self.cpuif.addr_width - 1}:0] {slave_prefix}PADDR", + f"input logic [{self.cpuif.data_width - 1}:0] {slave_prefix}PWDATA", + f"output logic [{self.cpuif.data_width - 1}:0] {slave_prefix}PRDATA", + f"output logic {slave_prefix}PREADY", + f"output logic {slave_prefix}PSLVERR", + ] + + def _get_master_port_declarations(self, child: AddressableNode, master_prefix: str) -> list[str]: + return [ + f"output logic {self.signal('PCLK', child)}", + f"output logic {self.signal('PRESETn', child)}", + f"output logic {self.signal('PSEL', child)}", + f"output logic {self.signal('PENABLE', child)}", + f"output logic {self.signal('PWRITE', child)}", + f"output logic [{self.cpuif.addr_width - 1}:0] {self.signal('PADDR', child)}", + f"output logic [{self.cpuif.data_width - 1}:0] {self.signal('PWDATA', child)}", + f"input logic [{self.cpuif.data_width - 1}:0] {self.signal('PRDATA', child)}", + f"input logic {self.signal('PREADY', child)}", + f"input logic {self.signal('PSLVERR', child)}", + ] diff --git a/src/peakrdl_busdecoder/cpuif/apb4/apb4_cpuif.py b/src/peakrdl_busdecoder/cpuif/apb4/apb4_cpuif.py index a2ea70a..d80bb13 100644 --- a/src/peakrdl_busdecoder/cpuif/apb4/apb4_cpuif.py +++ b/src/peakrdl_busdecoder/cpuif/apb4/apb4_cpuif.py @@ -1,48 +1,37 @@ -from typing import overload +from typing import TYPE_CHECKING, overload from systemrdl.node import AddressableNode from ...utils import get_indexed_path from ..base_cpuif import BaseCpuif +from .apb4_interface import APB4SVInterface + +if TYPE_CHECKING: + from ...exporter import BusDecoderExporter class APB4Cpuif(BaseCpuif): template_path = "apb4_tmpl.sv" - is_interface = True - def _port_declaration(self, child: AddressableNode) -> str: - base = f"apb4_intf.master m_apb_{child.inst_name}" + def __init__(self, exp: "BusDecoderExporter") -> None: + super().__init__(exp) + self._interface = APB4SVInterface(self) - # When unrolled, current_idx is set - append it to the name - if child.current_idx is not None: - base = f"{base}_{'_'.join(map(str, child.current_idx))}" - - # Only add array dimensions if this should be treated as an array - if self.check_is_array(child): - assert child.array_dimensions is not None - return f"{base} {''.join(f'[{dim}]' for dim in child.array_dimensions)}" - - return base + @property + def is_interface(self) -> bool: + return self._interface.is_interface @property def port_declaration(self) -> str: """Returns the port declaration for the APB4 interface.""" - slave_ports: list[str] = ["apb4_intf.slave s_apb"] - master_ports: list[str] = list(map(self._port_declaration, self.addressable_children)) - - return ",\n".join(slave_ports + master_ports) + return self._interface.get_port_declaration("s_apb", "m_apb_") @overload def signal(self, signal: str, node: None = None, indexer: None = None) -> str: ... @overload def signal(self, signal: str, node: AddressableNode, indexer: str) -> str: ... def signal(self, signal: str, node: AddressableNode | None = None, indexer: str | None = None) -> str: - if node is None or indexer is None: - # Node is none, so this is a slave signal - return f"s_apb.{signal}" - - # Master signal - return f"m_apb_{get_indexed_path(node.parent, node, indexer, skip_kw_filter=True)}.{signal}" + return self._interface.signal(signal, node, indexer) def fanout(self, node: AddressableNode) -> str: fanout: dict[str, str] = {} diff --git a/src/peakrdl_busdecoder/cpuif/apb4/apb4_cpuif_flat.py b/src/peakrdl_busdecoder/cpuif/apb4/apb4_cpuif_flat.py index 8c6ec3c..357c46b 100644 --- a/src/peakrdl_busdecoder/cpuif/apb4/apb4_cpuif_flat.py +++ b/src/peakrdl_busdecoder/cpuif/apb4/apb4_cpuif_flat.py @@ -1,50 +1,29 @@ +from typing import TYPE_CHECKING + from systemrdl.node import AddressableNode from ...utils import get_indexed_path from ..base_cpuif import BaseCpuif +from .apb4_interface import APB4FlatInterface + +if TYPE_CHECKING: + from ...exporter import BusDecoderExporter class APB4CpuifFlat(BaseCpuif): template_path = "apb4_tmpl.sv" - is_interface = False - def _port_declaration(self, child: AddressableNode) -> list[str]: - return [ - f"input logic {self.signal('PCLK', child)}", - f"input logic {self.signal('PRESETn', child)}", - f"output logic {self.signal('PSELx', child)}", - f"output logic {self.signal('PENABLE', child)}", - f"output logic {self.signal('PWRITE', child)}", - f"output logic [{self.addr_width - 1}:0] {self.signal('PADDR', child)}", - f"output logic [2:0] {self.signal('PPROT', child)}", - f"output logic [{self.data_width - 1}:0] {self.signal('PWDATA', child)}", - f"output logic [{self.data_width // 8 - 1}:0] {self.signal('PSTRB', child)}", - f"input logic [{self.data_width - 1}:0] {self.signal('PRDATA', child)}", - f"input logic {self.signal('PREADY', child)}", - f"input logic {self.signal('PSLVERR', child)}", - ] + def __init__(self, exp: "BusDecoderExporter") -> None: + super().__init__(exp) + self._interface = APB4FlatInterface(self) + + @property + def is_interface(self) -> bool: + return self._interface.is_interface @property def port_declaration(self) -> str: - slave_ports: list[str] = [ - f"input logic {self.signal('PCLK')}", - f"input logic {self.signal('PRESETn')}", - f"input logic {self.signal('PSELx')}", - f"input logic {self.signal('PENABLE')}", - f"input logic {self.signal('PWRITE')}", - f"input logic [{self.addr_width - 1}:0] {self.signal('PADDR')}", - f"input logic [2:0] {self.signal('PPROT')}", - f"input logic [{self.data_width - 1}:0] {self.signal('PWDATA')}", - f"input logic [{self.data_width // 8 - 1}:0] {self.signal('PSTRB')}", - f"output logic [{self.data_width - 1}:0] {self.signal('PRDATA')}", - f"output logic {self.signal('PREADY')}", - f"output logic {self.signal('PSLVERR')}", - ] - master_ports: list[str] = [] - for child in self.addressable_children: - master_ports.extend(self._port_declaration(child)) - - return ",\n".join(slave_ports + master_ports) + return self._interface.get_port_declaration("s_apb_", "m_apb_") def signal( self, @@ -52,27 +31,11 @@ class APB4CpuifFlat(BaseCpuif): node: AddressableNode | None = None, idx: str | int | None = None, ) -> str: - mapped_signal = "PSELx" if signal == "PSEL" else signal - if node is None: - # Node is none, so this is a slave signal - return f"s_apb_{mapped_signal}" - - # Master signal - base = f"m_apb_{node.inst_name}" - if not self.check_is_array(node): - # Not an array or an unrolled element - if node.current_idx is not None: - # This is a specific instance of an unrolled array - return f"{base}_{mapped_signal}_{'_'.join(map(str, node.current_idx))}" - return f"{base}_{mapped_signal}" - # Is an array - if idx is not None: - return f"{base}_{mapped_signal}[{idx}]" - return f"{base}_{mapped_signal}[N_{node.inst_name.upper()}S]" + return self._interface.signal(signal, node, idx) def fanout(self, node: AddressableNode) -> str: fanout: dict[str, str] = {} - fanout[self.signal("PSELx", node)] = ( + fanout[self.signal("PSEL", node)] = ( f"cpuif_wr_sel.{get_indexed_path(self.exp.ds.top_node, node, 'i')}|cpuif_rd_sel.{get_indexed_path(self.exp.ds.top_node, node, 'i')}" ) fanout[self.signal("PENABLE", node)] = self.signal("PENABLE") diff --git a/src/peakrdl_busdecoder/cpuif/apb4/apb4_interface.py b/src/peakrdl_busdecoder/cpuif/apb4/apb4_interface.py new file mode 100644 index 0000000..be87240 --- /dev/null +++ b/src/peakrdl_busdecoder/cpuif/apb4/apb4_interface.py @@ -0,0 +1,60 @@ +"""APB4-specific interface implementations.""" + +from systemrdl.node import AddressableNode + +from ..interface import FlatInterface, SVInterface + + +class APB4SVInterface(SVInterface): + """APB4 SystemVerilog interface.""" + + def get_interface_type(self) -> str: + return "apb4_intf" + + def get_slave_name(self) -> str: + return "s_apb" + + def get_master_prefix(self) -> str: + return "m_apb_" + + +class APB4FlatInterface(FlatInterface): + """APB4 flat signal interface.""" + + def get_slave_prefix(self) -> str: + return "s_apb_" + + def get_master_prefix(self) -> str: + return "m_apb_" + + def _get_slave_port_declarations(self, slave_prefix: str) -> list[str]: + return [ + f"input logic {slave_prefix}PCLK", + f"input logic {slave_prefix}PRESETn", + f"input logic {slave_prefix}PSEL", + f"input logic {slave_prefix}PENABLE", + f"input logic {slave_prefix}PWRITE", + f"input logic [{self.cpuif.addr_width - 1}:0] {slave_prefix}PADDR", + f"input logic [2:0] {slave_prefix}PPROT", + f"input logic [{self.cpuif.data_width - 1}:0] {slave_prefix}PWDATA", + f"input logic [{self.cpuif.data_width // 8 - 1}:0] {slave_prefix}PSTRB", + f"output logic [{self.cpuif.data_width - 1}:0] {slave_prefix}PRDATA", + f"output logic {slave_prefix}PREADY", + f"output logic {slave_prefix}PSLVERR", + ] + + def _get_master_port_declarations(self, child: AddressableNode, master_prefix: str) -> list[str]: + return [ + f"output logic {self.signal('PCLK', child)}", + f"output logic {self.signal('PRESETn', child)}", + f"output logic {self.signal('PSEL', child)}", + f"output logic {self.signal('PENABLE', child)}", + f"output logic {self.signal('PWRITE', child)}", + f"output logic [{self.cpuif.addr_width - 1}:0] {self.signal('PADDR', child)}", + f"output logic [2:0] {self.signal('PPROT', child)}", + f"output logic [{self.cpuif.data_width - 1}:0] {self.signal('PWDATA', child)}", + f"output logic [{self.cpuif.data_width // 8 - 1}:0] {self.signal('PSTRB', child)}", + f"input logic [{self.cpuif.data_width - 1}:0] {self.signal('PRDATA', child)}", + f"input logic {self.signal('PREADY', child)}", + f"input logic {self.signal('PSLVERR', child)}", + ] diff --git a/src/peakrdl_busdecoder/cpuif/axi4lite/axi4_lite_cpuif.py b/src/peakrdl_busdecoder/cpuif/axi4lite/axi4_lite_cpuif.py index 1745235..fb2a50d 100644 --- a/src/peakrdl_busdecoder/cpuif/axi4lite/axi4_lite_cpuif.py +++ b/src/peakrdl_busdecoder/cpuif/axi4lite/axi4_lite_cpuif.py @@ -1,51 +1,37 @@ -from typing import overload +from typing import TYPE_CHECKING, overload from systemrdl.node import AddressableNode from ...utils import get_indexed_path from ..base_cpuif import BaseCpuif +from .axi4lite_interface import AXI4LiteSVInterface + +if TYPE_CHECKING: + from ...exporter import BusDecoderExporter class AXI4LiteCpuif(BaseCpuif): template_path = "axi4lite_tmpl.sv" - is_interface = True - def _port_declaration(self, child: AddressableNode) -> list[str]: - base = f"axi4lite_intf.master m_axil_{child.inst_name}" + def __init__(self, exp: "BusDecoderExporter") -> None: + super().__init__(exp) + self._interface = AXI4LiteSVInterface(self) - # When unrolled, current_idx is set - append it to the name - if child.current_idx is not None: - base = f"{base}_{'_'.join(map(str, child.current_idx))}" - - # Only add array dimensions if this should be treated as an array - if self.check_is_array(child): - assert child.array_dimensions is not None - return [f"{base} {''.join(f'[{dim}]' for dim in child.array_dimensions)}"] - - return [base] + @property + def is_interface(self) -> bool: + return self._interface.is_interface @property def port_declaration(self) -> str: """Returns the port declaration for the AXI4-Lite interface.""" - slave_ports: list[str] = ["axi4lite_intf.slave s_axil"] - - master_ports: list[str] = [] - for child in self.addressable_children: - master_ports.extend(self._port_declaration(child)) - - return ",\n".join(slave_ports + master_ports) + return self._interface.get_port_declaration("s_axil", "m_axil_") @overload def signal(self, signal: str, node: None = None, indexer: None = None) -> str: ... @overload def signal(self, signal: str, node: AddressableNode, indexer: str | None = None) -> str: ... def signal(self, signal: str, node: AddressableNode | None = None, indexer: str | None = None) -> str: - if node is None or indexer is None: - # Node is none, so this is a slave signal - return f"s_axil.{signal}" - - # Master signal - return f"m_axil_{get_indexed_path(node.parent, node, indexer, skip_kw_filter=True)}.{signal}" + return self._interface.signal(signal, node, indexer) def fanout(self, node: AddressableNode) -> str: fanout: dict[str, str] = {} diff --git a/src/peakrdl_busdecoder/cpuif/axi4lite/axi4_lite_cpuif_flat.py b/src/peakrdl_busdecoder/cpuif/axi4lite/axi4_lite_cpuif_flat.py index a8e83f1..df6ab6d 100644 --- a/src/peakrdl_busdecoder/cpuif/axi4lite/axi4_lite_cpuif_flat.py +++ b/src/peakrdl_busdecoder/cpuif/axi4lite/axi4_lite_cpuif_flat.py @@ -1,92 +1,86 @@ -from typing import overload +from typing import TYPE_CHECKING, overload from systemrdl.node import AddressableNode -from .axi4_lite_cpuif import AXI4LiteCpuif +from ...utils import get_indexed_path +from ..base_cpuif import BaseCpuif +from .axi4lite_interface import AXI4LiteFlatInterface + +if TYPE_CHECKING: + from ...exporter import BusDecoderExporter -class AXI4LiteCpuifFlat(AXI4LiteCpuif): +class AXI4LiteCpuifFlat(BaseCpuif): """Verilator-friendly variant that flattens the AXI4-Lite interface ports.""" template_path = "axi4lite_tmpl.sv" - is_interface = False - def _port_declaration(self, child: AddressableNode) -> list[str]: - return [ - f"input logic {self.signal('AWREADY', child)}", - f"output logic {self.signal('AWVALID', child)}", - f"output logic [{self.addr_width - 1}:0] {self.signal('AWADDR', child)}", - f"output logic [2:0] {self.signal('AWPROT', child)}", - f"input logic {self.signal('WREADY', child)}", - f"output logic {self.signal('WVALID', child)}", - f"output logic [{self.data_width - 1}:0] {self.signal('WDATA', child)}", - f"output logic [{self.data_width_bytes - 1}:0] {self.signal('WSTRB', child)}", - f"output logic {self.signal('BREADY', child)}", - f"input logic {self.signal('BVALID', child)}", - f"input logic [1:0] {self.signal('BRESP', child)}", - f"input logic {self.signal('ARREADY', child)}", - f"output logic {self.signal('ARVALID', child)}", - f"output logic [{self.addr_width - 1}:0] {self.signal('ARADDR', child)}", - f"output logic [2:0] {self.signal('ARPROT', child)}", - f"output logic {self.signal('RREADY', child)}", - f"input logic {self.signal('RVALID', child)}", - f"input logic [{self.data_width - 1}:0] {self.signal('RDATA', child)}", - f"input logic [1:0] {self.signal('RRESP', child)}", - ] + def __init__(self, exp: "BusDecoderExporter") -> None: + super().__init__(exp) + self._interface = AXI4LiteFlatInterface(self) + + @property + def is_interface(self) -> bool: + return self._interface.is_interface @property def port_declaration(self) -> str: - slave_ports: list[str] = [ - f"input logic {self.signal('ACLK')}", - f"input logic {self.signal('ARESETn')}", - f"output logic {self.signal('AWREADY')}", - f"input logic {self.signal('AWVALID')}", - f"input logic [{self.addr_width - 1}:0] {self.signal('AWADDR')}", - f"input logic [2:0] {self.signal('AWPROT')}", - f"output logic {self.signal('WREADY')}", - f"input logic {self.signal('WVALID')}", - f"input logic [{self.data_width - 1}:0] {self.signal('WDATA')}", - f"input logic [{self.data_width_bytes - 1}:0] {self.signal('WSTRB')}", - f"input logic {self.signal('BREADY')}", - f"output logic {self.signal('BVALID')}", - f"output logic [1:0] {self.signal('BRESP')}", - f"output logic {self.signal('ARREADY')}", - f"input logic {self.signal('ARVALID')}", - f"input logic [{self.addr_width - 1}:0] {self.signal('ARADDR')}", - f"input logic [2:0] {self.signal('ARPROT')}", - f"input logic {self.signal('RREADY')}", - f"output logic {self.signal('RVALID')}", - f"output logic [{self.data_width - 1}:0] {self.signal('RDATA')}", - f"output logic [1:0] {self.signal('RRESP')}", - ] - - master_ports: list[str] = [] - for child in self.addressable_children: - master_ports.extend(self._port_declaration(child)) - - return ",\n".join(slave_ports + master_ports) + """Returns the port declaration for the AXI4-Lite interface.""" + return self._interface.get_port_declaration("s_axil_", "m_axil_") @overload def signal(self, signal: str, node: None = None, indexer: None = None) -> str: ... - @overload def signal(self, signal: str, node: AddressableNode, indexer: str | None = None) -> str: ... + def signal(self, signal: str, node: AddressableNode | None = None, indexer: str | None = None) -> str: + return self._interface.signal(signal, node, indexer) - def signal( - self, - signal: str, - node: AddressableNode | None = None, - indexer: str | None = None, - ) -> str: + def fanout(self, node: AddressableNode) -> str: + fanout: dict[str, str] = {} + + wr_sel = f"cpuif_wr_sel.{get_indexed_path(self.exp.ds.top_node, node, 'gi')}" + rd_sel = f"cpuif_rd_sel.{get_indexed_path(self.exp.ds.top_node, node, 'gi')}" + + # Write address channel + fanout[self.signal("AWVALID", node, "gi")] = wr_sel + fanout[self.signal("AWADDR", node, "gi")] = self.signal("AWADDR") + fanout[self.signal("AWPROT", node, "gi")] = self.signal("AWPROT") + + # Write data channel + fanout[self.signal("WVALID", node, "gi")] = wr_sel + fanout[self.signal("WDATA", node, "gi")] = "cpuif_wr_data" + fanout[self.signal("WSTRB", node, "gi")] = "cpuif_wr_byte_en" + + # Write response channel (master -> slave) + fanout[self.signal("BREADY", node, "gi")] = self.signal("BREADY") + + # Read address channel + fanout[self.signal("ARVALID", node, "gi")] = rd_sel + fanout[self.signal("ARADDR", node, "gi")] = self.signal("ARADDR") + fanout[self.signal("ARPROT", node, "gi")] = self.signal("ARPROT") + + # Read data channel (master -> slave) + fanout[self.signal("RREADY", node, "gi")] = self.signal("RREADY") + + return "\n".join(f"assign {lhs} = {rhs};" for lhs, rhs in fanout.items()) + + def fanin(self, node: AddressableNode | None = None) -> str: + fanin: dict[str, str] = {} if node is None: - return f"s_axil_{signal}" + fanin["cpuif_rd_ack"] = "'0" + fanin["cpuif_rd_err"] = "'0" + else: + # Read side: ack comes from RVALID; err if RRESP[1] is set (SLVERR/DECERR) + fanin["cpuif_rd_ack"] = self.signal("RVALID", node, "i") + fanin["cpuif_rd_err"] = f"{self.signal('RRESP', node, 'i')}[1]" - base = f"m_axil_{node.inst_name}_{signal}" - if not self.check_is_array(node): - if node.current_idx is not None: - return f"{base}_{'_'.join(map(str, node.current_idx))}" - return base + return "\n".join(f"{lhs} = {rhs};" for lhs, rhs in fanin.items()) - if indexer is None: - return f"{base}[N_{node.inst_name.upper()}S]" - return f"{base}[{indexer}]" + def readback(self, node: AddressableNode | None = None) -> str: + fanin: dict[str, str] = {} + if node is None: + fanin["cpuif_rd_data"] = "'0" + else: + fanin["cpuif_rd_data"] = self.signal("RDATA", node, "i") + + return "\n".join(f"{lhs} = {rhs};" for lhs, rhs in fanin.items()) diff --git a/src/peakrdl_busdecoder/cpuif/axi4lite/axi4lite_interface.py b/src/peakrdl_busdecoder/cpuif/axi4lite/axi4lite_interface.py new file mode 100644 index 0000000..70a65bb --- /dev/null +++ b/src/peakrdl_busdecoder/cpuif/axi4lite/axi4lite_interface.py @@ -0,0 +1,84 @@ +"""AXI4-Lite-specific interface implementations.""" + +from systemrdl.node import AddressableNode + +from ..interface import FlatInterface, SVInterface + + +class AXI4LiteSVInterface(SVInterface): + """AXI4-Lite SystemVerilog interface.""" + + def get_interface_type(self) -> str: + return "axi4lite_intf" + + def get_slave_name(self) -> str: + return "s_axil" + + def get_master_prefix(self) -> str: + return "m_axil_" + + +class AXI4LiteFlatInterface(FlatInterface): + """AXI4-Lite flat signal interface.""" + + def get_slave_prefix(self) -> str: + return "s_axil_" + + def get_master_prefix(self) -> str: + return "m_axil_" + + def _get_slave_port_declarations(self, slave_prefix: str) -> list[str]: + return [ + # Write address channel + f"input logic {slave_prefix}AWVALID", + f"output logic {slave_prefix}AWREADY", + f"input logic [{self.cpuif.addr_width - 1}:0] {slave_prefix}AWADDR", + f"input logic [2:0] {slave_prefix}AWPROT", + # Write data channel + f"input logic {slave_prefix}WVALID", + f"output logic {slave_prefix}WREADY", + f"input logic [{self.cpuif.data_width - 1}:0] {slave_prefix}WDATA", + f"input logic [{self.cpuif.data_width // 8 - 1}:0] {slave_prefix}WSTRB", + # Write response channel + f"output logic {slave_prefix}BVALID", + f"input logic {slave_prefix}BREADY", + f"output logic [1:0] {slave_prefix}BRESP", + # Read address channel + f"input logic {slave_prefix}ARVALID", + f"output logic {slave_prefix}ARREADY", + f"input logic [{self.cpuif.addr_width - 1}:0] {slave_prefix}ARADDR", + f"input logic [2:0] {slave_prefix}ARPROT", + # Read data channel + f"output logic {slave_prefix}RVALID", + f"input logic {slave_prefix}RREADY", + f"output logic [{self.cpuif.data_width - 1}:0] {slave_prefix}RDATA", + f"output logic [1:0] {slave_prefix}RRESP", + ] + + def _get_master_port_declarations(self, child: AddressableNode, master_prefix: str) -> list[str]: + return [ + # Write address channel + f"output logic {self.signal('AWVALID', child)}", + f"input logic {self.signal('AWREADY', child)}", + f"output logic [{self.cpuif.addr_width - 1}:0] {self.signal('AWADDR', child)}", + f"output logic [2:0] {self.signal('AWPROT', child)}", + # Write data channel + f"output logic {self.signal('WVALID', child)}", + f"input logic {self.signal('WREADY', child)}", + f"output logic [{self.cpuif.data_width - 1}:0] {self.signal('WDATA', child)}", + f"output logic [{self.cpuif.data_width // 8 - 1}:0] {self.signal('WSTRB', child)}", + # Write response channel + f"input logic {self.signal('BVALID', child)}", + f"output logic {self.signal('BREADY', child)}", + f"input logic [1:0] {self.signal('BRESP', child)}", + # Read address channel + f"output logic {self.signal('ARVALID', child)}", + f"input logic {self.signal('ARREADY', child)}", + f"output logic [{self.cpuif.addr_width - 1}:0] {self.signal('ARADDR', child)}", + f"output logic [2:0] {self.signal('ARPROT', child)}", + # Read data channel + f"input logic {self.signal('RVALID', child)}", + f"output logic {self.signal('RREADY', child)}", + f"input logic [{self.cpuif.data_width - 1}:0] {self.signal('RDATA', child)}", + f"input logic [1:0] {self.signal('RRESP', child)}", + ] diff --git a/src/peakrdl_busdecoder/cpuif/interface.py b/src/peakrdl_busdecoder/cpuif/interface.py new file mode 100644 index 0000000..8971aa8 --- /dev/null +++ b/src/peakrdl_busdecoder/cpuif/interface.py @@ -0,0 +1,190 @@ +"""Interface abstraction for handling flat and non-flat signal declarations.""" + +from abc import ABC, abstractmethod +from typing import TYPE_CHECKING + +from systemrdl.node import AddressableNode + +if TYPE_CHECKING: + from .base_cpuif import BaseCpuif + + +class Interface(ABC): + """Abstract base class for interface signal handling.""" + + def __init__(self, cpuif: "BaseCpuif") -> None: + self.cpuif = cpuif + + @property + @abstractmethod + def is_interface(self) -> bool: + """Whether this uses SystemVerilog interfaces.""" + ... + + @abstractmethod + def get_port_declaration(self, slave_name: str, master_prefix: str) -> str: + """ + Generate port declarations for the interface. + + Args: + slave_name: Name of the slave interface/signal prefix + master_prefix: Prefix for master interfaces/signals + + Returns: + Port declarations as a string + """ + ... + + @abstractmethod + def signal( + self, + signal: str, + node: AddressableNode | None = None, + indexer: str | int | None = None, + ) -> str: + """ + Generate signal reference. + + Args: + signal: Signal name + node: Optional addressable node for master signals + indexer: Optional indexer for arrays. + For SVInterface: str like "i" or "gi" for loop indices + For FlatInterface: str or int for array subscript + + Returns: + Signal reference as a string + """ + ... + + +class SVInterface(Interface): + """SystemVerilog interface-based signal handling.""" + + @property + def is_interface(self) -> bool: + return True + + def get_port_declaration(self, slave_name: str, master_prefix: str) -> str: + """Generate SystemVerilog interface port declarations.""" + slave_ports: list[str] = [f"{self.get_interface_type()}.slave {slave_name}"] + master_ports: list[str] = [] + + for child in self.cpuif.addressable_children: + base = f"{self.get_interface_type()}.master {master_prefix}{child.inst_name}" + + # When unrolled, current_idx is set - append it to the name + if child.current_idx is not None: + base = f"{base}_{'_'.join(map(str, child.current_idx))}" + + # Only add array dimensions if this should be treated as an array + if self.cpuif.check_is_array(child): + assert child.array_dimensions is not None + base = f"{base} {''.join(f'[{dim}]' for dim in child.array_dimensions)}" + + master_ports.append(base) + + return ",\n".join(slave_ports + master_ports) + + def signal( + self, + signal: str, + node: AddressableNode | None = None, + indexer: str | int | None = None, + ) -> str: + """Generate SystemVerilog interface signal reference.""" + from ..utils import get_indexed_path + + # SVInterface only supports string indexers (loop variable names like "i", "gi") + if indexer is not None and not isinstance(indexer, str): + raise TypeError(f"SVInterface.signal() requires string indexer, got {type(indexer).__name__}") + + if node is None or indexer is None: + # Node is none, so this is a slave signal + slave_name = self.get_slave_name() + return f"{slave_name}.{signal}" + + # Master signal + master_prefix = self.get_master_prefix() + return f"{master_prefix}{get_indexed_path(node.parent, node, indexer, skip_kw_filter=True)}.{signal}" + + @abstractmethod + def get_interface_type(self) -> str: + """Get the SystemVerilog interface type name.""" + ... + + @abstractmethod + def get_slave_name(self) -> str: + """Get the slave interface instance name.""" + ... + + @abstractmethod + def get_master_prefix(self) -> str: + """Get the master interface name prefix.""" + ... + + +class FlatInterface(Interface): + """Flat signal-based interface handling.""" + + @property + def is_interface(self) -> bool: + return False + + def get_port_declaration(self, slave_name: str, master_prefix: str) -> str: + """Generate flat port declarations.""" + slave_ports = self._get_slave_port_declarations(slave_name) + master_ports: list[str] = [] + + for child in self.cpuif.addressable_children: + master_ports.extend(self._get_master_port_declarations(child, master_prefix)) + + return ",\n".join(slave_ports + master_ports) + + def signal( + self, + signal: str, + node: AddressableNode | None = None, + indexer: str | int | None = None, + ) -> str: + """Generate flat signal reference.""" + if node is None: + # Node is none, so this is a slave signal + slave_prefix = self.get_slave_prefix() + return f"{slave_prefix}{signal}" + + # Master signal + master_prefix = self.get_master_prefix() + base = f"{master_prefix}{node.inst_name}" + + if not self.cpuif.check_is_array(node): + # Not an array or an unrolled element + if node.current_idx is not None: + # This is a specific instance of an unrolled array + return f"{base}_{signal}_{'_'.join(map(str, node.current_idx))}" + return f"{base}_{signal}" + + # Is an array + if indexer is not None: + return f"{base}_{signal}[{indexer}]" + return f"{base}_{signal}[N_{node.inst_name.upper()}S]" + + @abstractmethod + def _get_slave_port_declarations(self, slave_prefix: str) -> list[str]: + """Get slave port declarations.""" + ... + + @abstractmethod + def _get_master_port_declarations(self, child: AddressableNode, master_prefix: str) -> list[str]: + """Get master port declarations for a child node.""" + ... + + @abstractmethod + def get_slave_prefix(self) -> str: + """Get the slave signal name prefix.""" + ... + + @abstractmethod + def get_master_prefix(self) -> str: + """Get the master signal name prefix.""" + ... diff --git a/tests/cocotb/apb3/smoke/test_register_access.py b/tests/cocotb/apb3/smoke/test_register_access.py index e64bf94..be4873f 100644 --- a/tests/cocotb/apb3/smoke/test_register_access.py +++ b/tests/cocotb/apb3/smoke/test_register_access.py @@ -12,7 +12,7 @@ READ_DATA = 0x0BAD_F00D class _Apb3SlaveShim: def __init__(self, dut): prefix = "s_apb" - self.PSEL = getattr(dut, f"{prefix}_PSELx") + self.PSEL = getattr(dut, f"{prefix}_PSEL") self.PENABLE = getattr(dut, f"{prefix}_PENABLE") self.PWRITE = getattr(dut, f"{prefix}_PWRITE") self.PADDR = getattr(dut, f"{prefix}_PADDR") @@ -24,7 +24,7 @@ class _Apb3SlaveShim: class _Apb3MasterShim: def __init__(self, dut, base: str): - self.PSEL = getattr(dut, f"{base}_PSELx") + self.PSEL = getattr(dut, f"{base}_PSEL") self.PENABLE = getattr(dut, f"{base}_PENABLE") self.PWRITE = getattr(dut, f"{base}_PWRITE") self.PADDR = getattr(dut, f"{base}_PADDR") diff --git a/tests/cocotb/apb4/smoke/test_register_access.py b/tests/cocotb/apb4/smoke/test_register_access.py index abb2a1e..d27078b 100644 --- a/tests/cocotb/apb4/smoke/test_register_access.py +++ b/tests/cocotb/apb4/smoke/test_register_access.py @@ -12,7 +12,7 @@ READ_DATA = 0x89AB_CDEF class _Apb4SlaveShim: def __init__(self, dut): prefix = "s_apb" - self.PSEL = getattr(dut, f"{prefix}_PSELx") + self.PSEL = getattr(dut, f"{prefix}_PSEL") self.PENABLE = getattr(dut, f"{prefix}_PENABLE") self.PWRITE = getattr(dut, f"{prefix}_PWRITE") self.PADDR = getattr(dut, f"{prefix}_PADDR") @@ -26,7 +26,7 @@ class _Apb4SlaveShim: class _Apb4MasterShim: def __init__(self, dut, base: str): - self.PSEL = getattr(dut, f"{base}_PSELx") + self.PSEL = getattr(dut, f"{base}_PSEL") self.PENABLE = getattr(dut, f"{base}_PENABLE") self.PWRITE = getattr(dut, f"{base}_PWRITE") self.PADDR = getattr(dut, f"{base}_PADDR") diff --git a/tests/pytest.ini b/tests/pytest.ini deleted file mode 100644 index d8c8096..0000000 --- a/tests/pytest.ini +++ /dev/null @@ -1,5 +0,0 @@ -[pytest] -python_files = test_*.py testcase.py -markers = - simulation: marks hardware simulation tests requiring cocotb/verilator - verilator: marks tests that invoke the Verilator toolchain