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 <jiaxun.yang@flygoat.com>
This commit is contained in:
Jiaxun Yang 2024-06-23 15:58:39 +01:00
parent 22f9c063db
commit af3d2a29fc
No known key found for this signature in database
GPG Key ID: 43710C7DD77729C3
5 changed files with 25 additions and 12 deletions

View File

@ -148,7 +148,7 @@ class AXILiteRemapper(LiteXModule):
# AXI-Lite to Simple Bus --------------------------------------------------------------------------- # 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""" """Connection of AXILite to simple bus with 1-cycle latency, such as CSR bus or Memory port"""
bus_data_width = axi_lite.data_width bus_data_width = axi_lite.data_width
adr_shift = log2_int(bus_data_width//8) 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: else:
comb.append(port_we.eq(axi_lite.w.valid & axi_lite.w.ready & (axi_lite.w.strb != 0))) 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)) port_adr_reg = Signal(len(port_adr))
fsm = FSM() fsm = FSM()

View File

@ -32,6 +32,7 @@ class AXILite2CSR(LiteXModule):
fsm, comb = axi_lite_to_simple( fsm, comb = axi_lite_to_simple(
axi_lite = self.axi_lite, axi_lite = self.axi_lite,
port_adr = self.csr.adr, port_adr = self.csr.adr,
port_re = self.csr.re,
port_dat_r = self.csr.dat_r, port_dat_r = self.csr.dat_r,
port_dat_w = self.csr.dat_w, port_dat_w = self.csr.dat_w,
port_we = self.csr.we) port_we = self.csr.we)

View File

@ -18,9 +18,11 @@ sys_clk domain of the SoC, completing writes in a single cycle and reads in two
Write in 1 cycle: Write in 1 cycle:
- adr/we/dat_w set by bridge. - adr/we/dat_w set by bridge.
adr adr
Read in 2 cycles: | | Read in 2 cycles:
Main SoC Bus CSR we - adr set by bridge re - adr and re set by bridge.
Bridge - dat_r set returned by user logic. 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_w
dat_r dat_r
@ -46,6 +48,7 @@ from litex.soc.interconnect.csr import CSRStorage
_layout = [ _layout = [
("adr", "address_width", DIR_M_TO_S), ("adr", "address_width", DIR_M_TO_S),
("re", 1, DIR_M_TO_S),
("we", 1, DIR_M_TO_S), ("we", 1, DIR_M_TO_S),
("dat_w", "data_width", DIR_M_TO_S), ("dat_w", "data_width", DIR_M_TO_S),
("dat_r", "data_width", DIR_S_TO_M) ("dat_r", "data_width", DIR_S_TO_M)
@ -81,9 +84,11 @@ class Interface(Record):
def read(self, adr): def read(self, adr):
yield self.adr.eq(adr) yield self.adr.eq(adr)
yield self.re.eq(1)
yield yield
yield value = (yield self.dat_r)
return (yield self.dat_r) yield self.re.eq(0)
return value
# CSR Interconnect --------------------------------------------------------------------------------- # CSR Interconnect ---------------------------------------------------------------------------------
@ -97,6 +102,7 @@ class InterconnectShared(Module):
intermediate = Interface.like(masters[0]) intermediate = Interface.like(masters[0])
self.comb += [ self.comb += [
intermediate.adr.eq( Reduce("OR", [masters[i].adr for i in range(len(masters))])), 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.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))])) 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 += [ self.comb += [
c.r.eq(self.bus.dat_w[:c.size]), c.r.eq(self.bus.dat_w[:c.size]),
If(sel & (self.bus.adr[:log2_int(aligned_paging)] == i), If(sel & (self.bus.adr[:log2_int(aligned_paging)] == i),
c.re.eq( self.bus.we), c.re.eq(self.bus.we),
c.we.eq(~self.bus.we) c.we.eq(self.bus.re)
) )
] ]

View File

@ -602,12 +602,14 @@ class Wishbone2CSR(LiteXModule):
NextValue(self.csr.dat_w, self.wishbone.dat_w), NextValue(self.csr.dat_w, self.wishbone.dat_w),
If(self.wishbone.cyc & self.wishbone.stb, If(self.wishbone.cyc & self.wishbone.stb,
NextValue(self.csr.adr, self.wishbone.adr[wishbone_adr_shift:]), NextValue(self.csr.adr, self.wishbone.adr[wishbone_adr_shift:]),
NextValue(self.csr.re, ~self.wishbone.we & (self.wishbone.sel != 0)),
NextValue(self.csr.we, self.wishbone.we & (self.wishbone.sel != 0)), NextValue(self.csr.we, self.wishbone.we & (self.wishbone.sel != 0)),
NextState("WRITE-READ") NextState("WRITE-READ")
) )
) )
fsm.act("WRITE-READ", fsm.act("WRITE-READ",
NextValue(self.csr.adr, 0), NextValue(self.csr.adr, 0),
NextValue(self.csr.re, 0),
NextValue(self.csr.we, 0), NextValue(self.csr.we, 0),
NextState("ACK") NextState("ACK")
) )
@ -623,7 +625,8 @@ class Wishbone2CSR(LiteXModule):
self.csr.dat_w.eq(self.wishbone.dat_w), self.csr.dat_w.eq(self.wishbone.dat_w),
If(self.wishbone.cyc & self.wishbone.stb, If(self.wishbone.cyc & self.wishbone.stb,
self.csr.adr.eq(self.wishbone.adr[wishbone_adr_shift:]), 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") NextState("ACK")
) )
) )

View File

@ -19,9 +19,9 @@ def csr32_write(dut, adr, dat):
def csr32_read(dut, adr): def csr32_read(dut, adr):
dat = 0 dat = 0
for i in range(4): for i in range(5):
dat |= ((yield from dut.csr.read(adr + 3 - i)) << 8*i) dat |= ((yield from dut.csr.read(adr + 3 - i)) << 8*i)
return dat return dat >> 8
class CSRModule(Module, csr.AutoCSR): class CSRModule(Module, csr.AutoCSR):