A protocol controller: the APB completer

Design an APB completer that reads the bus state from the wires, acts at exactly one edge per transfer, inserts wait states and refuses bad transfers, and passes the protocol checker.

Lesson: R3.5
Time: 30 min
Runs on: Your browser
Check: 5 questions

A GPIO block counted the writes it received, for a driver that polled the count. After one write the count read 2. The completer acted in every cycle its select was high: once in the setup cycle, once at completion. The protocol checker was silent, because every wire obeyed the rules; the bug was inside, in the one line that decides when a transfer has happened.

One write with one wait state, as the completer sees itThe requester writes 0x5a to LEDS. The completer decodes the state from PSEL and PENABLE, counts one wait cycle in ACCESS, then raises PREADY. LEDS and COUNT change once, after the completion edge.

Cycle 1
Edge 3: PSEL is 1 and PENABLE is 0, so the state is SETUP. The completer decodes the address; nothing is written.
Cycle 2
Edge 4: PENABLE is 1: ACCESS. The wait counter is still 0 and WAITS is 1, so PREADY stays 0. The counter advances.
Cycle 3
Edge 5: still ACCESS, the counter reads 1, PREADY is 1. This is the completion edge: PSEL, PENABLE and PREADY are all 1, and LEDS takes 0x5a at this edge.
Cycle 4
Edge 6: the requester has started the next transfer, so the state is SETUP again. count shows 1 and leds 0x5a: one write, one action.

Measured on the bench with WAITS = 1, values sampled at edges 2 to 7. A completer that acted whenever selected would show count = 3 here: it acts at the setup, wait and completion edges.

The three states are on the wires

The APB specification draws the requester as a state machine with three states: IDLE, SETUP for exactly one cycle, and ACCESS until the completer raises PREADY. A completer does not need to track that machine in a register. Both state signals arrive on the wires: PSEL says whether a transfer is in progress, PENABLE says whether its setup cycle is over. Decode them and the state is known in the same cycle.

PSEL	PENABLE	State	What the completer does
0	any	IDLE	nothing; outputs may be anything but PSLVERR should be 0
1	0	SETUP	decodes PADDR and PWRITE; may prepare read data; changes no register
1	1	ACCESS	counts wait cycles, raises PREADY when they are done; at the edge where PREADY is 1 the transfer completes

IHI 0024E 4.1 names the states. The completer reads them; it does not store them.

The operating states, as the completer decodes themIDLE while PSEL is 0. SETUP in the one cycle where PSEL is 1 and PENABLE is 0. ACCESS while PENABLE is 1, which ends at the edge where PREADY is 1; the next cycle is IDLE or, back to back, another SETUP.

Setup
The requester drives the address, the direction and the data, and raises PSEL. The completer sees SETUP and decodes.
Follow the calls, values and waits
1. data write: IDLE → SETUP
Access
PENABLE rises. The completer counts its wait cycles with PREADY low, then raises PREADY. The edge where PREADY is 1 is the completion edge: the write lands there.
Follow the calls, values and waits
1. data write: SETUP → ACCESS
2. data write: ACCESS → Completion edge
After completion
If the requester has another transfer the next cycle is SETUP with PSEL still high; otherwise PSEL falls and the bus is IDLE. The completer decodes either from the wires.
Follow the calls, values and waits
1. return write: Completion edge → IDLE

Read the diagram as text

IDLE: PSEL = 0
SETUP: PSEL = 1, PENABLE = 0: one cycle
ACCESS: PENABLE = 1: wait cycles with PREADY 0
Completion edge: PSEL, PENABLE, PREADY all 1: the write lands, PRDATA and PSLVERR are read

IDLE → SETUP: PSEL rises
SETUP → ACCESS: next edge
ACCESS → Completion edge: PREADY = 1
Completion edge → IDLE: no more transfers
Completion edge → SETUP: back to back

A decoded state is right in the same cycle. A state kept in a register is right one cycle later.

One edge to act on

A transfer completes at exactly one rising edge: the one where PSEL, PENABLE and PREADY are all 1. That edge is where a write lands, a counter counts, a FIFO pops, and where PSLVERR and PRDATA are read by the requester. Every side effect in the completer sits under that one condition. The two tempting shortcuts both break it.

Acts whenever selected

wire act = (st != IDLE) && pwrite;
// lands the write in SETUP and
// again at completion

Two actions per write: COUNT reads 2. The checker is silent: the wires are legal.

Acts at the completion edge

wire done = (st == ACCESS) && pready;
// one edge per transfer

One action per write. PSLVERR and PRDATA are read at the same edge.

The other shortcut is to keep the state in a register and move it on each edge, as a requester would. A registered copy of SETUP is written at the edge that ends the setup cycle, so it reads SETUP during the access cycle: one cycle late. Measured on this bench with WAITS = 1, that version completes the first write at edge 6 instead of edge 5, two wait cycles instead of one. It works, and it costs a cycle on every transfer. The wires already carry the state; read them.

Wait states, errors and the brief

Wait states
PREADY is 1 when the completer has spent WAITS cycles in ACCESS. The counter runs only in ACCESS with PREADY low and clears otherwise, so back-to-back transfers each get their own count.
Errors
PSLVERR is 1 only at the completion edge, and only for a transfer the completer refuses: an unmapped address or a write to a read-only register. A refused transfer changes nothing.
Read data
PRDATA is driven from the decoded address throughout the transfer; the requester takes it at the completion edge. A PRDATA registered only at the completion edge would arrive one transfer late.
Reset
Active-low, as the bus requires, and asynchronous by house style; the registers clear and the counter clears.

Design brief

