Refactor read & write protocols to take in extra address arg

Author: ngernest

protocols/tests/wishbone_manager/hbexec.v

@@ -60,7 +60,7 @@
 
 `define	CMD_SUB_RD	2'b00         // Read request
 `define	CMD_SUB_WR	2'b01         // Write request 
-`define	CMD_SUB_BUS	1'b0          // A request in general 
+`define	CMD_SUB_BUS	1'b0          // Bus request (either a read or a write)
 `define	CMD_SUB_ADDR	2'b10     // Set an address
 `define	CMD_SUB_SPECIAL	2'b11     // Bus reset
 

protocols/tests/wishbone_manager/wishbone_manager.prot

@@ -27,6 +27,7 @@ struct WBManager {
     in i_wb_data: u32,
 }
 
+
 prot reset<DUT: WBManager>() {
     DUT.i_reset := 1'b1;
 
@@ -51,136 +52,127 @@ prot idle<DUT: WBManager>() {
     step();
 }
 
-// Sets the Wishbone bus address to `addr`.
-// Sends a CMD_SUB_ADDR command where the "opcode" is i_cmd_word[33:32] = 2'b10:
-//   i_cmd_word[31:2] = addr   (30-bit absolute address)
-//   i_cmd_word[1]    = 0      
-//   i_cmd_word[0]    = 0      
-//
-// The manager echoes the new address back on the response channel one cycle
-// after the command is accepted. Specifically, after one cycle,
-// `o_rsp_stb` is set to 1 and `o_rsp_word` contains the address.
-//
-// response format in `o_rsp_word`: { 2'b10, addr[29:0], 1'b0, !inc } 
-// where:
-// - RSP_SUB_ADDR = 2'b10 is the "opcode" for the response
-// - `inc` is an internal variable in the Verilog module.
-// When i_cmd_word[0]=0, inc=1, so !inc=0, i.e. the response is {2'b10, addr, 2'b00}.
-prot set_address<DUT: WBManager>(in addr: u30) {
+// This protocol writes a 32-bit `data` word to the Wishbone bus at `addr`, 
+// assuming that the subordinate does not stall (i_wb_stall = 0 throughout).
+// Under the hood, this corresponds to a "set address" command (`CMD_SUB_ADDR`),
+// followed immediately by a "write" command (`CMD_SUB_WR`).
+// (The two commands can be issued back to back, since updating the address
+// only updates the address and doesn't affect `cyc` or `stb`, and the latter
+// is what governs when `read` / `write` protocols can fire.)
+// State progression:
+// 1. IDLE: send CMD_SUB_ADDR, step -> address updated 
+// 2. IDLE: send CMD_SUB_WR, step -> cyc=1, stb=1, we=1 (bus request starts)
+// 3. BUS REQUEST (cyc=1, stb=1, we=1): subordinate accepts (stall=0), step
+// 4. BUS WAIT (cyc=1, stb=0): subordinate acknowledges (ack=1), step
+// 5. IDLE (cyc=0): RSP_WRITE_ACKNOWLEDGEMENT = {2'b01, 32'x0} on response channel
+prot write<DUT: WBManager>(in addr: u30, in data: u32) {
+    // Issue a "set address" command (`CMD_SUB_ADDR`)
+    // In `i_cmd_word`, we have:
+    // - bits[33:32] = 2'b10 (opcode for `CMD_SUB_ADDR`)
+    // - bits[31:2] = the actual address
     DUT.i_reset    := 1'b0;
     DUT.i_cmd_stb  := 1'b1;
-
-    // bit[33:32]=2'b10 (CMD_SUB_ADDR), bits[31:2]=addr, bit[1]=0 (absolute), bit[0]=0 (auto-increment)
     DUT.i_cmd_word := 30'b10 + addr + 30'b0;
     DUT.i_wb_stall := 1'b0;
     DUT.i_wb_ack   := 1'b0;
     DUT.i_wb_err   := 1'b0;
     assert_eq(DUT.o_cmd_busy, 1'b0);
-    step();
+    step();  // Address is updated in the Verilog during this step 
 
-    // At this point, in the Verilog, `o_wb_addr` and `newaddr` are updated. 
-    // TODO: figure out how to express address increment since our langauge
-    // doesn't have addition 
-    DUT.i_cmd_stb := 1'b0;
-    step();
-
-    // Manager echoes the new address back on the response channel
-    assert_eq(DUT.o_rsp_stb, 1'b1);
-    assert_eq(DUT.o_rsp_word, 30'b10 + addr + 30'b0);
-    step();
-}
-
-// Writes a 32-bit `data` word to the Wishbone bus at the current address.
-// Sends the CMD_SUB_WR "opcode" (i_cmd_word[33:32] = 2'b01) with `data` in bits [31:0].
-// This assumes the subordinate does not stall (i_wb_stall = 0 throughout).
-//
-// State machine progression:
-//   IDLE -> issue command -> step
-//   BUS REQUEST (cyc=1, stb=1, we=1): subordinate accepts immediately (stall=0) -> step
-//   BUS WAIT (cyc=1, stb=0): subordinate acknowledges (ack=1) -> step
-//   IDLE (cyc=0): RSP_WRITE_ACKNOWLEDGEMENT = {2'b01, 32'x0} on response channel
-prot write<DUT: WBManager>(in data: u32) {
-    // IDLE: issue write command
-    DUT.i_reset    := 1'b0;
+    // Issue a write command immediately by setting `stb = 1` 
+    // (`o_cmd_busy = 0` since `cyc = 0`)
+    // In `i_cmd_word`, we have:
+    // - bits[33:32] = 2'b01 (opcode for `CMD_SUB_WR`)
+    // - bits[31:0] = data
     DUT.i_cmd_stb  := 1'b1;
     DUT.i_cmd_word := 32'x1 + data;
-    DUT.i_wb_stall := 1'b0;
-    DUT.i_wb_ack   := 1'b0;
-    DUT.i_wb_err   := 1'b0;
     assert_eq(DUT.o_cmd_busy, 1'b0);
-    step();
+    assert_eq(DUT.o_wb_addr,  addr);
+    step();  // cyc=1, stb=1, we=1; address echo arrives on rsp channel simultaneously
 
-    // BUS REQUEST: manager asserts cyc=1, stb=1, we=1
+    // BUS REQUEST: up to now, the manager has asserted cyc=1, stb=1, we=1
+    // The manager now sets `stb = 0` (since the subordinate doesn't stall)
     DUT.i_cmd_stb := 1'b0;
-    assert_eq(DUT.o_wb_cyc, 1'b1);
-    assert_eq(DUT.o_wb_stb, 1'b1);
-    assert_eq(DUT.o_wb_we, 1'b1);
+    assert_eq(DUT.o_wb_cyc,   1'b1);
+    assert_eq(DUT.o_wb_stb,   1'b1);
+    assert_eq(DUT.o_wb_we,    1'b1);
     assert_eq(DUT.o_cmd_busy, 1'b1);
-    // stall=0, so manager will drop `o_wb_stb` after this step
     step();
 
-    // BUS WAIT: stb has dropped (request accepted), cyc still high
+    // BUS WAIT: We now have `stb = 0` (the request has been accepted)
+    // `cyc` is still 1 since we're still in the same session
     assert_eq(DUT.o_wb_cyc, 1'b1);
     assert_eq(DUT.o_wb_stb, 1'b0);
     // Subordinate acknowledges the write
     DUT.i_wb_ack := 1'b1;
     step();
 
     // IDLE: cyc dropped, write acknowledged on response channel
-    assert_eq(DUT.o_wb_cyc, 1'b0);
+    assert_eq(DUT.o_wb_cyc,   1'b0);
     assert_eq(DUT.o_cmd_busy, 1'b0);
-    assert_eq(DUT.o_rsp_stb, 1'b1);
-
+    assert_eq(DUT.o_rsp_stb,  1'b1);
     // RSP_WRITE_ACKNOWLEDGEMENT = { RSP_SUB_ACK(2'b01), 32'x0 }
     assert_eq(DUT.o_rsp_word, 32'x1 + 32'x0);
     DUT.i_wb_ack := 1'b0;
     step();
 }
 
-// Reads a 32-bit word from the Wishbone bus at the current address.
-// `rdata` is the value provided by the subordinate (driven on i_wb_data).
-// Sends CMD_SUB_RD (i_cmd_word[33:32] = 2'b00).
-// Assumes the subordinate does not stall (i_wb_stall = 0 throughout).
-//
+// This protocol reads a 32-bit word from the Wishbone bus at `addr`,
+// where `rdata` is the value provided by the subordinate (driven on i_wb_data)
+// (this is why `rdata` is an input parameter).
+// Like the `write` protocol above, we issue two commands back to back,
+// first by setting the address via `CMD_SUB_ADDR`
+// then by performing a read (`CMD_SUB_RD`)//
 // State progression:
-//   IDLE -> issue command -> step
-//   BUS REQUEST (cyc=1, stb=1, we=0): subordinate accepts immediately (stall=0) -> step
-//   BUS WAIT (cyc=1, stb=0): subordinate acknowledges with data (ack=1) -> step
-//   IDLE (cyc=0): { RSP_SUB_DATA(2'b00), rdata } on response channel
-prot read<DUT: WBManager>(in rdata: u32) {
-    // IDLE: issue read command (lower 32 bits are ignored by the manager)
+// 1. IDLE: send CMD_SUB_ADDR, step -> addr latched, newaddr=1
+// 2. IDLE: send CMD_SUB_RD, step -> cyc=1, stb=1, we=0 (bus cycle starts)
+// 3. BUS REQUEST (cyc=1, stb=1, we=0): subordinate accepts (stall=0), step
+// 4. BUS WAIT (cyc=1, stb=0): subordinate acknowledges with data (ack=1), step
+// 5. IDLE (cyc=0): Send response 
+prot read<DUT: WBManager>(in addr: u30, in rdata: u32) {
+    // Set the bus address 
     DUT.i_reset    := 1'b0;
     DUT.i_cmd_stb  := 1'b1;
-    DUT.i_cmd_word := 34'x0;
+    DUT.i_cmd_word := 30'b10 + addr + 30'b0;
     DUT.i_wb_stall := 1'b0;
     DUT.i_wb_ack   := 1'b0;
     DUT.i_wb_err   := 1'b0;
     DUT.i_wb_data  := rdata;
     assert_eq(DUT.o_cmd_busy, 1'b0);
-    step();
+    step();  
+
+    // Issue read command immediately 
+    // In `i_cmd_word`, we have `bits[33:32]=2'b00 (opcode for CMD_SUB_RD)`
+    // and the lower 32 bits are ignored by the manager (i.e. all zeroes)
+    DUT.i_cmd_stb  := 1'b1;
+    DUT.i_cmd_word := 34'x0;
+    assert_eq(DUT.o_cmd_busy, 1'b0);
+    assert_eq(DUT.o_wb_addr, addr);
+    step(); 
 
-    // BUS REQUEST: manager asserts cyc=1, stb=1, we=0
+    // BUS REQUEST: manager DUT has asserted cyc=1, stb=1, we=0
     DUT.i_cmd_stb := 1'b0;
     assert_eq(DUT.o_wb_cyc,   1'b1);
     assert_eq(DUT.o_wb_stb,   1'b1);
     assert_eq(DUT.o_wb_we,    1'b0);
     assert_eq(DUT.o_cmd_busy, 1'b1);
-    // stall=0, so manager will drop stb after this step
+    // We currently have stall=0, so the manager sets `stb = 0` after this step
     step();
 
-    // BUS WAIT: stb has dropped (request accepted), cyc still high
+    // BUS WAIT: The manager sets `stb = 0` (request accepted), cyc still high
     assert_eq(DUT.o_wb_cyc, 1'b1);
     assert_eq(DUT.o_wb_stb, 1'b0);
     // Subordinate acknowledges with read data
     DUT.i_wb_ack  := 1'b1;
     DUT.i_wb_data := rdata;
     step();
 
-    // IDLE: cyc dropped, read data on response channel
+    // IDLE: `cyc` becomes 0 (to indicate the end of a session)
+    // The read data is conveyed via the response
     assert_eq(DUT.o_wb_cyc,   1'b0);
     assert_eq(DUT.o_cmd_busy, 1'b0);
     assert_eq(DUT.o_rsp_stb,  1'b1);
-    // { RSP_SUB_DATA(2'b00), rdata }
+    // response format is { 2'b00 (opcode for RSP_SUB_DATA), rdata }
     assert_eq(DUT.o_rsp_word, 32'x0 + rdata);
     DUT.i_wb_ack := 1'b0;
     step();

protocols/tests/wishbone_manager/wishbone_manager.tx

@@ -2,22 +2,14 @@
 trace {
     reset();
 
-    // Set the bus address to 0x10 (absolute, auto-increment enabled)
-    set_address(30'h10);
+    // Write 0xDEADBEEF to address 0x10
+    write(16, 32'xDEADBEEF);
 
-    // Write 0xDEADBEEF to address 0x10.
-    // Address auto-increments to 0x11 after the request is accepted.
-    // TODO: need to figure out how to increment the address since our 
-    // language doesn't have addition
-    write(0xDEADBEEF);
+    // Read from address 0x11; subordinate returns 0xCAFEF00D
+    read(17, 32'xCAFEF00D);
 
-    // The subordinate returns 0xCAFEF00D from address 0x11.
-    // Manager should echo it as { 2'b00, 0xCAFEF00D } in o_rsp_word.
-    // Address auto-increments to 0x12.
-    read(0xCAFEF00D);
-
-    // Write 0xABCD1234 to address 0x12.
-    write(0xABCD1234);
+    // Write 0xABCD1234 to address 0x12
+    write(18, 32'xABCD1234);
 
     idle();
 }