opentitan icon indicating copy to clipboard operation
opentitan copied to clipboard

[i2c] Add multi-controller support

Open a-will opened this issue 1 year ago • 4 comments

Add support for...

  • Bus arbitration for both the controller (multi-controller environments) and the target (broadcast commands)
  • Clock synchronization
  • Simultaneously active host and target modules

Change fault reporting (SDA interference, SDA instability, SCL interference) to harmonize with multi-controller support.

  • "SDA interference" no longer triggers from SDA being 0 at the first controller enablement. Just read the RX Oversample CSR instead.
  • "SCL interference" should be ignored in multi-controller environments, as this is simply clock synchronization. Leave the interrupt masked.

Add clock stretching and lost arbitration detection for control symbols. These were overlooked, and potential clock stretching applies even in single-controller environments.

Fix up "fault" injection timing so pulses do not get deleted by CDC instrumentation and so they don't trigger assertions.

Builds on https://github.com/lowRISC/opentitan/pull/22865. The last 9 commits are new, but the very last commit will likely be dropped.

a-will avatar Apr 29 '24 03:04 a-will

I'm not sure what's going on with the Xcelium tests. They don't seem to actually run.

a-will avatar May 07 '24 04:05 a-will

Note that DV pass rates are maintained. The last commit is there just to show that I've tested responses to multi-controller "fault" conditions while the target module was enabled. We can delete that commit and have proper sequences for simultaneous operation.

Technically, the i2c controller module should even be able to communicate with its own sibling target module, but that hasn't been tested. ;)

a-will avatar May 07 '24 21:05 a-will

Here's how the FPGA utilization numbers look compared to before the split FSM PR:

branch flops luts
before-split 1029 1612
this-pr 1040 1715

a-will avatar May 08 '24 17:05 a-will

I made a guess that Xcelium poorly handles dependency tracking between always_comb blocks and created an infinite loop that prevented forward progress with simulation. Let's see if moving log_start and log_stop into the same always_comb block as ctrl_symbol_failed brings the Xcelium CI jobs to pass.

Edit: That did the trick...

a-will avatar May 08 '24 19:05 a-will

How did you check that the implementation works (at least in the common case)?

Mostly, I made sure that DV regression test (+ top-level test) pass rates did not drop for all the known use cases. I also fixed failures in i2c_host_error_intr when control symbols would get squashed due to the timing of the interference. Finally, I manually checked that the target module's FSM responded appropriately to "interference" cases when it wasn't being addressed--That last commit turns on both FSMs simultaneously.

There are still features from this PR to validate:

  • The full responses to loss of arbitration and clock synchronization should be checked. The sequence would produce the same interference stimulus from i2c_host_error_intr, but it shouldn't reset the full IP as soon as the interrupt goes high. These are events that should not prevent the IP from continuing operation, and checking and working with CONTROLLER_EVENTS is the proper route.
  • Loss of arbitration for the target needs to be checked. This can manifest in a way that is similar to i2c_host_error_intr, but with the checks for proper responses in TARGET_EVENTS and the ACQ FIFO.
  • Controller and target sequences should all be able to pass in a multi-controller configuration (e.g. where both modules are enabled). The only one I've checked so far is i2c_host_error_intr.

Then from prior PRs...

  • Wait for idle after init in multi-controller monitor mode
  • Bus timeouts
  • Target stretch timeout counter accumulation across multiple bits / bytes
  • Stale data in the TX FIFO properly being blocked until software clears the TARGET_EVENTS.TX_PENDING bit

a-will avatar May 10 '24 20:05 a-will

SG, thx @a-will for the elaboration. CC @hcallahan-lowrisc for the test points listed above.

andreaskurth avatar May 13 '24 20:05 andreaskurth