I2C slave debug retrospective: six bugs, two fixes, and a reusable verification method

A supplementary I2C slave simulation found six bugs, the worst hidden by a testbench acting as a friendly master. This note gives the diagnosis, margin accounting and re-verification matrix.

1. Bottom line

The device under test is an I2C slave that oversamples SCL/SDA with a high-frequency clock (~70 MHz) and locates its sampling instants through four stages of synchroniser delay compensation. The original simulation “passed completely”. Supplementary simulation produced six problems and two fixes:

In one sentence: both fixes are one-line changes about when to sample and when to drive. The cost was not in writing them but in first quantifying the behaviour until it proved those lines were wrong.

2. Problem zero: why the original simulation could not see the worst bug

The master BFM of the original testbench keeps driving SDA after every byte, holding it until ACK ends (tb_sda_oe is never released). That is exactly the non-standard “the master keeps the data valid past the SCL falling edge” case, so Bug A never fires.

task i2c_wr_byte(...);
   // wait ack
   tb_scl_o = 1'b0;   // only SCL goes low; SDA is still driven by the master with the last bit
   #T tb_scl_o = 1'b1;

Lesson: a verification environment bakes the DUT’s defects into “normal behaviour”. Before trusting a simulation of a slow serial slave, check three things about the BFM: (1) is SDA released after the 8th bit of each byte; (2) is there any skew between SDA and SCL; (3) is only a single bit period ever tried.

3. The diagnostic environment: parameterise the master, add three probes

The new regression environment turns master timing into four parameters:

Three observation means were what made this qualitative-to-quantitative:

1. Scoreboard: every register write the DUT actually drives (wr_en sampled on its edge) is compared item by item with the expected write sequence — address, data, order, count. “Not a single register was written” then becomes a check result, not an impression.
2. Illegal-drive probe: if sda_oe=1 appears outside the read-data/ACK window, [ILLEGAL-DRIVE] is printed and the case counts as a bus conflict; the counts are per sampled point (the 699 and 88 quoted below are this counter).
3. Sampling-instant probe: prints the exact moment the R/W bit is decided, together with sda_i / d2 / d4 / the bus SDA / whether the master is still driving.

17 cases run with a single ./run.sh, one log per case at logs/{case}.log; EXTRA="+TV=130" sets the master’s t_HD;DAT. (Later, targeted cases were added — an address decode table, restart transactions, a direct read — bringing the full run to 21.)

4. Bug A: one sampling instant, quantify before touching the code

4.1 Locating it

The three places that decide the address R/W bit (the “write” decision, the “read” decision, and the RDDATA continuation) used the raw, unsynchronised, uncompensated input sda_i. The shift register uses sda_i_sync_d4 (four stages of delay compensation) and the address comparison uses d2. Of the three, only sda_i falls in the SCL low period.

4.2 Measured sampling instant

Aligned with the waveform, the R/W decision happens about 112 ns ≈ 4 clock cycles after the SCL falling edge at the slave pin — the fixed delay of “2-stage synchroniser + 2-stage edge detect”. By that time a compliant master has long released SDA (the spec’s lower bound for t_HD;DAT is 0 ns), so the slave samples the released value 1:

• the write address 0x40 is classified as a read (wr_op=0) → after the address byte is ACKed the FSM reaches the register-address state and returns to IDLE;
• every following data byte is NACKed, and not a single register is written;
• every read sequence returns 0xFF (restart detection depends on wr_op, so a misclassified write address means the restart is never recognised).

One trap when reading the numbers: the probe prints “+140.9 ns”, one clock later than the real decision cycle (112 ns), because the probe only triggers once the ACK has updated. Probe time ≠ decision time; margins must be computed on the decision cycle, otherwise the error in that line gets attributed to something else.

4.3 Threshold sweep: turn “a guess” into “a number”

With the master genuinely releasing SDA (TS_BYTE=1), sweeping t_HD;DAT:

The threshold sits between 110 and 115 ns, matching the 112 ns sampling instant exactly.

The key control case: let the master hold SDA throughout (TS_BYTE=0) and sweep t_HD;DAT from 625 down to 0 — everything passes. That rules out the wrong attribution to “rate / bit period / phase”: failure depends only on when the master releases SDA.

4.4 Margin accounting (the most reusable part of this note)

Convert everything in simulation to a bus-side window relative to the SCL falling edge, then compare it against the spec:

The acceptance criterion is not “it passed after the change” but “after the change the slave no longer demands anything the spec does not require of the master”.

4.5 The fix and its re-verification

