Omit unecessary hwif signals if an external register is read-only or …

…write-only. #58

docs/rdl_features/external.rst

@@ -44,10 +44,16 @@ hwif_out..req
     When asserted, a read or write transfer will be initiated.
     Qualifies all other request signals.
 
-    If a register is wide (``regwidth`` > ``accesswidth``), then the
+    If the register is wide (``regwidth`` > ``accesswidth``), then the
     ``hwif_out..req`` will consist of multiple bits, representing the access
     strobe for each sub-word of the register.
 
+    If the register does not contain any readable fields, this strobe will be
+    suppressed for read operations.
+
+    If the register does not contain any writable readable fields, this strobe
+    will be suppressed for write operations.
+
 hwif_out..req_is_wr
     If ``1``, denotes that the current transfer is a write. Otherwise transfer is
     a read.
@@ -59,11 +65,15 @@ hwif_out..wr_data
     The bit-width of this signal always matches the CPUIF's bus width,
     regardless of the regwidth.
 
+    If the register does not contain any writable fields, this signal is omitted.
+
 hwif_out..wr_biten
     Active-high bit-level write-enable strobes.
     Only asserted bit positions will change the register value during a write
     transfer.
 
+    If the register does not contain any writable fields, this signal is omitted.
+
 
 Read Response
 ^^^^^^^^^^^^^
@@ -74,9 +84,13 @@ hwif_in..rd_ack
     If the transfer is always completed in the same cycle, it is acceptable to
     tie this signal to ``hwif_out..req && !hwif_out..req_is_wr``.
 
+    If the register does not contain any readable fields, this signal is omitted.
+
 hwif_in..rd_data
     Read response data.
 
+    If the register does not contain any readable fields, this signal is omitted.
+
 Write Response
 ^^^^^^^^^^^^^^
 hwif_in..wr_ack
@@ -85,6 +99,8 @@ hwif_in..wr_ack
     If the transfer is always completed in the same cycle, it is acceptable to
     tie this signal to ``hwif_out..req && hwif_out..req_is_wr``.
 
+    If the register does not contain any writable fields, this signal is omitted.
+
 
 
 External Blocks

src/peakrdl_regblock/addr_decode.py

@@ -184,7 +184,16 @@ def enter_Reg(self, node: RegNode) -> None:
             s = f"{self.addr_decode.get_access_strobe(node)} = {rhs};"
             self.add_content(s)
             if node.external:
-                self.add_content(f"is_external |= {rhs};")
+                readable = node.has_sw_readable
+                writable = node.has_sw_writable
+                if readable and writable:
+                    self.add_content(f"is_external |= {rhs};")
+                elif readable and not writable:
+                    self.add_content(f"is_external |= {rhs} & !cpuif_req_is_wr;")
+                elif not readable and writable:
+                    self.add_content(f"is_external |= {rhs} & cpuif_req_is_wr;")
+                else:
+                    raise RuntimeError
         else:
             # Register is wide. Create a substrobe for each subword
             n_subwords = regwidth // accesswidth
@@ -194,7 +203,16 @@ def enter_Reg(self, node: RegNode) -> None:
                 s = f"{self.addr_decode.get_access_strobe(node)}[{i}] = {rhs};"
                 self.add_content(s)
                 if node.external:
-                    self.add_content(f"is_external |= {rhs};")
+                    readable = node.has_sw_readable
+                    writable = node.has_sw_writable
+                    if readable and writable:
+                        self.add_content(f"is_external |= {rhs};")
+                    elif readable and not writable:
+                        self.add_content(f"is_external |= {rhs} & !cpuif_req_is_wr;")
+                    elif not readable and writable:
+                        self.add_content(f"is_external |= {rhs} & cpuif_req_is_wr;")
+                    else:
+                        raise RuntimeError
 
     def exit_AddressableComponent(self, node: 'AddressableNode') -> None:
         super().exit_AddressableComponent(node)

src/peakrdl_regblock/external_acks.py

@@ -1,6 +1,7 @@
 from typing import TYPE_CHECKING
 
 from systemrdl.walker import WalkerAction
+from systemrdl.node import RegNode
 
 from .forloop_generator import RDLForLoopGenerator
 
@@ -24,7 +25,8 @@ def enter_AddressableComponent(self, node: 'AddressableNode') -> WalkerAction:
         super().enter_AddressableComponent(node)
 
         if node.external:
-            self.add_content(f"wr_ack |= {self.exp.hwif.get_external_wr_ack(node)};")
+            if not isinstance(node, RegNode) or node.has_sw_writable:
+                self.add_content(f"wr_ack |= {self.exp.hwif.get_external_wr_ack(node)};")
             return WalkerAction.SkipDescendants
 
         return WalkerAction.Continue
@@ -45,7 +47,8 @@ def enter_AddressableComponent(self, node: 'AddressableNode') -> WalkerAction:
         super().enter_AddressableComponent(node)
 
         if node.external:
