From: whitequark <whitequark@whitequark.org>
Date: Sat, 26 Oct 2019 00:58:27 +0000 (+0000)
Subject: csr.bus.Multiplexer: fix element w_stb getting stuck.
X-Git-Tag: 24jan2021_ls180~32
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=d52d0b909b597e039ba1f8e75bff4c24d68657ed;p=nmigen-soc.git

csr.bus.Multiplexer: fix element w_stb getting stuck.

Also, don't clear shadow; this would break e.g. reading a 64-bit
CSR register through a 32-bit Wishbone bus if a code fetch happens
between the halves. Instead, clear shadow enable flag driving OR-mux.
---

diff --git a/nmigen_soc/csr/bus.py b/nmigen_soc/csr/bus.py
index 3e3e992..ffb8c3b 100644
--- a/nmigen_soc/csr/bus.py
+++ b/nmigen_soc/csr/bus.py
@@ -235,8 +235,12 @@ class Multiplexer(Elaboratable):
 
         for elem, (elem_start, elem_end) in self._map.resources():
             shadow = Signal(elem.width, name="{}__shadow".format(elem.name))
+            if elem.access.readable():
+                shadow_en = Signal(elem_end - elem_start, name="{}__shadow_en".format(elem.name))
+                m.d.sync += shadow_en.eq(0)
             if elem.access.writable():
                 m.d.comb += elem.w_data.eq(shadow)
+                m.d.sync += elem.w_stb.eq(0)
 
             # Enumerate every address used by the register explicitly, rather than using
             # arithmetic comparisons, since some toolchains (e.g. Yosys) are too eager to infer
@@ -244,20 +248,17 @@ class Multiplexer(Elaboratable):
             # to be powers of 2.)
             with m.Switch(self.bus.addr):
                 for chunk_offset, chunk_addr in enumerate(range(elem_start, elem_end)):
-                    with m.Case(chunk_addr):
-                        shadow_slice = shadow[chunk_offset * self.bus.data_width:
-                                              (chunk_offset + 1) * self.bus.data_width]
+                    shadow_slice = shadow.word_select(chunk_offset, self.bus.data_width)
 
+                    with m.Case(chunk_addr):
                         if elem.access.readable():
-                            chunk_r_stb = Signal(self.bus.data_width,
-                                name="{}__r_stb_{}".format(elem.name, chunk_offset))
-                            r_data_fanin |= Mux(chunk_r_stb, shadow_slice, 0)
+                            r_data_fanin |= Mux(shadow_en[chunk_offset], shadow_slice, 0)
                             if chunk_addr == elem_start:
                                 m.d.comb += elem.r_stb.eq(self.bus.r_stb)
                                 with m.If(self.bus.r_stb):
                                     m.d.sync += shadow.eq(elem.r_data)
                             # Delay by 1 cycle, allowing reads to be pipelined.
-                            m.d.sync += chunk_r_stb.eq(self.bus.r_stb)
+                            m.d.sync += shadow_en.eq(self.bus.r_stb << chunk_offset)
 
                         if elem.access.writable():
                             if chunk_addr == elem_end - 1:
@@ -267,9 +268,6 @@ class Multiplexer(Elaboratable):
                             with m.If(self.bus.w_stb):
                                 m.d.sync += shadow_slice.eq(self.bus.w_data)
 
-                with m.Default():
-                    m.d.sync += shadow.eq(0)
-
         m.d.comb += self.bus.r_data.eq(r_data_fanin)
 
         return m
diff --git a/nmigen_soc/test/test_csr_bus.py b/nmigen_soc/test/test_csr_bus.py
index d2ce624..d889128 100644
--- a/nmigen_soc/test/test_csr_bus.py
+++ b/nmigen_soc/test/test_csr_bus.py
@@ -167,10 +167,13 @@ class MultiplexerTestCase(unittest.TestCase):
             yield bus.w_stb.eq(1)
             yield
             yield bus.w_stb.eq(0)
+            yield bus.addr.eq(2) # change address
             yield
             self.assertEqual((yield elem_8_w.w_stb), 1)
             self.assertEqual((yield elem_8_w.w_data), 0x3d)
             self.assertEqual((yield elem_16_rw.w_stb), 0)
+            yield
+            self.assertEqual((yield elem_8_w.w_stb), 0)
 
             yield bus.addr.eq(2)
             yield bus.w_data.eq(0x55)