// round one: R/W decision + RDDATA continuation
194: else if (bit_cnt == 4'd8 && ~sda_i_sync_d4) i2c_ns = RDDATA;
266: else if (cs == SLV_ADDR_WR && bit_cnt == 4'd7 && sda_i_sync_d4 == 1'b0) wr_op <= 1;
282: else if (cs == SLV_ADDR_RD && bit_cnt == 4'd7 && sda_i_sync_d4 == 1'b1) ...
// round two: A0 bit comparison, same sampling instant everywhere
223: if (bit_cnt == 4'd6 && {p_dat[5:0], sda_i_sync_d4} == addr)

Re-verified against the harshest criterion, t_HD;DAT = 0:

5. Bug B: ACKing an address that is not its own — one gate

On the 8th bit of SLV_ADDR_WR, sda_oe/i2c_ack pulled the bus low unconditionally, without testing dev_chk_pass. On a multi-slave bus an unselected device must NACK, otherwise the master concludes “the device exists”.

The fix (one gated term):

if (bit_cnt == 4'd7 && (((cs == SLV_ADDR_WR) && dev_chk_pass) ||
                        cs == REG_ADDR || cs == WRDATA ||
                        cs == RESTART  || cs == SLV_ADDR_RD))
  sda_oe <= 1'b1;

Besides the original foreign-address case, an address decode table was added (DUT 7-bit address 0x20; send START + address one byte at a time and watch ACK):

12/12 correct, including 0x42/0x43, which differ from the slave address by the A0 bit alone. Full regression, 21 cases: 16 PASS, and the 5 failures are exactly the still-unfixed C/D/E/F plus one skew case that is expected to fail — that is what “no regression” looks like as evidence.

One inconsistency left behind: the i2c_ack register itself still pulls low unconditionally (it only selects the FSM branch SLV_ADDR_WR → REG_ADDR/IDLE, it does not drive the bus). Externally the behaviour is now right, but the internal state disagrees with the bus; gating it as well is an optional cleanup, not a bug.

6. The four still open: symptom, mechanism, criterion

For C/D specifically: on a single-slave bus they are only wasteful, but with several devices on the bus they are a real conflict — they drive this slave’s data onto someone else’s address and break access to the other parts.

7. Three non-bug risks noticed along the way

1. sda_o is a purely combinational output (selected directly from the register-file read data); read data is not output-registered → register-file read delay plus routing must fit inside half a bit period. Registering the read data out during SCL low would be safer.
2. One internal read-enable signal is generated but never brought out as a port: dead logic.
3. The ~86 ns delay compensation of the four-stage synchroniser chain is hard-wired to the clock frequency: change the clock from 70 MHz and every sampling constant must be recomputed. Parameterise it, or rewrite the scheme as “sample at the middle of the SCL high period” so it does not depend on an absolute delay — a structural issue, out of scope for these bug fixes.

8. Pitfalls hit along the way (process, not RTL)

9. Reusable checklist (verifying a slow serial slave)

1. Audit the BFM first: is it permanently a “friendly master” (always driving, one rate, no skew)?
2. Parameterise all master timing, and sweep t_HD;DAT / t_SU;DAT down to the spec’s lower bound.
3. All three probes are mandatory: scoreboard (did the write actually land), illegal drive (bus conflict), sampling instant (margin).
4. Write the conclusion as margin accounting: convert simulation instants into a bus-side window relative to the SCL falling edge and compare with the spec; never stop at “it passes after the change”.
5. Every “failure” needs a control case that rules out the most tempting wrong attribution.
6. After each fix, run the related cases plus one full regression, and claim “no regression” with counts — total / PASS / FAIL, every FAIL explained.
7. State explicitly which failures are outside legal master behaviour (e.g. SDA changing while SCL is high is itself a START/STOP condition, not a bug), otherwise they become noise that buries the real issue.

10. Numbers at a glance

11. Glossary

• t_HD;DAT (data hold time): how long data must stay valid after the SCL falling edge; the spec’s lower bound is 0 ns. The central parameter of this note.
• t_SU;DAT (data setup time): how long data must be stable before the SCL rising edge (250 ns class in standard/fast mode).
• current address read: a transaction that reads from the current pointer without sending a register-address byte.
• repeated START: another START without an intervening STOP, to change direction or target.
• combined transaction: write + repeated START + write/read in one transfer; I2C allows it.
• open-drain / wired-AND: the bus has only “pull low” and “release”; devices can only pull low and pull-ups restore the high level.
• oversampling: sampling SCL/SDA with a clock much faster than the bus, locating the sampling instant through a synchroniser chain. It saves a clock domain but ties the sampling instant to the clock frequency.
• 2-stage synchroniser / 4-stage delay compensation: the clock-domain-crossing chain whose depth decides where the sampling instant sits relative to the edge — the source of the margins in this note.
• scoreboard: the checker that compares DUT output against expectations item by item; “did it actually get written” has to be proven by it.
• BFM (bus functional model): the model of the master inside the testbench; its habits decide which DUT defects can be seen at all.