Skip to content

Commit

Permalink
[i2c] Harmonize fault cases with multi-controller
Browse files Browse the repository at this point in the history 
Remove SDA interference triggering during SCL low, and fold the three
fault cases into multi-controller types:

- SDA interference represents lost arbitration due to the controller
  "transmitting" high SDA, but seeing low while SCL is high.
- SDA unstable represents unexpected Stop or Start conditions occuring
  while SCL is high.
- SCL interference is simply clock synchronization in a multi-controller
  environment. In single-controller environments, it's not expected, but
  in multi-controller cases, the interrupt should be disabled.

Signed-off-by: Alexander Williams <[email protected]>
  • Loading branch information
@a-will
a-will committed May 15, 2024
1 parent e43ed46 commit 2a7e193
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 40 deletions.
68 changes: 33 additions & 35 deletions hw/ip/i2c/rtl/i2c_controller_fsm.sv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ module i2c_controller_fsm import i2c_pkg::*;
input [12:0] tsu_sta_i, // setup time for repeated START in clock units
input [12:0] tsu_sto_i, // setup time for STOP in clock units
input [12:0] thd_dat_i, // data hold time in clock units
input arbitration_lost_i, // High when bus arbitration has been lost.
input sda_interference_i, // High when SCL is high and SDA doesn't match while transmitting
input [29:0] stretch_timeout_i, // max time target connected to this host may stretch the clock
input timeout_enable_i, // assert if target stretches clock past max
input [30:0] host_nack_handler_timeout_i, // Timeout threshold for unhandled Host-Mode 'nak' irq.
Expand All @@ -57,7 +57,6 @@ module i2c_controller_fsm import i2c_pkg::*;
output logic event_unhandled_nak_timeout_o, // SW didn't handle the NACK in time
output logic event_arbitration_lost_o, // Lost arbitration after beginning a transaction
output logic event_scl_interference_o, // other device forcing SCL low
output logic event_sda_interference_o, // other device forcing SDA low
output logic event_stretch_timeout_o, // target stretches clock past max time
output logic event_sda_unstable_o, // SDA is not constant during SCL pulse
output logic event_cmd_complete_o // Command is complete
Expand Down Expand Up @@ -271,14 +270,16 @@ module i2c_controller_fsm import i2c_pkg::*;
end
end

assign event_arbitration_lost_o = event_sda_unstable_o || sda_interference_i;

// Registers whether a transaction start has been observed.
// A transaction start does not include a "restart", but rather
// the first start after enabling i2c, or a start observed after a
// stop.
always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
trans_started <= '0;
end else if (trans_started && (!host_enable_i || arbitration_lost_i)) begin
end else if (trans_started && (!host_enable_i || event_arbitration_lost_o)) begin
trans_started <= '0;
end else if (log_start) begin
trans_started <= 1'b1;
Expand Down Expand Up @@ -331,17 +332,6 @@ module i2c_controller_fsm import i2c_pkg::*;
state_e state_q, state_d;


// enable sda interference detection
// Detects when the controller releases sda to be pulled high, but the line
// is unexpectedly held low by another driver.
logic en_sda_interf_det;

// Report SDA interference when the host function tries to drive a 1 but observes a 0 instead.
//
// When the count is reached, we are pass the rise time period.
// Now check for any inconsistency in the sda value.
assign event_sda_interference_o = (en_sda_interf_det & arbitration_lost_i);

