soc/cores/i2c: rewrite state machine

* Fix READ: was reading too many bits
* CLeaner transitions between states: ACK=>IDLE with scl=0. Other to IDLE with scl=1
* Now cleanly supports RESTART
* conceptual support for compound commands - not exposed yet
* fix tests: now appears to be I2C compliant

litex/soc/cores/i2c.py

@@ -65,41 +65,57 @@ class I2CMasterMachine(LiteXModule):
         self.fsm = fsm
 
         fsm.act("IDLE",
-            If(self.start,
+            # Valid combinations (lowest to highest priority):
-                NextState("START0"),
+            # stop: lowest priority
-            ).Elif(self.stop & self.start,
+            # read (& optional stop with automatic NACK)
-                NextState("RESTART0"),
+            # write (& optional stop)
-            ).Elif(self.stop,
+            # start (indicates start or restart)
+            # start & write (& optional stop)
+            # start & write & read (& optional stop)
+            # lowest priority
+            # *** TODO: support compound commands with I2CMaster ***
+            If(self.stop & ~self.scl_o,
+                # stop is only valid after an ACK
                 NextState("STOP0"),
-            ).Elif(self.write,
+            ),
+            If(self.read,
+                # post decrement so read first bit and shift in 7
+                NextValue(bits, 8-1),
+                NextState("READ0"),
+            ),
+            If(self.write,
                 NextValue(bits, 8),
                 NextState("WRITE0"),
-            ).Elif(self.read,
+            ),
-                NextValue(bits, 8),
+            # start could be requesting a restart
-                NextState("READ0"),
+            If(self.start,
-            )
+                NextState("RESTART0"),
+            ),
+            # highest priority: start only if scl is high
+            If(self.start & self.scl_o,
+                NextState("START0"),
+            ),
         )
 
         fsm.act("START0",
-            NextValue(self.scl_o, 1),
+            # Always entered with scl_o = 1
-            NextState("START1"))
-        fsm.act("START1",
             NextValue(self.sda_o, 0),
             NextState("IDLE"))
 
         fsm.act("RESTART0",
-            NextValue(self.scl_o, 0),
+            # Only entered from IDLE with scl_o = 0
+            NextValue(self.sda_o, 1),
             NextState("RESTART1"))
         fsm.act("RESTART1",
-            NextValue(self.sda_o, 1),
+            NextValue(self.scl_o, 1),
             NextState("START0"))
 
         fsm.act("STOP0",
-            NextValue(self.scl_o, 0),
+            # Only entered from IDLE with scl_o = 0
+            NextValue(self.sda_o, 0),
             NextState("STOP1"))
         fsm.act("STOP1",
             NextValue(self.scl_o, 1),
-            NextValue(self.sda_o, 0),
             NextState("STOP2"))
         fsm.act("STOP2",
             NextValue(self.sda_o, 1),
@@ -126,13 +142,15 @@ class I2CMasterMachine(LiteXModule):
             NextState("READACK1"),
         )
         fsm.act("READACK1",
+            # ACK => IDLE always with scl_o = 0
+            NextValue(self.scl_o, 0),
             NextValue(self.ack, ~self.sda_i),
             NextState("IDLE")
         )
 
         fsm.act("READ0",
-            NextValue(self.scl_o, 0),
+            # ACK => IDLE => READ0 always with scl_o = 0
-            NextValue(self.sda_o, 1),
+            NextValue(self.scl_o, 1),
             NextState("READ1"),
         )
         fsm.act("READ1",
@@ -154,7 +172,13 @@ class I2CMasterMachine(LiteXModule):
         )
         fsm.act("WRITEACK0",
             NextValue(self.scl_o, 1),
-            NextState("IDLE"),
+            NextState("WRITEACK1"),
+        )
+        fsm.act("WRITEACK1",
+            # ACK => IDLE always with scl_o = 0
+            NextValue(self.scl_o, 0),
+            NextValue(self.sda_o, 1),
+            NextState("IDLE")
         )
 
         run = Signal()

test/test_i2c.py

@@ -78,7 +78,7 @@ class TestI2C(unittest.TestCase):
         pads = _MockPads()
         dut = I2CMaster(pads)
 
-        def check_trans(scl, sda):
+        def check_trans(scl, sda, msg=""):
             scl, sda = int(scl), int(sda)
             scl_init, sda_init = (yield dut.scl_t.i), (yield dut.sda_t.i)
             timeout = 0
