Add I2C target SCL high-spike filtering

CHANGELOG.rst

@@ -1,6 +1,15 @@
 lib_i2c change log
 ==================
 
+UNRELEASED
+----------
+
+  * ADDED:  Option to disable I2C target clock stretching for I3C bus
+    coexistence.
+  * ADDED:   I2C target SCL high-spike filtering for I3C bus coexistence.
+  * CHANGED: Const-ify I2C bulk write buffer argument to avoid XC concurrency
+    errors.
+
 6.4.1
 -----
 

README.rst

@@ -7,9 +7,9 @@ lib_i2c: I²C Library
 
 :vendor: XMOS
 :version: 6.4.1
-:scope: General Use
-:description: I²C controller and peripheral library
-:category: General Purpose
+:scope: General use
+:description: I²C controller and target library
+:category: General purpose
 :keywords: IO
 :devices: xcore.ai, xcore-200
 
@@ -27,21 +27,21 @@ Plus and Ultra-Fast Mode, and allows multiple devices to share the same bus.
 ``lib_i2c`` contains a software defined, industry-standard, I²C library that allows control of an
 I²C bus via `xcore` ports.
 
-``lib_i2c`` provides both controller ("master") and peripheral ("slave") functionality.
+``lib_i2c`` provides both controller ("master") and target ("slave") functionality.
 
-The I²C master component can be used by multiple tasks within the `xcore` device (each addressing
-the same or different peripheral devices).
+The I²C master component can be used by multiple tasks within the `XCORE` device (each addressing
+the same or different target devices).
 
-The library can also be used to implement multiple I²C physical interfaces on a single `xcore`
+The library can also be used to implement multiple I²C physical interfaces on a single `XCORE`
 device simultaneously.
 
 ********
 Features
 ********
 
-* I²C controller (master) and I²C peripheral (slave) modes
+* I²C controller (master) and I²C target (slave) modes
 * Supports speed up to 400 Kb/s (I²C Fast-mode)
-* Clock stretching support
+* Clock stretching support (optional)
 * Synchronous and asynchronous APIs
 
 ************
@@ -55,7 +55,7 @@ Known issues
 Development repo
 ****************
 