-            self.add_content(f"rd_ack |= {self.exp.hwif.get_external_rd_ack(node)};")
+            if not isinstance(node, RegNode) or node.has_sw_readable:
+                self.add_content(f"rd_ack |= {self.exp.hwif.get_external_rd_ack(node)};")
             return WalkerAction.SkipDescendants
 
         return WalkerAction.Continue

src/peakrdl_regblock/field_logic/generators.py

@@ -347,6 +347,8 @@ def assign_external_reg_outputs(self, node: 'RegNode') -> None:
             bslice = ""
 
         context = {
+            "has_sw_writable": node.has_sw_writable,
+            "has_sw_readable": node.has_sw_readable,
             "prefix": prefix,
             "strb": strb,
             "bslice": bslice,

src/peakrdl_regblock/field_logic/templates/external_reg.sv

@@ -5,24 +5,42 @@ always_ff {{get_always_ff_event(resetsignal)}} begin
     if({{get_resetsignal(resetsignal)}}) begin
         {{prefix}}.req <= '0;
         {{prefix}}.req_is_wr <= '0;
+    {%- if has_sw_writable %}
         {{prefix}}.wr_data <= '0;
         {{prefix}}.wr_biten <= '0;
+    {%- endif %}
     end else begin
+    {%- if has_sw_readable and has_sw_writable %}
         {{prefix}}.req <= {{strb}};
+    {%- elif has_sw_readable and not has_sw_writable %}
+        {{prefix}}.req <= !decoded_req_is_wr ? {{strb}} : '0;
+    {%- elif not has_sw_readable and has_sw_writable %}
+        {{prefix}}.req <= decoded_req_is_wr ? {{strb}} : '0;
+    {%- endif %}
         {{prefix}}.req_is_wr <= decoded_req_is_wr;
+    {%- if has_sw_writable %}
         {{prefix}}.wr_data <= decoded_wr_data{{bslice}};
         {{prefix}}.wr_biten <= decoded_wr_biten{{bslice}};
+    {%- endif %}
     end
 end
 
 
 {%- else -%}
 
 
+{%- if has_sw_readable and has_sw_writable %}
 assign {{prefix}}.req = {{strb}};
+{%- elif has_sw_readable and not has_sw_writable %}
+assign {{prefix}}.req = !decoded_req_is_wr ? {{strb}} : '0;
+{%- elif not has_sw_readable and has_sw_writable %}
+assign {{prefix}}.req = decoded_req_is_wr ? {{strb}} : '0;
+{%- endif %}
 assign {{prefix}}.req_is_wr = decoded_req_is_wr;
+{%- if has_sw_writable %}
 assign {{prefix}}.wr_data = decoded_wr_data{{bslice}};
 assign {{prefix}}.wr_biten = decoded_wr_biten{{bslice}};
+{%- endif %}
 
 
 {%- endif %}

src/peakrdl_regblock/hwif/generators.py

@@ -97,9 +97,11 @@ def enter_Reg(self, node: 'RegNode') -> Optional[WalkerAction]:
         super().enter_Reg(node)
         if node.external:
             width = min(self.hwif.ds.cpuif_data_width, node.get_property('regwidth'))
-            self.add_member("rd_ack")
-            self.add_member("rd_data", width)
-            self.add_member("wr_ack")
+            if node.has_sw_readable:
+                self.add_member("rd_ack")
+                self.add_member("rd_data", width)
+            if node.has_sw_writable:
+                self.add_member("wr_ack")
             return WalkerAction.SkipDescendants
 
         return WalkerAction.Continue
@@ -194,8 +196,9 @@ def enter_Reg(self, node: 'RegNode') -> Optional[WalkerAction]:
             n_subwords = node.get_property("regwidth") // node.get_property("accesswidth")
             self.add_member("req", n_subwords)
             self.add_member("req_is_wr")
-            self.add_member("wr_data", width)
-            self.add_member("wr_biten", width)
+            if node.has_sw_writable:
+                self.add_member("wr_data", width)
+                self.add_member("wr_biten", width)
             return WalkerAction.SkipDescendants
 
         return WalkerAction.Continue

src/peakrdl_regblock/readback/generators.py

@@ -92,11 +92,11 @@ def enter_AddressableComponent(self, node: 'AddressableNode') -> WalkerAction:
         return WalkerAction.Continue
 
     def enter_Reg(self, node: RegNode) -> WalkerAction:
-        if node.external:
-            self.process_external_reg(node)
+        if not node.has_sw_readable:
             return WalkerAction.SkipDescendants
 
-        if not node.has_sw_readable:
+        if node.external:
+            self.process_external_reg(node)
             return WalkerAction.SkipDescendants
 
         accesswidth = node.get_property('accesswidth')

tests/run.sh

@@ -15,7 +15,7 @@ pip install -r $this_dir/requirements.txt
 
 # Install dut
 cd $this_dir/../
-python $this_dir/../setup.py install
+pip install -U .
 cd $this_dir
 
 # Run unit tests

tests/test_external/regblock.rdl

@@ -26,4 +26,26 @@ addrmap top {
         memwidth = 32;
         mementries = 8;
     } mm @ 0x3000;
+
+    reg my_ro_reg {
+        field {sw=r; hw=w;} whatever[32] = 0;
+    };
+    reg my_wo_reg {
+        field {sw=w; hw=r;} whatever[32] = 0;
+    };
+    external my_ro_reg ro_reg @ 0x4000;
+    external my_wo_reg wo_reg @ 0x4004;
+
+    reg my_wide_ro_reg {
+        regwidth = 64;
+        accesswidth = 32;
+        field {sw=r; hw=w;} whatever[32] = 0;
+    };
+    reg my_wide_wo_reg {
+        regwidth = 64;
+        accesswidth = 32;
+        field {sw=w; hw=r;} whatever[32] = 0;
+    };
+    external my_wide_ro_reg wide_ro_reg @ 0x4010;
+    external my_wide_wo_reg wide_wo_reg @ 0x4018;
 };