@@ -88,7 +88,7 @@ class TestI2C(unittest.TestCase):
                     return
                 timeout += 1
                 self.assertLess(timeout, 20,
-                    "\n*** timeout waiting for: " +
+                    f"\n*** {msg} timeout. Waiting for: " +
                     f"scl:{scl_now} checking:{scl_init}=>{scl} " +
                     f"sda:{sda_now} checking:{sda_init}=>{sda} ***"
                 )
@@ -116,11 +116,11 @@ class TestI2C(unittest.TestCase):
             yield from wait_idle()
 
         def read_bit(value):
-            #print(f"read_bit:{value}")
+            print(f"read_bit:{value}")
-            yield from check_trans(scl=False, sda=(yield dut.sda_tristate.i_mock))
             yield dut.sda_tristate.i_mock.eq(value)
-            yield from check_trans(scl=True,  sda=value)
+            yield from check_trans(scl=True, sda=value)
-            # need to restore i_mock elsewhere
+            yield from check_trans(scl=False, sda=value)
+            yield dut.sda_tristate.i_mock.eq(True)
 
         def read_ack(value):
             #print(f"read_ack:{value}")
@@ -132,6 +132,28 @@ class TestI2C(unittest.TestCase):
             ack = ((yield from dut.bus.read(I2C_XFER_ADDR)) & I2C_ACK) != 0
             self.assertEqual(ack, value)
 
+        def i2c_restart():
+            yield from check_trans(scl=False, sda=True, msg="checking restart precondition")
+            yield from dut.bus.write(I2C_XFER_ADDR, I2C_START)
+            yield from check_trans(scl=False, sda=True, msg="checking restart0")
+            yield from check_trans(scl=True, sda=True, msg="checking restart1")
+            yield from check_trans(scl=True, sda=False, msg="checking start0")
+            yield from wait_idle()
+
+        def i2c_start():
+            yield from check_trans(scl=True, sda=True, msg="checking start precondition")
+            yield from dut.bus.write(I2C_XFER_ADDR, I2C_START)
+            yield from check_trans(scl=True, sda=False, msg="checking start0")
+            yield from wait_idle()
+
+        def i2c_stop():
+            yield from check_trans(scl=False, sda=True, msg="checking stop after read or write")
+            yield from dut.bus.write(I2C_XFER_ADDR, I2C_STOP)
+            yield from check_trans(scl=False, sda=False, msg="checking STOP0")
+            yield from check_trans(scl=True, sda=False, msg="checking STOP1")
+            yield from check_trans(scl=True, sda=True, msg="checking STOP2")
+            yield from wait_idle()
+
         def i2c_write(value, ack=True):
             value = int(value)
             test_bin = '{0:08b}'.format(value)
@@ -144,7 +166,7 @@ class TestI2C(unittest.TestCase):
         def i2c_read(value, ack=True):
             value = int(value)
             test_bin = '{0:08b}'.format(value)
-            #print(f"I2C_READ | {hex(value)}:0x{test_bin}")
+            print(f"I2C_READ | {hex(value)}:0x{test_bin}")
             yield from dut.bus.write(I2C_XFER_ADDR, I2C_READ | (I2C_ACK if ack else 0))
             for i in list(test_bin):
                 yield from read_bit(int(i))
@@ -159,23 +181,23 @@ class TestI2C(unittest.TestCase):
             data = (yield from dut.bus.read(I2C_CONFIG_ADDR)) & 0xff
             self.assertEqual(data, 4)
 
-            yield from dut.bus.write(I2C_XFER_ADDR, I2C_START)
+            print("write 1 byte 0x18 to address 0x41")
-            yield from check_trans(scl=True, sda=False)
+            yield from i2c_start()
-            yield from wait_idle()
+            yield from i2c_write(0x41<<1 | 0)
-
-            yield from i2c_write(0x82)
             yield from i2c_write(0x18, ack=False)
+            yield from i2c_stop()
 
-            yield from dut.bus.write(I2C_XFER_ADDR, I2C_START | I2C_STOP)
+            print("read 1 byte from address 0x41")
-            yield from check_trans(scl=True,  sda=False)
+            yield from i2c_start()
-            yield from wait_idle()
+            yield from i2c_write(0x41<<1 | 1)
-
             yield from i2c_read(0x18, ack=False)
-            yield from i2c_read(0x88)
 
-            yield from dut.bus.write(I2C_XFER_ADDR, I2C_STOP)
+            print("read 2 bytes from address 0x55")
-            yield from check_trans(scl=False,  sda=False)
+            yield from i2c_restart()
-            yield from wait_idle()
+            yield from i2c_write(0x55<<1 | 1)
+            yield from i2c_read(0xDE, ack=True)
+            yield from i2c_read(0xAD, ack=False)
+            yield from i2c_stop()
 
         clocks = {
             "sys": 10,