lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.49k stars 742 forks source link

[i2c] Noise on SDA/SCL #17456

Open marnovandermaas opened 1 year ago

marnovandermaas commented 1 year ago

Description

Issue raised by @zi-v and written up by @GregAC

Just following up from the discussion we had in the I2C meeting and Ziv's concerns about noise on SDA/SCL leading to malfunctions.

I did a bit of background reading and the I2C spec has some recommendations/requirements here.

  1. A schmitt trigger is used on the SDA/SCL inputs
  2. The logic incorporates spike suppression, a pulse of up to 50ns must be suppressed by input filtering for Fast-mode (400 kbps) and Fast-mode Plus (1 mbps) (Table 10. Characteristics of the SDA and SCL I/O stages section 6.1)

I note a schmitt trigger enable is already part of the standard set of pad attributes in pinmux, though of course it still needs to be physically implemented in the pad itself.

For the pulse suppression we could sample on every clock (24 MHz Peripheral Domain) and then only change the registered SDA/SCL being fed to the i2c core when all samples were 0 or 1. For 50 ns pulse suppression at 24 MHz (~42ms period) we'd only need 2 samples, we could make something configurable to use 4/3/2 samples for instance to give us some flexibility. This will be simple to implement and should have minimal impact on verification.

Three questions:

  1. Will schmitt triggers be implemented by the pads and usable for I2C?
  2. Does the spike suppression scheme described above sound reasonable?
  3. Are there any other concerns here?
zi-v commented 1 year ago

@GregAC , thanks for looking into this. I looked at the pinout and IOs specification and it seems that the IOs which Google have designated for I2C have internal 50ns glitch suppression filter (hard wired) and a configurable Schmitt trigger option. So, as long as we select those IOs by pin mux and make sure the Schmitt trigger option is activated, we are good.

The pinout is described at: https://opentitan.org/book/hw/top_earlgrey/doc/design/index.html The I2C compatible IO type is BidirOd

GregAC commented 1 year ago

Thanks @zi-v.

In which case no changes required here for M2.5. I'll leave this tagged as future release as the proposed spike suppression logic may be useful in other uses of the IP or OpenTitan integratations where the Schmitt Trigger/Glitch suppression filter is not available.

msfschaffner commented 7 months ago

Given that we are using the same pad configuration and pad cells for Earlgrey production silicon, I don't think this change is needed. Putting into the backlog again.