Port	Direction	Width	Meaning
pclk	input	1	APB clock
presetn	input	1	APB reset, asynchronous, active-low
psel	input	1	APB select
penable	input	1	APB access phase
pwrite	input	1	APB write
paddr	input	[31:0]	APB address
pwdata	input	[31:0]	APB write data
pstrb	input	[3:0]	APB write strobes
pprot	input	[2:0]	APB protection
pready	output	1	APB ready: every access takes WAITS wait cycles
prdata	output	[31:0]	APB read data
pslverr	output	1	unmapped address, or a write to COUNT or ID: PSLVERR, no change
leds	output	[7:0]	the LEDS register

Reset: PRESETn asynchronous active-low: LEDS 0, COUNT 0, counter 0
Latency: every access takes WAITS wait cycles; actions and PSLVERR at the completion edge only
Parameters: WAITS, wait cycles per access. Registers: 0x00 LEDS read/write (byte 0, under PSTRB[0]); 0x04 COUNT read-only, the completed LEDS writes; 0x08 ID read-only
Corner cases: back-to-back transfers with PSEL held; a refused write; a read during a wait

The lab ships the completer with WAITS = 0 behind the Protocols track's checker. Run it: five transfers, the refused write marked ERROR, LEDS lit, zero rules broken. Change the parameter in the bench to WAITS = 1 and the log shows one wait per transfer. Then replace (st == ACCESS) && pready in done by (st != IDLE) and read COUNT again: 3 for one write, one action per selected edge. The checker still reports zero rules broken.

Asked in interviews

At which edge does an APB completer act on a write?
The completion edge: the rising edge where PSEL, PENABLE and PREADY are all 1. Acting earlier, in the setup cycle or a wait cycle, lands the write twice or lands data the requester may still change.
Does a completer need a state machine for APB?
It needs to know the state, and PSEL and PENABLE tell it directly: IDLE, SETUP or ACCESS. A registered copy is one cycle late. What it does need to register is a wait counter and its own registers.
When may PSLVERR be high?
It is valid only at the completion edge. Outside it the requester ignores it, and the specification recommends keeping it low; this completer ties it to the completion condition.

Try it

Each log line names the setup edge and the completion edge of a transfer. Every transfer takes two edges, because WAITS is 0: the next SETUP starts right after a completion. The write to COUNT is refused with ERROR and changes nothing: COUNT still reads 1 afterwards, and the checker counts no broken rule, because a refusal is a legal answer.

In the bench file, change apb_led dut to apb_led #(.WAITS(1)) dut and run: every transfer takes three edges and the checker counts five wait edges. Then edit the design's done to (st != IDLE) and run again: the read of COUNT returns 3 after one write, one action per selected edge, with no rule broken.

RTLIcarus Verilog · in your browser · Lint: Verilator service · Synthesize: Yosys service

Edit the design, then Run, Lint or Synthesize

Expected: e3-e4 WRITE 0x00000000 data=0x0000005a strb=1111 waits=0 OKAY e5-e6 READ 0x00000000 data=0x0000005a waits=0 OKAY e7-e8 READ 0x00000004 data=0x00000001 waits=0 OKAY e9-e10 WRITE 0x00000004 data=0x00000001 strb=1111 waits=0 ERROR e11-e12 READ 0x00000008 data=0x4c454430 waits=0 OKAY leds=01011010 checker: 2 writes, 3 reads, 0 wait edges, 0 rules broken, 0 notes

Original design · 190 cells · 40 flip-flops · 0 latches · 31 logic levels. Synthesis uses Yosys; counts depend on the toolchain.

Original example source