// Increment the NACK timeout count if the controller is halted in Idle and
// the timeout hasn't yet occurred.
assign incr_nak_cnt = unhandled_unexp_nak_i && host_enable_i && (state_q == Idle) &&
Expand All @@ -357,7 +347,6 @@ module i2c_controller_fsm import i2c_pkg::*;
rx_fifo_wvalid_o = 1'b0;
rx_fifo_wdata_o = RX_FIFO_WIDTH'(0);
event_nak_o = 1'b0;
event_arbitration_lost_o = 1'b0;
event_scl_interference_o = 1'b0;
event_sda_unstable_o = 1'b0;
event_cmd_complete_o = 1'b0;
Expand Down Expand Up @@ -421,7 +410,10 @@ module i2c_controller_fsm import i2c_pkg::*;
transmitting_o = 1'b1;
stretch_en = 1'b1;
if (scl_i_q && !scl_i) event_scl_interference_o = 1'b1;
if (sda_i_q != sda_i) event_sda_unstable_o = 1'b1;
if (scl_i_q && scl_i && (sda_i_q != sda_i)) begin
// Unexpected Stop / Start
event_sda_unstable_o = 1'b1;
end
end
// HoldBit: SCL is pulled low
HoldBit : begin
Expand All @@ -444,7 +436,10 @@ module i2c_controller_fsm import i2c_pkg::*;
if (!scl_i_q && scl_i && sda_i && !fmt_flag_nak_ok_i) event_nak_o = 1'b1;
stretch_en = 1'b1;
if (scl_i_q && !scl_i) event_scl_interference_o = 1'b1;
if (sda_i_q != sda_i) event_sda_unstable_o = 1'b1;
if (scl_i_q && scl_i && (sda_i_q != sda_i)) begin
// Unexpected Stop / Start
event_sda_unstable_o = 1'b1;
end
end
// HoldDevAck: SCL is pulled low
HoldDevAck : begin
Expand All @@ -464,7 +459,10 @@ module i2c_controller_fsm import i2c_pkg::*;
scl_d = 1'b1;
stretch_en = 1'b1;
if (scl_i_q && !scl_i) event_scl_interference_o = 1'b1;
if (sda_i_q != sda_i) event_sda_unstable_o = 1'b1;
if (scl_i_q && scl_i && (sda_i_q != sda_i)) begin
// Unexpected Stop / Start
event_sda_unstable_o = 1'b1;
end
end
// ReadHoldBit: SCL is pulled low
ReadHoldBit : begin
Expand Down Expand Up @@ -497,7 +495,10 @@ module i2c_controller_fsm import i2c_pkg::*;
transmitting_o = 1'b1;
stretch_en = 1'b1;
if (scl_i_q && !scl_i) event_scl_interference_o = 1'b1;
if (sda_i_q != sda_i) event_sda_unstable_o = 1'b1;
if (scl_i_q && scl_i && (sda_i_q != sda_i)) begin
// Unexpected Stop / Start
event_sda_unstable_o = 1'b1;
end
end
// HostHoldBitAck: SCL is pulled low
HostHoldBitAck : begin
Expand Down Expand Up @@ -562,10 +563,6 @@ module i2c_controller_fsm import i2c_pkg::*;
event_cmd_complete_o = 1'b0;
end
endcase // unique case (state_q)

if (trans_started && arbitration_lost_i) begin
event_arbitration_lost_o = 1'b1;
end
end

// Conditional state transition
Expand All @@ -582,7 +579,6 @@ module i2c_controller_fsm import i2c_pkg::*;
log_start = 1'b0;
log_stop = 1'b0;
req_restart = 1'b0;
en_sda_interf_det = 1'b0;
auto_stop_d = auto_stop_q;

