From f99658200edad158c1a00d9a899817ab4fe86721 Mon Sep 17 00:00:00 2001 From: Andrew Dennison Date: Tue, 29 Aug 2023 17:31:28 +1000 Subject: [PATCH] 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 | 64 +++++++++++++++++++++++++++++------------- test/test_i2c.py | 62 +++++++++++++++++++++++++++------------- 2 files changed, 86 insertions(+), 40 deletions(-) diff --git a/litex/soc/cores/i2c.py b/litex/soc/cores/i2c.py index 4a932acc4..702b35c22 100644 --- a/litex/soc/cores/i2c.py +++ b/litex/soc/cores/i2c.py @@ -65,41 +65,57 @@ class I2CMasterMachine(LiteXModule): self.fsm = fsm fsm.act("IDLE", - If(self.start, - NextState("START0"), - ).Elif(self.stop & self.start, - NextState("RESTART0"), - ).Elif(self.stop, + # Valid combinations (lowest to highest priority): + # stop: lowest priority + # read (& optional stop with automatic NACK) + # write (& optional 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), - NextState("READ0"), - ) + ), + # start could be requesting a restart + 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), - NextState("START1")) - fsm.act("START1", + # Always entered with scl_o = 1 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), - NextValue(self.sda_o, 1), + # ACK => IDLE => READ0 always with scl_o = 0 + 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() diff --git a/test/test_i2c.py b/test/test_i2c.py index 147ad063c..309357476 100755 --- a/test/test_i2c.py +++ b/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}") - yield from check_trans(scl=False, sda=(yield dut.sda_tristate.i_mock)) + print(f"read_bit:{value}") yield dut.sda_tristate.i_mock.eq(value) - yield from check_trans(scl=True, sda=value) - # need to restore i_mock elsewhere + yield from check_trans(scl=True, sda=value) + 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) - yield from check_trans(scl=True, sda=False) - yield from wait_idle() - - yield from i2c_write(0x82) + print("write 1 byte 0x18 to address 0x41") + yield from i2c_start() + yield from i2c_write(0x41<<1 | 0) yield from i2c_write(0x18, ack=False) + yield from i2c_stop() - yield from dut.bus.write(I2C_XFER_ADDR, I2C_START | I2C_STOP) - yield from check_trans(scl=True, sda=False) - yield from wait_idle() - + print("read 1 byte from address 0x41") + yield from i2c_start() + 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) - yield from check_trans(scl=False, sda=False) - yield from wait_idle() + print("read 2 bytes from address 0x55") + yield from i2c_restart() + 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,