// apb_led: an APB4 completer with LEDS (0x00, read/write), COUNT (0x04, read-only: writes to LEDS
// that completed) and ID (0x08, read-only). WAITS wait states in every access.
module apb_led #(parameter int WAITS = 0) (
  input  logic        pclk,
  input  logic        presetn,     // active-low, asynchronous
  input  logic        psel,
  input  logic        penable,
  input  logic        pwrite,
  input  logic [31:0] paddr,
  input  logic [31:0] pwdata,
  input  logic [3:0]  pstrb,
  input  logic [2:0]  pprot,
  output logic        pready,
  output logic [31:0] prdata,
  output logic        pslverr,
  output logic [7:0]  leds
);
  // The bus state, read from the wires: IDLE (PSEL 0), SETUP (PSEL 1, PENABLE 0), ACCESS (PENABLE 1).
  typedef enum logic [1:0] {IDLE, SETUP, ACCESS} state_e;
  state_e st;
  always_comb begin
    if (!psel)         st = IDLE;
    else if (!penable) st = SETUP;
    else               st = ACCESS;
  end

  logic [2:0]  wcnt_q;             // wait cycles spent in this access
  logic [31:0] count_q;
  wire is_leds  = paddr[7:0] == 8'h00;
  wire is_count = paddr[7:0] == 8'h04;
  wire is_id    = paddr[7:0] == 8'h08;
  wire bad      = !(is_leds || is_count || is_id) || (pwrite && (is_count || is_id));

  assign pready  = (st == ACCESS) && (wcnt_q >= WAITS);
  wire   done    = (st == ACCESS) && pready;          // the one edge where the transfer completes
  assign pslverr = done && bad;
  assign prdata  = is_leds ? {24'd0, leds} : is_count ? count_q : is_id ? 32'h4c454430 : 32'd0;

  always_ff @(posedge pclk or negedge presetn)
    if (!presetn) begin
      wcnt_q <= '0; leds <= '0; count_q <= '0;
    end else begin
      wcnt_q <= (st == ACCESS && !pready) ? wcnt_q + 3'd1 : 3'd0;
      if (done && !bad && pwrite && is_leds) begin   // act once, at the completion edge
        if (pstrb[0]) leds <= pwdata[7:0];
        count_q <= count_q + 1;
      end
    end
endmodule

Read included file: tb.sv

`timescale 1ns/1ps
module tb;
  logic pclk = 0, presetn = 0;
  always #5 pclk = ~pclk;                 // rising edges at 5, 15, 25, ...
  logic        psel = 0, penable = 0, pwrite = 0;
  logic [31:0] paddr = 0, pwdata = 0;
  logic [3:0]  pstrb = 0;
  logic [2:0]  pprot = 0;
  logic        pready, pslverr;
  logic [31:0] prdata;
  logic [7:0] leds;
  apb_led dut (.pclk(pclk), .presetn(presetn), .psel(psel), .penable(penable), .pwrite(pwrite), .paddr(paddr),
      .pwdata(pwdata), .pstrb(pstrb), .pprot(pprot), .pready(pready), .prdata(prdata), .pslverr(pslverr), .leds(leds));
  apb_checker chk (.pclk(pclk), .presetn(presetn), .psel(psel), .penable(penable), .pwrite(pwrite), .paddr(paddr),
                   .pwdata(pwdata), .pstrb(pstrb), .pprot(pprot), .pready(pready), .pslverr(pslverr));
  `include "rtl_apb_tasks.svh"
  initial begin

    repeat (2) @(posedge pclk); #1 presetn = 1;   // reset released after edge 1
    @(posedge pclk); #1;                          // the first transfer may start after edge 2
    apb_write(32'h00, 32'h5a);              // LEDS
    apb_read(32'h00);
    apb_read(32'h04);                       // COUNT
    apb_write(32'h04, 32'h1);               // read-only: refused
    apb_read(32'h08);                       // ID
    $display("leds=%b", leds);
    repeat (3) @(posedge pclk);
    $finish(0);
  end
endmodule

Read included file: apb_checker.sv

`timescale 1ns/1ns
// apb_checker.sv: the rules of an APB interface, checked at every rising edge (read-only).
//   reset        PSEL is 1 while PRESETn is low;
//   known        PSEL is x, or a control signal, a strobed write byte or PREADY in access is x;
//   setup        PENABLE is 1 without a setup cycle (PSEL 1, PENABLE 0) before it;
//   setup_len    a setup cycle not followed by an access cycle;
//   stable       PSEL, PADDR, PWRITE, PPROT, PSTRB or the write data changed inside a transfer,
//                or PSEL or PENABLE dropped before PREADY;
//   enable_drop  PENABLE still 1 in the cycle after a completed transfer;
//   strb_read    PSTRB not zero during a read;
//   onehot       more than one PSELx at once;
//   timeout      no PREADY after TIMEOUT wait cycles (a liveness watchdog, not a protocol rule).
// A NOTE (not a broken rule): PSLVERR is 1 outside the completion cycle, which the spec allows
// and recommends against (IHI 0024E 3.4).
module apb_checker #(parameter int NSEL = 1, parameter int TIMEOUT = 16) (
  input logic            pclk,
  input logic            presetn,
  input logic [NSEL-1:0] psel,
  input logic            penable,
  input logic            pwrite,
  input logic [31:0]     paddr,
  input logic [31:0]     pwdata,
  input logic [3:0]      pstrb,
  input logic [2:0]      pprot,
  input logic            pready,
  input logic            pslverr);
  int broken = 0, notes = 0, writes = 0, reads = 0, waits = 0, edge_n = -1, stall = 0;
  logic            p_any = 0, p_en = 0, p_rdy = 0, p_wr = 0;
  logic [NSEL-1:0] p_sel = 0;
  logic [31:0]     p_addr = 0, p_wdata = 0;
  logic [3:0]      p_strb = 0;
  logic [2:0]      p_prot = 0;
  task automatic report(input string rule, input string msg);
    broken++;
    $display("RULE %0s broken at edge %0d: %0s", rule, edge_n, msg);
  endtask
  task automatic note(input string rule, input string msg);
    notes++;
    $display("NOTE %0s at edge %0d: %0s", rule, edge_n, msg);
  endtask
  always @(posedge pclk) begin
    logic any, en, rdy, bytes_x;
    edge_n = edge_n + 1;
    any = (|psel) === 1'b1; en = (penable === 1'b1); rdy = (pready === 1'b1);
    if (presetn !== 1'b1) begin
      if (any) report("reset", "PSEL is 1 while PRESETn is low");
      any = 0; en = 0;
    end else begin
      bytes_x = 0;
      for (int l = 0; l < 4; l++) if (pstrb[l] === 1'b1 && (^pwdata[8*l +: 8]) === 1'bx) bytes_x = 1;
      if ((^psel) === 1'bx) report("known", "PSEL is x");
      else if (any && (^{paddr, pwrite, penable, pstrb, pprot}) === 1'bx) report("known", "a control signal is x while PSEL is 1");
      else if (any && pwrite === 1'b1 && bytes_x) report("known", "a strobed byte of PWDATA is x");
      else if (any && en && pready === 1'bx) report("known", "PREADY is x in an access cycle");
      if (any && en && !(p_any && (!p_en || !p_rdy)))
        report("setup", "PENABLE is 1 without a setup cycle (PSEL 1, PENABLE 0) before it");
      if (p_any && !p_en && !(any && en))
        report("setup_len", "the setup cycle was not followed by an access cycle");
      if (p_any && (!p_en || !p_rdy) && any) begin
        if (psel !== p_sel) report("stable", $sformatf("PSEL changed from %b to %b inside a transfer", p_sel, psel));
        if (paddr !== p_addr) report("stable", $sformatf("PADDR changed from 0x%0h to 0x%0h inside a transfer", p_addr, paddr));
        if (pwrite !== p_wr) report("stable", "PWRITE changed inside a transfer");
        if (pprot !== p_prot) report("stable", "PPROT changed inside a transfer");
        if (pstrb !== p_strb) report("stable", "PSTRB changed inside a transfer");
        if (p_wr && pwdata !== p_wdata) report("stable", $sformatf("PWDATA changed from 0x%0h to 0x%0h inside a transfer", p_wdata, pwdata));
        if (p_en && !en) report("stable", "PENABLE dropped before PREADY");
      end
      if (p_any && p_en && !p_rdy && !any) report("stable", "PSEL dropped before PREADY");
      if (p_any && p_en && p_rdy && en) report("enable_drop", "PENABLE is still 1 in the cycle after a completed transfer");
      if (any && pwrite === 1'b0 && pstrb !== 4'b0000) report("strb_read", $sformatf("PSTRB is %b during a read", pstrb));
      if ($countones(psel) > 1) report("onehot", $sformatf("PSELx is %b: two completers selected", psel));
      if (any && en && rdy) begin
        if (pwrite === 1'b1) writes++; else reads++;
        stall = 0;
      end else if (any && en) begin
        waits++; stall++;
        if (stall == TIMEOUT) report("timeout", $sformatf("no PREADY after %0d wait cycles", TIMEOUT));
      end
      if (pslverr === 1'b1 && !(any && en && rdy))
        note("pslverr", "PSLVERR is 1 outside the completion cycle (ignored by the requester; recommended LOW)");
    end
    p_any = any; p_en = en; p_rdy = rdy; p_sel = psel; p_addr = paddr; p_wr = (pwrite === 1'b1);
    p_wdata = pwdata; p_strb = pstrb; p_prot = pprot;
  end
  final $display("checker: %0d writes, %0d reads, %0d wait edges, %0d rules broken, %0d notes", writes, reads, waits, broken, notes);
endmodule

Read included file: rtl_apb_tasks.svh

// rtl_apb_tasks.svh: the requester side of the bench (read-only).
// apb_write(addr, data, strb) and apb_read(addr) run one transfer: SETUP for one cycle, ACCESS until
// PREADY, then the bus goes idle. Each transfer prints one line at its completion edge:
//   e3-e4 WRITE 0x00000000 data=0x0000005a strb=1111 waits=0 OKAY
// (setup edge - completion edge). A read leaves its data in prdata_q and its error in pslverr_q.
// quiet = 1 turns the log off; watch = 1 prints the bus at every edge.
// cycle is the number of the last rising edge; it updates after the edge's active events, so at an
// edge every reader (the tasks, the watch line) sees the same old value and names the edge cycle + 1.
int cycle = -1;
always @(posedge pclk) cycle <= cycle + 1;
bit quiet = 0, watch = 0;
logic [31:0] prdata_q = 0;
logic        pslverr_q = 0;
int          waits_q = 0, give_up = 24;
always @(posedge pclk) if (watch)
  $display("wave e%0d: psel=%b penable=%b pwrite=%b paddr=%h pwdata=%h pstrb=%b pready=%b prdata=%h pslverr=%b",
           cycle + 1, psel, penable, pwrite, paddr, pwdata, pstrb, pready, prdata, pslverr);

task automatic apb_xfer(input bit wr, input logic [31:0] addr, input logic [31:0] data, input logic [3:0] strb);
  int s_edge, w;
  paddr = addr; pwrite = wr; pwdata = wr ? data : 32'h0; pstrb = wr ? strb : 4'h0; pprot = 3'b000;
  psel = 1; penable = 0;                       // SETUP
  s_edge = cycle + 1;
  @(posedge pclk); #1 penable = 1;             // ACCESS
  w = 0;
  @(posedge pclk);
  while (pready !== 1'b1 && w < give_up) begin w++; @(posedge pclk); end
  prdata_q = prdata; pslverr_q = pslverr; waits_q = w;
  if (!quiet) begin
    if (wr && pslverr)  $display("e%0d-e%0d WRITE 0x%08h data=0x%08h strb=%b waits=%0d ERROR", s_edge, cycle + 1, addr, data, strb, w);
    else if (wr)        $display("e%0d-e%0d WRITE 0x%08h data=0x%08h strb=%b waits=%0d OKAY", s_edge, cycle + 1, addr, data, strb, w);
    else if (pslverr)   $display("e%0d-e%0d READ  0x%08h data=0x%08h waits=%0d ERROR", s_edge, cycle + 1, addr, prdata, w);
    else                $display("e%0d-e%0d READ  0x%08h data=0x%08h waits=%0d OKAY", s_edge, cycle + 1, addr, prdata, w);
  end
  #1 psel = 0; penable = 0; pwrite = 0; pstrb = 0; pwdata = 0;   // idle
endtask
task automatic apb_write(input logic [31:0] addr, input logic [31:0] data, input logic [3:0] strb = 4'hf);
  apb_xfer(1, addr, data, strb);
endtask
task automatic apb_read(input logic [31:0] addr);
  apb_xfer(0, addr, 32'h0, 4'h0);
endtask
task automatic idle(input int n);
  repeat (n) @(posedge pclk); #1;
endtask

Check yourself

Predict the outputThe completer is built with WAITS = 2 and the log is off. The requester writes LEDS twice and reads COUNT. The checker prints one summary line. What is it?

// apb_led: an APB4 completer with LEDS (0x00, read/write), COUNT (0x04, read-only: writes to LEDS
// that completed) and ID (0x08, read-only). WAITS wait states in every access.
module apb_led #(parameter int WAITS = 0) (
  input  logic        pclk,
  input  logic        presetn,     // active-low, asynchronous
  input  logic        psel,
  input  logic        penable,
  input  logic        pwrite,
  input  logic [31:0] paddr,
  input  logic [31:0] pwdata,
  input  logic [3:0]  pstrb,
  input  logic [2:0]  pprot,
  output logic        pready,
  output logic [31:0] prdata,
  output logic        pslverr,
  output logic [7:0]  leds
);
  // The bus state, read from the wires: IDLE (PSEL 0), SETUP (PSEL 1, PENABLE 0), ACCESS (PENABLE 1).
  typedef enum logic [1:0] {IDLE, SETUP, ACCESS} state_e;
  state_e st;
  always_comb begin
    if (!psel)         st = IDLE;
    else if (!penable) st = SETUP;
    else               st = ACCESS;
  end

  logic [2:0]  wcnt_q;             // wait cycles spent in this access
  logic [31:0] count_q;
  wire is_leds  = paddr[7:0] == 8'h00;
  wire is_count = paddr[7:0] == 8'h04;
  wire is_id    = paddr[7:0] == 8'h08;
  wire bad      = !(is_leds || is_count || is_id) || (pwrite && (is_count || is_id));

  assign pready  = (st == ACCESS) && (wcnt_q >= WAITS);
  wire   done    = (st == ACCESS) && pready;          // the one edge where the transfer completes
  assign pslverr = done && bad;
  assign prdata  = is_leds ? {24'd0, leds} : is_count ? count_q : is_id ? 32'h4c454430 : 32'd0;

  always_ff @(posedge pclk or negedge presetn)
    if (!presetn) begin
      wcnt_q <= '0; leds <= '0; count_q <= '0;
    end else begin
      wcnt_q <= (st == ACCESS && !pready) ? wcnt_q + 3'd1 : 3'd0;
      if (done && !bad && pwrite && is_leds) begin   // act once, at the completion edge
        if (pstrb[0]) leds <= pwdata[7:0];
        count_q <= count_q + 1;
      end
    end
endmodule
`timescale 1ns/1ps
module tb;
  logic pclk = 0, presetn = 0;
  always #5 pclk = ~pclk;                 // rising edges at 5, 15, 25, ...
  logic        psel = 0, penable = 0, pwrite = 0;
  logic [31:0] paddr = 0, pwdata = 0;
  logic [3:0]  pstrb = 0;
  logic [2:0]  pprot = 0;
  logic        pready, pslverr;
  logic [31:0] prdata;
  logic [7:0] leds;
  apb_led #(.WAITS(2)) dut (.pclk(pclk), .presetn(presetn), .psel(psel), .penable(penable), .pwrite(pwrite), .paddr(paddr),
      .pwdata(pwdata), .pstrb(pstrb), .pprot(pprot), .pready(pready), .prdata(prdata), .pslverr(pslverr), .leds(leds));
  apb_checker chk (.pclk(pclk), .presetn(presetn), .psel(psel), .penable(penable), .pwrite(pwrite), .paddr(paddr),
                   .pwdata(pwdata), .pstrb(pstrb), .pprot(pprot), .pready(pready), .pslverr(pslverr));
  `include "rtl_apb_tasks.svh"
  initial begin

    repeat (2) @(posedge pclk); #1 presetn = 1;   // reset released after edge 1
    @(posedge pclk); #1;                          // the first transfer may start after edge 2
    quiet = 1;
    apb_write(32'h00, 32'hff);
    apb_write(32'h00, 32'h0f);
    apb_read(32'h04);
    repeat (3) @(posedge pclk);
    $finish(0);
  end
endmodule

Read included file: apb_checker.sv

`timescale 1ns/1ns
// apb_checker.sv: the rules of an APB interface, checked at every rising edge (read-only).
//   reset        PSEL is 1 while PRESETn is low;
//   known        PSEL is x, or a control signal, a strobed write byte or PREADY in access is x;
//   setup        PENABLE is 1 without a setup cycle (PSEL 1, PENABLE 0) before it;
//   setup_len    a setup cycle not followed by an access cycle;
//   stable       PSEL, PADDR, PWRITE, PPROT, PSTRB or the write data changed inside a transfer,
//                or PSEL or PENABLE dropped before PREADY;
//   enable_drop  PENABLE still 1 in the cycle after a completed transfer;
//   strb_read    PSTRB not zero during a read;
//   onehot       more than one PSELx at once;
//   timeout      no PREADY after TIMEOUT wait cycles (a liveness watchdog, not a protocol rule).
// A NOTE (not a broken rule): PSLVERR is 1 outside the completion cycle, which the spec allows
// and recommends against (IHI 0024E 3.4).
module apb_checker #(parameter int NSEL = 1, parameter int TIMEOUT = 16) (
  input logic            pclk,
  input logic            presetn,
  input logic [NSEL-1:0] psel,
  input logic            penable,
  input logic            pwrite,
  input logic [31:0]     paddr,
  input logic [31:0]     pwdata,
  input logic [3:0]      pstrb,
  input logic [2:0]      pprot,
  input logic            pready,
  input logic            pslverr);
  int broken = 0, notes = 0, writes = 0, reads = 0, waits = 0, edge_n = -1, stall = 0;
  logic            p_any = 0, p_en = 0, p_rdy = 0, p_wr = 0;
  logic [NSEL-1:0] p_sel = 0;
  logic [31:0]     p_addr = 0, p_wdata = 0;
  logic [3:0]      p_strb = 0;
  logic [2:0]      p_prot = 0;
  task automatic report(input string rule, input string msg);
    broken++;
    $display("RULE %0s broken at edge %0d: %0s", rule, edge_n, msg);
  endtask
  task automatic note(input string rule, input string msg);
    notes++;
    $display("NOTE %0s at edge %0d: %0s", rule, edge_n, msg);
  endtask
  always @(posedge pclk) begin
    logic any, en, rdy, bytes_x;
    edge_n = edge_n + 1;
    any = (|psel) === 1'b1; en = (penable === 1'b1); rdy = (pready === 1'b1);
    if (presetn !== 1'b1) begin
      if (any) report("reset", "PSEL is 1 while PRESETn is low");
      any = 0; en = 0;
    end else begin
      bytes_x = 0;
      for (int l = 0; l < 4; l++) if (pstrb[l] === 1'b1 && (^pwdata[8*l +: 8]) === 1'bx) bytes_x = 1;
      if ((^psel) === 1'bx) report("known", "PSEL is x");
      else if (any && (^{paddr, pwrite, penable, pstrb, pprot}) === 1'bx) report("known", "a control signal is x while PSEL is 1");
      else if (any && pwrite === 1'b1 && bytes_x) report("known", "a strobed byte of PWDATA is x");
      else if (any && en && pready === 1'bx) report("known", "PREADY is x in an access cycle");
      if (any && en && !(p_any && (!p_en || !p_rdy)))
        report("setup", "PENABLE is 1 without a setup cycle (PSEL 1, PENABLE 0) before it");
      if (p_any && !p_en && !(any && en))
        report("setup_len", "the setup cycle was not followed by an access cycle");
      if (p_any && (!p_en || !p_rdy) && any) begin
        if (psel !== p_sel) report("stable", $sformatf("PSEL changed from %b to %b inside a transfer", p_sel, psel));
        if (paddr !== p_addr) report("stable", $sformatf("PADDR changed from 0x%0h to 0x%0h inside a transfer", p_addr, paddr));
        if (pwrite !== p_wr) report("stable", "PWRITE changed inside a transfer");
        if (pprot !== p_prot) report("stable", "PPROT changed inside a transfer");
        if (pstrb !== p_strb) report("stable", "PSTRB changed inside a transfer");
        if (p_wr && pwdata !== p_wdata) report("stable", $sformatf("PWDATA changed from 0x%0h to 0x%0h inside a transfer", p_wdata, pwdata));
        if (p_en && !en) report("stable", "PENABLE dropped before PREADY");
      end
      if (p_any && p_en && !p_rdy && !any) report("stable", "PSEL dropped before PREADY");
      if (p_any && p_en && p_rdy && en) report("enable_drop", "PENABLE is still 1 in the cycle after a completed transfer");
      if (any && pwrite === 1'b0 && pstrb !== 4'b0000) report("strb_read", $sformatf("PSTRB is %b during a read", pstrb));
      if ($countones(psel) > 1) report("onehot", $sformatf("PSELx is %b: two completers selected", psel));
      if (any && en && rdy) begin
        if (pwrite === 1'b1) writes++; else reads++;
        stall = 0;
      end else if (any && en) begin
        waits++; stall++;
        if (stall == TIMEOUT) report("timeout", $sformatf("no PREADY after %0d wait cycles", TIMEOUT));
      end
      if (pslverr === 1'b1 && !(any && en && rdy))
        note("pslverr", "PSLVERR is 1 outside the completion cycle (ignored by the requester; recommended LOW)");
    end
    p_any = any; p_en = en; p_rdy = rdy; p_sel = psel; p_addr = paddr; p_wr = (pwrite === 1'b1);
    p_wdata = pwdata; p_strb = pstrb; p_prot = pprot;
  end
  final $display("checker: %0d writes, %0d reads, %0d wait edges, %0d rules broken, %0d notes", writes, reads, waits, broken, notes);
endmodule

Read included file: rtl_apb_tasks.svh

// rtl_apb_tasks.svh: the requester side of the bench (read-only).
// apb_write(addr, data, strb) and apb_read(addr) run one transfer: SETUP for one cycle, ACCESS until
// PREADY, then the bus goes idle. Each transfer prints one line at its completion edge:
//   e3-e4 WRITE 0x00000000 data=0x0000005a strb=1111 waits=0 OKAY
// (setup edge - completion edge). A read leaves its data in prdata_q and its error in pslverr_q.
// quiet = 1 turns the log off; watch = 1 prints the bus at every edge.
// cycle is the number of the last rising edge; it updates after the edge's active events, so at an
// edge every reader (the tasks, the watch line) sees the same old value and names the edge cycle + 1.
int cycle = -1;
always @(posedge pclk) cycle <= cycle + 1;
bit quiet = 0, watch = 0;
logic [31:0] prdata_q = 0;
logic        pslverr_q = 0;
int          waits_q = 0, give_up = 24;
always @(posedge pclk) if (watch)
  $display("wave e%0d: psel=%b penable=%b pwrite=%b paddr=%h pwdata=%h pstrb=%b pready=%b prdata=%h pslverr=%b",
           cycle + 1, psel, penable, pwrite, paddr, pwdata, pstrb, pready, prdata, pslverr);

task automatic apb_xfer(input bit wr, input logic [31:0] addr, input logic [31:0] data, input logic [3:0] strb);
  int s_edge, w;
  paddr = addr; pwrite = wr; pwdata = wr ? data : 32'h0; pstrb = wr ? strb : 4'h0; pprot = 3'b000;
  psel = 1; penable = 0;                       // SETUP
  s_edge = cycle + 1;
  @(posedge pclk); #1 penable = 1;             // ACCESS
  w = 0;
  @(posedge pclk);
  while (pready !== 1'b1 && w < give_up) begin w++; @(posedge pclk); end
  prdata_q = prdata; pslverr_q = pslverr; waits_q = w;
  if (!quiet) begin
    if (wr && pslverr)  $display("e%0d-e%0d WRITE 0x%08h data=0x%08h strb=%b waits=%0d ERROR", s_edge, cycle + 1, addr, data, strb, w);
    else if (wr)        $display("e%0d-e%0d WRITE 0x%08h data=0x%08h strb=%b waits=%0d OKAY", s_edge, cycle + 1, addr, data, strb, w);
    else if (pslverr)   $display("e%0d-e%0d READ  0x%08h data=0x%08h waits=%0d ERROR", s_edge, cycle + 1, addr, prdata, w);
    else                $display("e%0d-e%0d READ  0x%08h data=0x%08h waits=%0d OKAY", s_edge, cycle + 1, addr, prdata, w);
  end
  #1 psel = 0; penable = 0; pwrite = 0; pstrb = 0; pwdata = 0;   // idle
endtask
task automatic apb_write(input logic [31:0] addr, input logic [31:0] data, input logic [3:0] strb = 4'hf);
  apb_xfer(1, addr, data, strb);
endtask
task automatic apb_read(input logic [31:0] addr);
  apb_xfer(0, addr, 32'h0, 4'h0);
endtask
task automatic idle(input int n);
  repeat (n) @(posedge pclk); #1;
endtask

Spot the bugEach option is the condition done under which the completer acts and reports PSLVERR, with WAITS = 1. The bench writes LEDS twice, then expects COUNT = 2 and LEDS = 0x22. Which condition fails?

(st != IDLE) && pready

psel && penable

(st == ACCESS) && pready

psel && penable && pready

Read included file: apb_checker.sv

`timescale 1ns/1ns
// apb_checker.sv: the rules of an APB interface, checked at every rising edge (read-only).
//   reset        PSEL is 1 while PRESETn is low;
//   known        PSEL is x, or a control signal, a strobed write byte or PREADY in access is x;
//   setup        PENABLE is 1 without a setup cycle (PSEL 1, PENABLE 0) before it;
//   setup_len    a setup cycle not followed by an access cycle;
//   stable       PSEL, PADDR, PWRITE, PPROT, PSTRB or the write data changed inside a transfer,
//                or PSEL or PENABLE dropped before PREADY;
//   enable_drop  PENABLE still 1 in the cycle after a completed transfer;
//   strb_read    PSTRB not zero during a read;
//   onehot       more than one PSELx at once;
//   timeout      no PREADY after TIMEOUT wait cycles (a liveness watchdog, not a protocol rule).
// A NOTE (not a broken rule): PSLVERR is 1 outside the completion cycle, which the spec allows
// and recommends against (IHI 0024E 3.4).
module apb_checker #(parameter int NSEL = 1, parameter int TIMEOUT = 16) (
  input logic            pclk,
  input logic            presetn,
  input logic [NSEL-1:0] psel,
  input logic            penable,
  input logic            pwrite,
  input logic [31:0]     paddr,
  input logic [31:0]     pwdata,
  input logic [3:0]      pstrb,
  input logic [2:0]      pprot,
  input logic            pready,
  input logic            pslverr);
  int broken = 0, notes = 0, writes = 0, reads = 0, waits = 0, edge_n = -1, stall = 0;
  logic            p_any = 0, p_en = 0, p_rdy = 0, p_wr = 0;
  logic [NSEL-1:0] p_sel = 0;
  logic [31:0]     p_addr = 0, p_wdata = 0;
  logic [3:0]      p_strb = 0;
  logic [2:0]      p_prot = 0;
  task automatic report(input string rule, input string msg);
    broken++;
    $display("RULE %0s broken at edge %0d: %0s", rule, edge_n, msg);
  endtask
  task automatic note(input string rule, input string msg);
    notes++;
    $display("NOTE %0s at edge %0d: %0s", rule, edge_n, msg);
  endtask
  always @(posedge pclk) begin
    logic any, en, rdy, bytes_x;
    edge_n = edge_n + 1;
    any = (|psel) === 1'b1; en = (penable === 1'b1); rdy = (pready === 1'b1);
    if (presetn !== 1'b1) begin
      if (any) report("reset", "PSEL is 1 while PRESETn is low");
      any = 0; en = 0;
    end else begin
      bytes_x = 0;
      for (int l = 0; l < 4; l++) if (pstrb[l] === 1'b1 && (^pwdata[8*l +: 8]) === 1'bx) bytes_x = 1;
      if ((^psel) === 1'bx) report("known", "PSEL is x");
      else if (any && (^{paddr, pwrite, penable, pstrb, pprot}) === 1'bx) report("known", "a control signal is x while PSEL is 1");
      else if (any && pwrite === 1'b1 && bytes_x) report("known", "a strobed byte of PWDATA is x");
      else if (any && en && pready === 1'bx) report("known", "PREADY is x in an access cycle");
      if (any && en && !(p_any && (!p_en || !p_rdy)))
        report("setup", "PENABLE is 1 without a setup cycle (PSEL 1, PENABLE 0) before it");
      if (p_any && !p_en && !(any && en))
        report("setup_len", "the setup cycle was not followed by an access cycle");
      if (p_any && (!p_en || !p_rdy) && any) begin
        if (psel !== p_sel) report("stable", $sformatf("PSEL changed from %b to %b inside a transfer", p_sel, psel));
        if (paddr !== p_addr) report("stable", $sformatf("PADDR changed from 0x%0h to 0x%0h inside a transfer", p_addr, paddr));
        if (pwrite !== p_wr) report("stable", "PWRITE changed inside a transfer");
        if (pprot !== p_prot) report("stable", "PPROT changed inside a transfer");
        if (pstrb !== p_strb) report("stable", "PSTRB changed inside a transfer");
        if (p_wr && pwdata !== p_wdata) report("stable", $sformatf("PWDATA changed from 0x%0h to 0x%0h inside a transfer", p_wdata, pwdata));
        if (p_en && !en) report("stable", "PENABLE dropped before PREADY");
      end
      if (p_any && p_en && !p_rdy && !any) report("stable", "PSEL dropped before PREADY");
      if (p_any && p_en && p_rdy && en) report("enable_drop", "PENABLE is still 1 in the cycle after a completed transfer");
      if (any && pwrite === 1'b0 && pstrb !== 4'b0000) report("strb_read", $sformatf("PSTRB is %b during a read", pstrb));
      if ($countones(psel) > 1) report("onehot", $sformatf("PSELx is %b: two completers selected", psel));
      if (any && en && rdy) begin
        if (pwrite === 1'b1) writes++; else reads++;
        stall = 0;
      end else if (any && en) begin
        waits++; stall++;
        if (stall == TIMEOUT) report("timeout", $sformatf("no PREADY after %0d wait cycles", TIMEOUT));
      end
      if (pslverr === 1'b1 && !(any && en && rdy))
        note("pslverr", "PSLVERR is 1 outside the completion cycle (ignored by the requester; recommended LOW)");
    end
    p_any = any; p_en = en; p_rdy = rdy; p_sel = psel; p_addr = paddr; p_wr = (pwrite === 1'b1);
    p_wdata = pwdata; p_strb = pstrb; p_prot = pprot;
  end
  final $display("checker: %0d writes, %0d reads, %0d wait edges, %0d rules broken, %0d notes", writes, reads, waits, broken, notes);
endmodule

Read included file: rtl_apb_tasks.svh

// rtl_apb_tasks.svh: the requester side of the bench (read-only).
// apb_write(addr, data, strb) and apb_read(addr) run one transfer: SETUP for one cycle, ACCESS until
// PREADY, then the bus goes idle. Each transfer prints one line at its completion edge:
//   e3-e4 WRITE 0x00000000 data=0x0000005a strb=1111 waits=0 OKAY
// (setup edge - completion edge). A read leaves its data in prdata_q and its error in pslverr_q.
// quiet = 1 turns the log off; watch = 1 prints the bus at every edge.
// cycle is the number of the last rising edge; it updates after the edge's active events, so at an
// edge every reader (the tasks, the watch line) sees the same old value and names the edge cycle + 1.
int cycle = -1;
always @(posedge pclk) cycle <= cycle + 1;
bit quiet = 0, watch = 0;
logic [31:0] prdata_q = 0;
logic        pslverr_q = 0;
int          waits_q = 0, give_up = 24;
always @(posedge pclk) if (watch)
  $display("wave e%0d: psel=%b penable=%b pwrite=%b paddr=%h pwdata=%h pstrb=%b pready=%b prdata=%h pslverr=%b",
           cycle + 1, psel, penable, pwrite, paddr, pwdata, pstrb, pready, prdata, pslverr);

task automatic apb_xfer(input bit wr, input logic [31:0] addr, input logic [31:0] data, input logic [3:0] strb);
  int s_edge, w;
  paddr = addr; pwrite = wr; pwdata = wr ? data : 32'h0; pstrb = wr ? strb : 4'h0; pprot = 3'b000;
  psel = 1; penable = 0;                       // SETUP
  s_edge = cycle + 1;
  @(posedge pclk); #1 penable = 1;             // ACCESS
  w = 0;
  @(posedge pclk);
  while (pready !== 1'b1 && w < give_up) begin w++; @(posedge pclk); end
  prdata_q = prdata; pslverr_q = pslverr; waits_q = w;
  if (!quiet) begin
    if (wr && pslverr)  $display("e%0d-e%0d WRITE 0x%08h data=0x%08h strb=%b waits=%0d ERROR", s_edge, cycle + 1, addr, data, strb, w);
    else if (wr)        $display("e%0d-e%0d WRITE 0x%08h data=0x%08h strb=%b waits=%0d OKAY", s_edge, cycle + 1, addr, data, strb, w);
    else if (pslverr)   $display("e%0d-e%0d READ  0x%08h data=0x%08h waits=%0d ERROR", s_edge, cycle + 1, addr, prdata, w);
    else                $display("e%0d-e%0d READ  0x%08h data=0x%08h waits=%0d OKAY", s_edge, cycle + 1, addr, prdata, w);
  end
  #1 psel = 0; penable = 0; pwrite = 0; pstrb = 0; pwdata = 0;   // idle
endtask
task automatic apb_write(input logic [31:0] addr, input logic [31:0] data, input logic [3:0] strb = 4'hf);
  apb_xfer(1, addr, data, strb);
endtask
task automatic apb_read(input logic [31:0] addr);
  apb_xfer(0, addr, 32'h0, 4'h0);
endtask
task automatic idle(input int n);
  repeat (n) @(posedge pclk); #1;
endtask

ConceptThe completer that acted in SETUP passed the protocol checker with zero rules broken. Why did the checker not catch it?

The checker was built for a completer with a different WAITS value, so its timing rules did not apply hereThe checker checks the requester side of the bus only, never a completer, so a completer bug cannot fire a ruleThe checker watches the wires, which obeyed the protocol; the double action showed only in a register valueActing in the setup cycle is allowed by the specification, so there was no rule for the checker to report

Read the waveformIn the measured trace of the hook, with WAITS = 1, at which edge does the write to LEDS complete?

The write with one wait state

Values sampled at edges 2 to 7, from left to right.

Edge 3, the setup edge, where the address is decodedEdge 4, the first ACCESS edge, where PENABLE risesEdge 6, where leds first shows 5aEdge 5, where PSEL, PENABLE and PREADY are all 1

ConceptA completer keeps the APB state in a register, moving IDLE to SETUP when it samples PSEL high and SETUP to ACCESS at the next edge. Compared with decoding the state from the wires, what changes?

The registered copy is a cycle late, so every transfer gets one extra wait cycleThe registered copy is a cycle early, so the completer acts during the setup cycleThe registered copy breaks the setup rule and the checker reports itNothing: the registered copy and the wires always agree in the same cycle

Remember

The APB operating states are visible on PSEL and PENABLE; a completer decodes IDLE, SETUP and ACCESS from the wires instead of storing them.
A transfer completes at exactly one edge, where PSEL, PENABLE and PREADY are all 1; every side effect, PSLVERR and the read of PRDATA belong to that edge.
A registered copy of the state is one cycle late and adds a wait cycle to every transfer; acting whenever selected acts at least twice per write.
The protocol checker proves the wires, not the peripheral: a wrong count shows in a read, so the bench reads back what it wrote.