-* `https://github.com/xmos/lib_i2c <https://github.com/xmos/lib_i2c>`_
+* `lib_i2c <https://www.github.com/xmos/lib_i2c>`_ (https://www.github.com/xmos/lib_i2c)
 
 **************
 Required tools
@@ -67,7 +67,7 @@ Required tools
 Required libraries (dependencies)
 *********************************
 
-* lib_xassert (www.github.com/xmos/lib_xassert)
+* `lib_xassert <https://www.github.com/xmos/lib_xassert>`_ (https://www.github.com/xmos/xassert)
 
 *************************
 Related application notes
@@ -84,4 +84,5 @@ Support
 *******
 
 This package is supported by XMOS Ltd. Issues can be raised against the software at
-`www.xmos.com/support <https://www.xmos.com/support>`_
+`www.xmos.com/support <https://www.xmos.com/support>`_ or using GitHub `issues <https://github.com/xmos/lib_xassert/issues>`_.
+

lib_i2c/api/i2c.h

@@ -709,10 +709,29 @@ typedef interface i2c_slave_callback_if {
 } i2c_slave_callback_if;
 #endif
 
+/** Enable the I2C slave SCL spike filter.
+ *
+ *  Set to 0 to disable filtering. When enabled, SCL high pulses shorter than
+ *  50 ns are ignored to support coexistence on a mixed I3C/I2C bus.
+ */
+#ifndef I2C_SLAVE_SCL_SPIKE_FILTER
+#define I2C_SLAVE_SCL_SPIKE_FILTER 1
+#endif
+
+/** Enable clock stretching by the I2C slave.
+ *
+ *  Set to 0 to prevent the slave from driving SCL low, as required for
+ *  coexistence on a mixed I3C/I2C bus.
+ */
+#ifndef I2C_SLAVE_CLOCK_STRETCH
+#define I2C_SLAVE_CLOCK_STRETCH 1
+#endif
 
 /** I2C slave task.
  *
  *  This function instantiates an i2c_slave component.
+ *  Define ``I2C_SLAVE_CLOCK_STRETCH`` as 0 when compiling the component to
+ *  prevent it from driving SCL low. Clock stretching is enabled by default.
  *
  *  \param i           the client end of the i2c_slave_callback_if interface. The component
  *                     takes the client end and will make calls on the interface when

lib_i2c/src/i2c_slave.xc

@@ -15,11 +15,32 @@ enum i2c_slave_state {
   MASTER_READ
 };
 
+// I3C mixed-bus SCL spike filter: 50 ns at the 100 MHz reference clock.
+#define T_DIG_H_MIXED_TICKS 5
+
+#if I2C_SLAVE_CLOCK_STRETCH
 static inline void ensure_setup_time()
 {
   // The I2C spec requires a 100ns setup time
   delay_ticks(10);
 }
+#endif
+
+#if I2C_SLAVE_SCL_SPIKE_FILTER
+static inline int scl_high_period_valid(port p_scl, unsigned short start_time)
+{
+  int scl_val;
+  unsigned short sample_time;
+  unsigned short sample_deadline = start_time + T_DIG_H_MIXED_TICKS;
+
+  p_scl :> scl_val @ sample_time;
+  if (porttimeafter(sample_deadline, sample_time)) {
+    p_scl @ sample_deadline :> scl_val;
+  }
+
+  return scl_val;
+}
+#endif
 
 [[combinable]]
 void i2c_slave(client i2c_slave_callback_if i,
@@ -35,12 +56,22 @@ void i2c_slave(client i2c_slave_callback_if i,
   int rw = 0;
   int stop_bit_check = 0;
   int ignore_stop_bit = 1;
+  unsigned short scl_time;
   p_sda when pinseq(1) :> void;
   while (1) {
     select {
     case i.shutdown():
       return;
-    case state != WAITING_FOR_START_OR_STOP => p_scl when pinseq(scl_val) :> void:
+    case state != WAITING_FOR_START_OR_STOP => p_scl when pinseq(scl_val) :> void @ scl_time:
+#if I2C_SLAVE_SCL_SPIKE_FILTER
+      if (scl_val == 1) {
+        if (!scl_high_period_valid(p_scl, scl_time)) {
+          // Short SCL high period; ignore this spike.
+          break;
+        }
+      }
+#endif
+
       switch (state) {
       case READING_ADDR:
         // If clock has gone low, wait for it to go high before doing anything
@@ -76,8 +107,10 @@ void i2c_slave(client i2c_slave_callback_if i,
         break;
 
       case ACK_ADDR:
+#if I2C_SLAVE_CLOCK_STRETCH
         // Stretch clock (hold low) while application code is called
         p_scl <: 0;
+#endif
 
         // Callback to the application to determine whether to ACK
         // or NACK the address.
@@ -105,10 +138,12 @@ void i2c_slave(client i2c_slave_callback_if i,
         scl_val = 1;
         state = ACK_WAIT_HIGH;
 
+#if I2C_SLAVE_CLOCK_STRETCH
         ensure_setup_time();
 
         // Release the clock
         p_scl :> void;
+#endif
         break;
 
       case ACK_WAIT_HIGH:
@@ -156,8 +191,10 @@ void i2c_slave(client i2c_slave_callback_if i,
           // Falling edge, drive data
           if (bitnum < 8) {
             if (bitnum == 0) {
+#if I2C_SLAVE_CLOCK_STRETCH
               // Stretch clock (hold low) while application code is called
               p_scl <: 0;
+#endif
               data = i.master_requires_data();
               // Data is transmitted MSB first
               data = bitrev(data) >> 24;
@@ -170,10 +207,12 @@ void i2c_slave(client i2c_slave_callback_if i,
                 p_sda <: 0;
               }
 
+#if I2C_SLAVE_CLOCK_STRETCH
               ensure_setup_time();
 
               // Release the clock
               p_scl :> void;
+#endif
             } else {
               if (data & 0x1) {
                 p_sda :> void;
@@ -215,8 +254,10 @@ void i2c_slave(client i2c_slave_callback_if i,
           stop_bit_check = 0;
 
           if (bitnum == 8) {
+#if I2C_SLAVE_CLOCK_STRETCH
             // Stretch clock (hold low) while application code is called
             p_scl <: 0;
+#endif
             int ack = i.master_sent_data(data);
             if (ack == I2C_SLAVE_NACK) {
               // Release the data bus so it is pulled high to signal NACK
@@ -227,10 +268,12 @@ void i2c_slave(client i2c_slave_callback_if i,
             }
             state = ACK_WAIT_HIGH;
 
+#if I2C_SLAVE_CLOCK_STRETCH
             ensure_setup_time();
 
             // Release the clock
             p_scl :> void;
+#endif
           }
           scl_val = 1;
         }
@@ -240,11 +283,19 @@ void i2c_slave(client i2c_slave_callback_if i,
 
     case (state == WAITING_FOR_START_OR_STOP) || stop_bit_check =>
             p_sda when pinseq(sda_val) :> void:
+      int val;
+#if I2C_SLAVE_SCL_SPIKE_FILTER
+      p_scl :> val @ scl_time;
+      if (val) {
+        val = scl_high_period_valid(p_scl, scl_time);
+      }
+#else
+      p_scl :> val;
+#endif
+
       if (sda_val == 1) {
         // SDA has transitioned from low to high, if SCL is high
         // then it is a stop bit.
-        int val;
-        p_scl :> val;
         if (val) {
           if (!ignore_stop_bit) {
             i.stop_bit();
@@ -257,9 +308,7 @@ void i2c_slave(client i2c_slave_callback_if i,
       } else {
         // SDA has transitioned from high to low, if SCL is high
         // then it is a start bit.
-        int val;
-        p_scl :> val;
-        if (val == 1) {
+        if (val) {
           state = READING_ADDR;
           bitnum = 0;
           data = 0;

tests/expected/no_clock_stretch.expect

@@ -0,0 +1,18 @@
+Starting read transaction to device id 0x3c
+Sending data 0x79
+xCORE got start of read transaction
+Master received ACK
+xCORE sending: 0xFF
+Received byte 0xff
+Master sending NACK
+Sending stop bit
+xCORE got stop bit
+Starting write transaction to device id 0x3c
+Sending data 0x78
+xCORE got start of write transaction
+Master received ACK
+Sending data 0x33
+xCORE got data: 0x33
+Master received ACK
+Sending data 0xff
+xCORE got data: 0xFF

tests/i2c_slave_checker.py

@@ -9,11 +9,12 @@ class I2CSlaveChecker(px.SimThread):
     """
 
     def __init__(self, scl_port, sda_port, speed,
-                 tsequence):
+                 tsequence, allow_clock_stretch=True):
         self._scl_port = scl_port
         self._sda_port = sda_port
         self._tsequence = tsequence
         self._speed = speed
+        self._allow_clock_stretch = allow_clock_stretch
         self._bit_time = 1000000e6 / speed
         #print("Checking I2C: SCL=%s, SDA=%s" % (self._scl_port, self._sda_port))
 
@@ -39,6 +40,7 @@ def high_pulse(self, xsi):
         self.wait_until(self._fall_time + self._bit_time / 2 + self._bit_time / 32)
         xsi.drive_port_pins(self._scl_port, 1)
         if xsi.is_port_driving(self._scl_port):
+            assert self._allow_clock_stretch, "I2C slave attempted to clock stretch"
             self.wait_for_port_pins_change([self._scl_port])
         new_fall_time = self._fall_time + self._bit_time
         if xsi.get_time() > new_fall_time:
@@ -50,6 +52,7 @@ def high_pulse(self, xsi):
     def high_pulse_sample(self, xsi):
         self.wait_until(self._fall_time + self._bit_time / 2 + self._bit_time / 32)
         if xsi.is_port_driving(self._scl_port):
+            assert self._allow_clock_stretch, "I2C slave attempted to clock stretch"
             self.wait_for_port_pins_change([self._scl_port])
         xsi.drive_port_pins(self._scl_port, 1)
         self.wait_until(xsi.get_time() + self._bit_time / 4)
@@ -123,4 +126,3 @@ def run(self):
                     self.read(xsi, 0);
                 self.read(xsi, 1)
                 self.stop_bit(xsi)
-

tests/i2c_slave_test/CMakeLists.txt

@@ -19,11 +19,25 @@ foreach(arch ${ARCH})
     set(APP_HW_TARGET   ${target})
 
     set(APP_COMPILER_FLAGS_${arch}
-                    -O2
+                    -O3
                     -g
                     -DDEBUG_PRINT_ENABLE=1
                     -report)
 
     XMOS_REGISTER_APP()
     unset(APP_COMPILER_FLAGS_${arch})
+
+    set(config no_stretch_${arch})
+    project(i2c_slave_test)
+    set(APP_HW_TARGET ${target})
+
+    set(APP_COMPILER_FLAGS_${config}
+                    -O3
+                    -g
+                    -DDEBUG_PRINT_ENABLE=1
+                    -DI2C_SLAVE_CLOCK_STRETCH=0
+                    -report)
+
+    XMOS_REGISTER_APP()
+    unset(APP_COMPILER_FLAGS_${config})
 endforeach()

tests/requirements.txt

@@ -34,4 +34,4 @@ pytest-xdist==3.6.1
 # of its own setup.py file, then this list must include an entry for that
 # setup.py file, e.g., '-e .' or '-e ./python' (without the quotes).
 
--e git+ssh://git@github.com/xmos/test_support.git@v2.0.0#egg=test_support
+-e git+https://github.com/xmos/test_support.git@v2.0.0#egg=test_support