unique case (state_q)
Expand Down Expand Up @@ -654,7 +650,6 @@ module i2c_controller_fsm import i2c_pkg::*;
end
// ClockLow: SCL stays low, shift indexed bit onto SDA
ClockLow : begin
en_sda_interf_det = 1'b1;
if (tcount_q == 20'd1) begin
load_tcount = 1'b1;
if (pend_restart) begin
Expand All @@ -668,11 +663,13 @@ module i2c_controller_fsm import i2c_pkg::*;
end
// ClockPulse: SCL is released, SDA keeps the indexed bit value
ClockPulse : begin
en_sda_interf_det = 1'b1;
if (!scl_i && !scl_i_q && stretch_predict_cnt_expired) begin
// Saw stretching. Remain in this state and don't count down until we see SCL high.
load_tcount = 1'b1;
tcount_sel = tClockHigh;
end else if (scl_i_q && scl_i && (sda_i_q != sda_i)) begin
// Unexpected Stop / Start
state_d = Idle;
end else if (tcount_q == 20'd1 || (!scl_i && scl_i_q)) begin
// Transition either when we finish counting our high period or
// another controller pulls clock low.
Expand All @@ -683,9 +680,7 @@ module i2c_controller_fsm import i2c_pkg::*;
end
// HoldBit: SCL is pulled low
HoldBit : begin
en_sda_interf_det = 1'b1;
if (tcount_q == 20'd1) begin
en_sda_interf_det = 1'b0;
load_tcount = 1'b1;
tcount_sel = tClockLow;
if (bit_index == '0) begin
Expand All @@ -712,6 +707,9 @@ module i2c_controller_fsm import i2c_pkg::*;
// Saw stretching. Remain in this state and don't count down until we see SCL high.
load_tcount = 1'b1;
tcount_sel = tClockHigh;
end else if (scl_i_q && scl_i && (sda_i_q != sda_i)) begin
// Unexpected Stop / Start
state_d = Idle;
end else begin
if (tcount_q == 20'd1 || (!scl_i && scl_i_q)) begin
state_d = HoldDevAck;
Expand Down Expand Up @@ -748,6 +746,9 @@ module i2c_controller_fsm import i2c_pkg::*;
// Saw stretching. Remain in this state and don't count down until we see SCL high.
load_tcount = 1'b1;
tcount_sel = tClockHigh;
end else if (scl_i_q && scl_i && (sda_i_q != sda_i)) begin
// Unexpected Stop / Start
state_d = Idle;
end else if (tcount_q == 20'd1 || (!scl_i && scl_i_q)) begin
state_d = ReadHoldBit;
load_tcount = 1'b1;
Expand All @@ -773,7 +774,6 @@ module i2c_controller_fsm import i2c_pkg::*;
// HostClockLowAck: SCL is pulled low, SDA is conditional based on
// byte position
HostClockLowAck : begin
en_sda_interf_det = 1'b1;
if (tcount_q == 20'd1) begin
state_d = HostClockPulseAck;
load_tcount = 1'b1;
Expand All @@ -782,11 +782,13 @@ module i2c_controller_fsm import i2c_pkg::*;
end
// HostClockPulseAck: SCL is released
HostClockPulseAck : begin
en_sda_interf_det = 1'b1;
if (!scl_i && !scl_i_q && stretch_predict_cnt_expired) begin
// Saw stretching. Remain in this state and don't count down until we see SCL high.
load_tcount = 1'b1;
tcount_sel = tClockHigh;
end else if (scl_i_q && scl_i && (sda_i_q != sda_i)) begin
// Unexpected Stop / Start
state_d = Idle;
end else if (tcount_q == 20'd1 || (!scl_i && scl_i_q)) begin
state_d = HostHoldBitAck;
load_tcount = 1'b1;
Expand All @@ -795,9 +797,7 @@ module i2c_controller_fsm import i2c_pkg::*;
end
// HostHoldBitAck: SCL is pulled low
HostHoldBitAck : begin
en_sda_interf_det = 1'b1;
if (tcount_q == 20'd1) begin
en_sda_interf_det = 1'b0;
if (byte_index == 9'd1) begin
if (fmt_flag_stop_after_i) begin
state_d = ClockStop;
Expand Down Expand Up @@ -833,9 +833,7 @@ module i2c_controller_fsm import i2c_pkg::*;
end
// HoldStop: SDA and SCL are released
HoldStop : begin
en_sda_interf_det = 1'b1;
if (scl_i && sda_i) begin
en_sda_interf_det = 1'b0;
auto_stop_d = 1'b0;
if (auto_stop_q) begin
// If this Stop symbol was generated automatically, go back to Idle.
Expand Down Expand Up @@ -903,7 +901,7 @@ module i2c_controller_fsm import i2c_pkg::*;
end
endcase // unique case (state_q)

if (trans_started && arbitration_lost_i) begin
if (trans_started && sda_interference_i) begin
state_d = Idle;
end
end
Expand Down
19 changes: 14 additions & 5 deletions hw/ip/i2c/rtl/i2c_core.sv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ module i2c_core import i2c_pkg::*;
logic bus_event_detect;
logic [10:0] bus_event_detect_cnt;
logic sda_released_but_low;
logic controller_arbitration_lost;
logic controller_sda_interference;
logic target_arbitration_lost;

logic bus_free;
Expand Down Expand Up @@ -474,16 +474,24 @@ module i2c_core import i2c_pkg::*;
// are allowed to propagate. The rise time is used here because it
// should be the longer value. The (-1) term here is to account for the
// delay in the counter.
// Note that there are limits to this method of detecting arbitration.
// A separate, buggy device that drives clock low faster than the counter
// can expire would not trigger loss of arbitration.
bus_event_detect_cnt <= reg2hw.timing1.t_r.q + 10'(RoundTripCycles - 1);
end else if (bus_event_detect_cnt != '0) begin
bus_event_detect_cnt <= bus_event_detect_cnt - 1'b1;
end
end
assign bus_event_detect = (bus_event_detect_cnt == '0);
assign sda_released_but_low = bus_event_detect && scl_sync && (sda_fsm_q != sda_sync);
assign controller_arbitration_lost = controller_transmitting && sda_released_but_low;
// What about unexpected start / stop on the bits that are read?
assign controller_sda_interference = controller_transmitting && sda_released_but_low;
assign target_arbitration_lost = target_transmitting && sda_released_but_low;

assign event_sda_interference = controller_sda_interference;

// The bus monitor detects starts, stops, and bus timeouts. It also reports
// when the bus is free for the controller to transmit.
i2c_bus_monitor u_i2c_bus_monitor (
.clk_i,
.rst_ni,
Expand Down Expand Up @@ -551,7 +559,7 @@ module i2c_core import i2c_pkg::*;
.tsu_sta_i (tsu_sta),
.tsu_sto_i (tsu_sto),
.thd_dat_i (thd_dat),
.arbitration_lost_i (controller_arbitration_lost),
.sda_interference_i (controller_sda_interference),
.stretch_timeout_i (bus_active_timeout),
.timeout_enable_i (stretch_timeout_enable),
.host_nack_handler_timeout_i (host_nack_handler_timeout),
Expand All @@ -560,7 +568,6 @@ module i2c_core import i2c_pkg::*;
.event_unhandled_nak_timeout_o (event_unhandled_nak_timeout),
.event_arbitration_lost_o (event_controller_arbitration_lost),
.event_scl_interference_o (event_scl_interference),
.event_sda_interference_o (event_sda_interference),
.event_stretch_timeout_o (event_stretch_timeout),
.event_sda_unstable_o (event_sda_unstable),
.event_cmd_complete_o (event_controller_cmd_complete)
Expand Down Expand Up @@ -871,8 +878,10 @@ module i2c_core import i2c_pkg::*;
// ASSERTIONS //
////////////////

// TODO: Decide whether to keep this assertion. It is primarily checking the
// testbench, not the IP, due to the CDC cycle deletion.
// Check to make sure scl_i is never a single cycle glitch
`ASSERT(SclInputGlitch_A, $rose(scl_sync) |-> ##1 scl_sync)
// `ASSERT(SclInputGlitch_A, $rose(scl_sync) |-> ##1 scl_sync)

`ASSERT_INIT(FifoDepthValid_A, FifoDepth > 0 && FifoDepthW <= MaxFifoDepthW)
`ASSERT_INIT(AcqFifoDepthValid_A, AcqFifoDepth > 0 && AcqFifoDepthW <= MaxFifoDepthW)
Expand Down

0 comments on commit 2a7e193

Please sign in to comment.