From af3d2a29fcc23167f26c3e1006a74cc9590c10c7 Mon Sep 17 00:00:00 2001 From: Jiaxun Yang Date: Sun, 23 Jun 2024 15:58:39 +0100 Subject: [PATCH] csr_bus: Honour re signal from the upstream bus Currently CSR bus assumed that ~we means reading, that created a problem that when for a CSR if reading has side effects and adr parked unintentionally at that CSR, the reading side effect will be triggered. For SoCs, this happened when upstream bus issued a write transaction with wishbone.sel, then on CSR bus it will be translated as adr = addr, we = 0, which will be interpreted as a read to such address, and trigger undesired side effect for such CSR. Such upstream transaction will be generated by our bus width converter. Given that we signal already presents in CSR Interface, the easiest way to handle such situation is to generate re signal at bus bridges and propagate it all the way down to the Interface. Signed-off-by: Jiaxun Yang --- litex/soc/interconnect/axi/axi_lite.py | 5 ++++- litex/soc/interconnect/axi/axi_lite_to_csr.py | 1 + litex/soc/interconnect/csr_bus.py | 20 ++++++++++++------- litex/soc/interconnect/wishbone.py | 7 +++++-- test/test_csr.py | 4 ++-- 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/litex/soc/interconnect/axi/axi_lite.py b/litex/soc/interconnect/axi/axi_lite.py index 273bc08ba..25224c5a5 100644 --- a/litex/soc/interconnect/axi/axi_lite.py +++ b/litex/soc/interconnect/axi/axi_lite.py @@ -148,7 +148,7 @@ class AXILiteRemapper(LiteXModule): # AXI-Lite to Simple Bus --------------------------------------------------------------------------- -def axi_lite_to_simple(axi_lite, port_adr, port_dat_r, port_dat_w=None, port_we=None): +def axi_lite_to_simple(axi_lite, port_adr, port_dat_r, port_dat_w=None, port_re=None, port_we=None): """Connection of AXILite to simple bus with 1-cycle latency, such as CSR bus or Memory port""" bus_data_width = axi_lite.data_width adr_shift = log2_int(bus_data_width//8) @@ -168,6 +168,9 @@ def axi_lite_to_simple(axi_lite, port_adr, port_dat_r, port_dat_w=None, port_we= else: comb.append(port_we.eq(axi_lite.w.valid & axi_lite.w.ready & (axi_lite.w.strb != 0))) + if port_re is not None: + comb.append(port_re.eq(axi_lite.ar.valid & axi_lite.ar.ready)) + port_adr_reg = Signal(len(port_adr)) fsm = FSM() diff --git a/litex/soc/interconnect/axi/axi_lite_to_csr.py b/litex/soc/interconnect/axi/axi_lite_to_csr.py index b21dfa5b8..d6ceb68c4 100644 --- a/litex/soc/interconnect/axi/axi_lite_to_csr.py +++ b/litex/soc/interconnect/axi/axi_lite_to_csr.py @@ -32,6 +32,7 @@ class AXILite2CSR(LiteXModule): fsm, comb = axi_lite_to_simple( axi_lite = self.axi_lite, port_adr = self.csr.adr, + port_re = self.csr.re, port_dat_r = self.csr.dat_r, port_dat_w = self.csr.dat_w, port_we = self.csr.we) diff --git a/litex/soc/interconnect/csr_bus.py b/litex/soc/interconnect/csr_bus.py index 6a60f35cb..29e798c5d 100644 --- a/litex/soc/interconnect/csr_bus.py +++ b/litex/soc/interconnect/csr_bus.py @@ -18,9 +18,11 @@ sys_clk domain of the SoC, completing writes in a single cycle and reads in two ┌───────────┐ Write in 1 cycle: │ │ - adr/we/dat_w set by bridge. │ ├───► adr - │ │ Read in 2 cycles: - Main SoC Bus ◄────► CSR ├───► we - adr set by bridge - │ Bridge │ - dat_r set returned by user logic. + | | Read in 2 cycles: + │ ├───► re - adr and re set by bridge. + Main SoC Bus ◄────► │ - User logic can ignore re if there is no reading side effect. + | CSR ├───► we - dat_r set returned by user logic. + │ Bridge │ │ ├───► dat_w │ │ │ ◄──── dat_r @@ -46,6 +48,7 @@ from litex.soc.interconnect.csr import CSRStorage _layout = [ ("adr", "address_width", DIR_M_TO_S), + ("re", 1, DIR_M_TO_S), ("we", 1, DIR_M_TO_S), ("dat_w", "data_width", DIR_M_TO_S), ("dat_r", "data_width", DIR_S_TO_M) @@ -81,9 +84,11 @@ class Interface(Record): def read(self, adr): yield self.adr.eq(adr) + yield self.re.eq(1) yield - yield - return (yield self.dat_r) + value = (yield self.dat_r) + yield self.re.eq(0) + return value # CSR Interconnect --------------------------------------------------------------------------------- @@ -97,6 +102,7 @@ class InterconnectShared(Module): intermediate = Interface.like(masters[0]) self.comb += [ intermediate.adr.eq( Reduce("OR", [masters[i].adr for i in range(len(masters))])), + intermediate.re.eq( Reduce("OR", [masters[i].re for i in range(len(masters))])), intermediate.we.eq( Reduce("OR", [masters[i].we for i in range(len(masters))])), intermediate.dat_w.eq(Reduce("OR", [masters[i].dat_w for i in range(len(masters))])) ] @@ -207,8 +213,8 @@ class CSRBank(csr.GenericBank): self.comb += [ c.r.eq(self.bus.dat_w[:c.size]), If(sel & (self.bus.adr[:log2_int(aligned_paging)] == i), - c.re.eq( self.bus.we), - c.we.eq(~self.bus.we) + c.re.eq(self.bus.we), + c.we.eq(self.bus.re) ) ] diff --git a/litex/soc/interconnect/wishbone.py b/litex/soc/interconnect/wishbone.py index 13df1bc60..245170d6f 100644 --- a/litex/soc/interconnect/wishbone.py +++ b/litex/soc/interconnect/wishbone.py @@ -602,12 +602,14 @@ class Wishbone2CSR(LiteXModule): NextValue(self.csr.dat_w, self.wishbone.dat_w), If(self.wishbone.cyc & self.wishbone.stb, NextValue(self.csr.adr, self.wishbone.adr[wishbone_adr_shift:]), - NextValue(self.csr.we, self.wishbone.we & (self.wishbone.sel != 0)), + NextValue(self.csr.re, ~self.wishbone.we & (self.wishbone.sel != 0)), + NextValue(self.csr.we, self.wishbone.we & (self.wishbone.sel != 0)), NextState("WRITE-READ") ) ) fsm.act("WRITE-READ", NextValue(self.csr.adr, 0), + NextValue(self.csr.re, 0), NextValue(self.csr.we, 0), NextState("ACK") ) @@ -623,7 +625,8 @@ class Wishbone2CSR(LiteXModule): self.csr.dat_w.eq(self.wishbone.dat_w), If(self.wishbone.cyc & self.wishbone.stb, self.csr.adr.eq(self.wishbone.adr[wishbone_adr_shift:]), - self.csr.we.eq(self.wishbone.we & (self.wishbone.sel != 0)), + self.csr.re.eq(~self.wishbone.we & (self.wishbone.sel != 0)), + self.csr.we.eq( self.wishbone.we & (self.wishbone.sel != 0)), NextState("ACK") ) ) diff --git a/test/test_csr.py b/test/test_csr.py index c86e2350b..0e044f91f 100644 --- a/test/test_csr.py +++ b/test/test_csr.py @@ -19,9 +19,9 @@ def csr32_write(dut, adr, dat): def csr32_read(dut, adr): dat = 0 - for i in range(4): + for i in range(5): dat |= ((yield from dut.csr.read(adr + 3 - i)) << 8*i) - return dat + return dat >> 8 class CSRModule(Module, csr.AutoCSR):