tests/test_external/tb_template.sv

@@ -95,6 +95,62 @@
         .rd_data(hwif_in.mm.rd_data),
         .wr_ack(hwif_in.mm.wr_ack)
     );
+
+    external_reg wo_reg_inst (
+        .clk(clk),
+        .rst(rst),
+
+        .req(hwif_out.wo_reg.req),
+        .req_is_wr(hwif_out.wo_reg.req_is_wr),
+        .wr_data(hwif_out.wo_reg.wr_data),
+        .wr_biten(hwif_out.wo_reg.wr_biten),
+        .rd_ack(),
+        .rd_data(),
+        .wr_ack(hwif_in.wo_reg.wr_ack)
+    );
+
+    external_reg ro_reg_inst (
+        .clk(clk),
+        .rst(rst),
+
+        .req(hwif_out.ro_reg.req),
+        .req_is_wr(hwif_out.ro_reg.req_is_wr),
+        .wr_data(32'b0),
+        .wr_biten(32'b0),
+        .rd_ack(hwif_in.ro_reg.rd_ack),
+        .rd_data(hwif_in.ro_reg.rd_data),
+        .wr_ack()
+    );
+
+    external_reg #(
+        .SUBWORDS(2)
+    ) wide_wo_reg_inst (
+        .clk(clk),
+        .rst(rst),
+
+        .req(hwif_out.wide_wo_reg.req),
+        .req_is_wr(hwif_out.wide_wo_reg.req_is_wr),
+        .wr_data(hwif_out.wide_wo_reg.wr_data),
+        .wr_biten(hwif_out.wide_wo_reg.wr_biten),
+        .rd_ack(),
+        .rd_data(),
+        .wr_ack(hwif_in.wide_wo_reg.wr_ack)
+    );
+
+    external_reg #(
+        .SUBWORDS(2)
+    ) wide_ro_reg_inst (
+        .clk(clk),
+        .rst(rst),
+
+        .req(hwif_out.wide_ro_reg.req),
+        .req_is_wr(hwif_out.wide_ro_reg.req_is_wr),
+        .wr_data(64'b0),
+        .wr_biten(64'b0),
+        .rd_ack(hwif_in.wide_ro_reg.rd_ack),
+        .rd_data(hwif_in.wide_ro_reg.rd_data),
+        .wr_ack()
+    );
 {%- endblock %}
 
 
@@ -161,6 +217,40 @@
         end
     end
 
+    repeat(20) begin
+        x = $urandom();
+        ro_reg_inst.value <= x;
+        cpuif.write('h4000, ~x);
+        cpuif.assert_read('h4000, x);
+        assert(ro_reg_inst.value == x);
+    end
+
+    repeat(20) begin
+        x = $urandom();
+        cpuif.write('h4004, x);
+        cpuif.assert_read('h4004, 0);
+        assert(wo_reg_inst.value == x);
+    end
+
+    for(int i=0; i<2; i++) begin
+        repeat(20) begin
+            x = $urandom();
+            wide_ro_reg_inst.value[i] <= x;
+            cpuif.write('h4010 + i*4, ~x);
+            cpuif.assert_read('h4010 + i*4, x);
+            assert(wide_ro_reg_inst.value[i] == x);
+        end
+    end
+
+    for(int i=0; i<2; i++) begin
+        repeat(20) begin
+            x = $urandom();
+            cpuif.write('h4018 + i*4, x);
+            cpuif.assert_read('h4018 + i*4, 0);
+            assert(wide_wo_reg_inst.value[i] == x);
+        end
+    end
+
     //--------------------------------------------------------------------------
     // Pipelined access
     //--------------------------------------------------------------------------