diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5d2f1725..9ec67f23 100644 --- a/CHANGELOG.rst +++ b/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 ----- diff --git a/README.rst b/README.rst index b824ba8c..c42ee331 100644 --- a/README.rst +++ b/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 `_ +* `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/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 `_ +`www.xmos.com/support `_ or using GitHub `issues `_. + diff --git a/lib_i2c/api/i2c.h b/lib_i2c/api/i2c.h index 775bf1c6..8e3bbe88 100644 --- a/lib_i2c/api/i2c.h +++ b/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 diff --git a/lib_i2c/src/i2c_slave.xc b/lib_i2c/src/i2c_slave.xc index 13ddfdda..4030a5b5 100644 --- a/lib_i2c/src/i2c_slave.xc +++ b/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; diff --git a/tests/expected/no_clock_stretch.expect b/tests/expected/no_clock_stretch.expect new file mode 100644 index 00000000..c7bce952 --- /dev/null +++ b/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 diff --git a/tests/i2c_slave_checker.py b/tests/i2c_slave_checker.py index fe064255..4c335c0a 100644 --- a/tests/i2c_slave_checker.py +++ b/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) - diff --git a/tests/i2c_slave_test/CMakeLists.txt b/tests/i2c_slave_test/CMakeLists.txt index ff999986..f1095958 100644 --- a/tests/i2c_slave_test/CMakeLists.txt +++ b/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() diff --git a/tests/requirements.txt b/tests/requirements.txt index 883b4a8e..f2694a8f 100644 --- a/tests/requirements.txt +++ b/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 diff --git a/tests/test_slave_no_clock_stretch.py b/tests/test_slave_no_clock_stretch.py new file mode 100644 index 00000000..ecd4be52 --- /dev/null +++ b/tests/test_slave_no_clock_stretch.py @@ -0,0 +1,44 @@ +# Copyright 2026 XMOS LIMITED. +# This Software is subject to the terms of the XMOS Public Licence: Version 1. +from pathlib import Path + +import Pyxsim +import pytest + +from i2c_slave_checker import I2CSlaveChecker + + +@pytest.mark.parametrize("arch", ["xs2", "xs3"]) +def test_slave_no_clock_stretch(capfd, request, arch): + cwd = Path(request.fspath).parent + config = f"no_stretch_{arch}" + binary = cwd / "i2c_slave_test" / "bin" / config / f"i2c_slave_test_{config}.xe" + + assert binary.exists(), f"Cannot find {binary}" + + checker = I2CSlaveChecker( + "tile[0]:XS1_PORT_1A", + "tile[0]:XS1_PORT_1B", + tsequence=[ + ("r", 0x3C, 1), + ("w", 0x3C, [0x33, 0xFF]), + ], + speed=10, + allow_clock_stretch=False, + ) + + tester = Pyxsim.testers.AssertiveComparisonTester( + f"{cwd}/expected/no_clock_stretch.expect", + regexp=True, + ordered=True, + suppress_multidrive_messages=True, + ) + + Pyxsim.run_on_simulator_( + str(binary), + tester=tester, + do_xe_prebuild=False, + simthreads=[checker], + simargs=["--weak-external-drive"], + capfd=capfd, + ) diff --git a/tests/test_slave_scl_spike_filter.py b/tests/test_slave_scl_spike_filter.py new file mode 100644 index 00000000..ccd07f2c --- /dev/null +++ b/tests/test_slave_scl_spike_filter.py @@ -0,0 +1,114 @@ +# Copyright 2026 XMOS LIMITED. +# This Software is subject to the terms of the XMOS Public Licence: Version 1. +from pathlib import Path + +import Pyxsim +import pytest + +from i2c_slave_checker import I2CSlaveChecker + + +test_name = "i2c_slave_test" + + +class I2CSlaveSclSpikeChecker(I2CSlaveChecker): + def __init__(self, *args, spike_pulse_index, spike_width_ns=49, + **kwargs): + super().__init__(*args, **kwargs) + self._pulse_index = 0 + if isinstance(spike_pulse_index, int): + spike_pulse_index = [spike_pulse_index] + self._spike_pulse_indices = set(spike_pulse_index) + self._spike_width = spike_width_ns * 1e6 + + def maybe_spike(self, xsi): + if self._pulse_index in self._spike_pulse_indices: + self.inject_scl_high_spike(xsi) + self._pulse_index += 1 + + def inject_scl_high_spike(self, xsi): + xsi.drive_port_pins(self._scl_port, 1) + self.wait_until(xsi.get_time() + self._spike_width) + xsi.drive_port_pins(self._scl_port, 0) + + def high_pulse(self, xsi): + self.maybe_spike(xsi) + super().high_pulse(xsi) + + def high_pulse_sample(self, xsi): + self.maybe_spike(xsi) + return super().high_pulse_sample(xsi) + + +def run_slave_scl_spike_filter_test(capfd, cwd, binary, spike_pulse_index): + checker = I2CSlaveSclSpikeChecker( + "tile[0]:XS1_PORT_1A", + "tile[0]:XS1_PORT_1B", + tsequence=[ + ("w", 0x3C, [0x33, 0x44, 0x03]), + ("r", 0x3C, 3), + ("w", 0x3C, [0x99]), + ("w", 0x44, [0x33]), + ("r", 0x3C, 1), + ("w", 0x3C, [0x22, 0xFF]), + ], + speed=400, + spike_pulse_index=spike_pulse_index, + ) + + tester = Pyxsim.testers.AssertiveComparisonTester( + f"{cwd}/expected/basic_slave_test.expect", + regexp=True, + ordered=True, + suppress_multidrive_messages=True, + ) + + Pyxsim.run_on_simulator_( + binary, + tester=tester, + do_xe_prebuild=False, + simthreads=[checker], + simargs=["--weak-external-drive"], + capfd=capfd, + ) + + +@pytest.mark.parametrize("arch", ["xs3"]) +@pytest.mark.parametrize( + "spike_name, spike_pulse_index", + [ + ("address_first_bit", 0), + ("address_mid_bit", 4), + ("address_ack", 8), + ("write_data_first_bit", 9), + ("write_data_ack", 17), + ("read_data_first_bit", 45), + ("read_data_ack", 53), + ("read_next_byte_first_bit", 54), + ("read_next_byte_ack", 62), + ("read_final_nack", 71), + ], +) +def test_slave_scl_spike_filter(capfd, request, arch, spike_name, + spike_pulse_index): + cwd = Path(request.fspath).parent + binary = f"{cwd}/{test_name}/bin/{arch}/{test_name}_{arch}.xe" + + assert Path(binary).exists(), f"Cannot find {binary}" + + run_slave_scl_spike_filter_test(capfd, cwd, binary, spike_pulse_index) + + +@pytest.mark.parametrize("arch", ["xs3"]) +def test_slave_scl_spike_filter_multiple_glitches(capfd, request, arch): + cwd = Path(request.fspath).parent + binary = f"{cwd}/{test_name}/bin/{arch}/{test_name}_{arch}.xe" + + assert Path(binary).exists(), f"Cannot find {binary}" + + run_slave_scl_spike_filter_test( + capfd, + cwd, + binary, + [0, 8, 17, 45, 53, 71], + )