From 223d2f98c63db9bd073741cc1c69c3b82ec7ae5f Mon Sep 17 00:00:00 2001
From: Peter McGoron <code@mcgoron.com>
Date: Thu, 7 Mar 2024 15:59:08 +0000
Subject: [PATCH] add pre_finalize() to PreemptiveInterface

CSR scans run *before* do_finalize() is called. This is a silent
failure: The signals get generated but the CSR is not in the csr.json
file or the generated verilog.

pre_finalize() is a function that must be called when all masters are
added to the module. This will generate the CSR before finalization,
(hopefully) ensuring that the CSR is added to the CSR bus.

I need to figure out a way to excise manual use of the CSR bus from the
design. Future options may include having a PreemptiveInterface
control module sitting in front of the PreemptiveInterfaces that exposes
the control lines as a Wishbone register.
---
 gateware/soc.py  | 17 ++++++++++++++---
 gateware/swic.py | 32 +++++++++++++++++++++++++++-----
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/gateware/soc.py b/gateware/soc.py
index 040739d..2b50a9f 100644
--- a/gateware/soc.py
+++ b/gateware/soc.py
@@ -174,7 +174,7 @@ class UpsilonSoC(SoCCore):
         :param slave_registers: Register inside the bus region.
         :return: The PI module.
         """
-        pi = PreemptiveInterface(slave_bus, addressing=addressing)
+        pi = PreemptiveInterface(slave_bus, addressing=addressing, name=name)
         self.add_module(name, pi)
         self.add_slave_with_registers(name, pi.add_master(),
                 SoCRegion(origin=None, size=slave_width, cached=False),
@@ -358,6 +358,10 @@ class UpsilonSoC(SoCCore):
 
         # Add waveform generator.
         self.add_waveform("wf0")
+        self.pico0.mmap.add_region("wf0",
+                BasicRegion(origin=0x400000, size=self.wf0.width,
+                    bus=self.wf0_PI.add_master(),
+                    registers=self.wf0.public_registers))
 
         # Waveform generator RAM storage
         self.add_blockram("wf0_ram", 4096)
@@ -390,6 +394,13 @@ class UpsilonSoC(SoCCore):
                                 bus=self.adc0_PI.add_master(),
                                 registers=self.adc0.public_registers))
 
+        # Pre-finalizations. Very bad hacks.
+        self.pico0_ram_PI.pre_finalize()
+        self.dac0_PI.pre_finalize()
+        self.adc0_PI.pre_finalize()
+        self.wf0_ram_PI.pre_finalize()
+        self.wf0_PI.pre_finalize()
+
     def do_finalize(self):
         with open('soc_subregions.json', 'wt') as f:
             json.dump(self.soc_subregions, f)
@@ -411,9 +422,9 @@ def generate_main_cpu_include(csr_file):
 
         for key in subregions:
             if subregions[key] is None:
-                print(f'{key} = const({csrs["memories"][key]["base"]})', file=out)
+                print(f'{key} = const({csrs["memories"][key.lower()]["base"]})', file=out)
             else:
-                print(f'{key}_base = const({csrs["memories"][key]["base"]})', file=out)
+                print(f'{key}_base = const({csrs["memories"][key.lower()]["base"]})', file=out)
                 print(f'{key} = {subregions[key].__repr__()}', file=out)
 
 def main():
diff --git a/gateware/swic.py b/gateware/swic.py
index d30a5fe..b4a2f66 100644
--- a/gateware/swic.py
+++ b/gateware/swic.py
@@ -25,35 +25,54 @@ class PreemptiveInterface(LiteXModule):
     size of the region will be the same as the slave interface.
     """
 
-    def __init__(self, slave_bus, addressing="word"):
+    def __init__(self, slave_bus, addressing="word", name=None):
         """
         :param slave_bus: Instance of Wishbone.Interface that connects to the
           slave's bus.
         :param addressing: Addressing style of the slave. Default is "word". Note
           that masters may have to convert when selecting self.buses. This conversion
           is not done in this module.
+        :param name: Name for debugging purposes.
         """
 
         self.slave_bus = slave_bus
         self.addressing=addressing
         self.buses = []
+        self.name = name
+
+        self.pre_finalize_done = False
 
     def add_master(self):
         """ Adds a new master bus to the PI.
 
         :return: The interface to the bus.
         """
+        if self.pre_finalize_done:
+            raise Exception(self.name + ": Attempted to modify PreemptiveInterface after pre-finalize")
+
         iface = Interface(data_width=32, address_width=32, addressing=self.addressing)
         self.buses.append(iface)
         return iface
- 
-    def do_finalize(self):
+
+    def pre_finalize(self):
+        # NOTE: DUMB HACK! CSR bus logic is NOT generated when inserted at do_finalize time!
+
+        if self.pre_finalize_done:
+            raise Exception(self.name + ": Cannot pre-finalize twice")
+        self.pre_finalize_done = True
+
         masters_len = len(self.buses)
-        if masters_len == 0:
-            return None
         if masters_len > 1:
             self.master_select = CSRStorage(masters_len, name='master_select', description='RW bitstring of which master interconnect to connect to')
  
+    def do_finalize(self):
+        if not self.pre_finalize_done:
+            raise Exception(self.name + ": PreemptiveInterface needs a manual call to pre_finalize()")
+
+        masters_len = len(self.buses)
+        if masters_len == 0:
+            return None
+
         """
         Construct a combinatorial case statement. In verilog, the case
         statment would look like
@@ -138,6 +157,9 @@ class SpecialRegister:
 class RegisterInterface(LiteXModule):
     """ Defines "registers" that are either exclusively CPU-write Pico-read
     or CPU-read pico-write. These registers are stored as flip-flops. """
+
+    # TODO: Add no-write registers that are ready only for both ends.
+    # Also make more flexible signal sizes.
     def __init__(self, registers):
         """
         :param special_registers: List of instances of SpecialRegister.