soc/cores/i2s: cleanup pass, rename to S7I2SSlave (since 7-Series specific for now...
authorFlorent Kermarrec <florent@enjoy-digital.fr>
Thu, 6 Feb 2020 16:00:04 +0000 (17:00 +0100)
committerFlorent Kermarrec <florent@enjoy-digital.fr>
Thu, 6 Feb 2020 16:00:04 +0000 (17:00 +0100)
litex/soc/cores/i2s.py
test/test_i2s.py [new file with mode: 0644]

index 512b21646142f6dd1181bda137d2a3b5a525208b..96011ad3eddd854c8e4db5218ed15e17c834edfe 100644 (file)
 # This file is Copyright (c) 2020 bunnie <bunnie@kosagi.com>
 # License: BSD
 
-from litex.soc.interconnect.csr_eventmanager import *
+from migen.genlib.cdc import MultiReg
+
 from litex.soc.interconnect import wishbone
+from litex.soc.interconnect.csr_eventmanager import *
+
 from litex.soc.integration.doc import AutoDoc, ModuleDoc
-from migen.genlib.cdc import MultiReg
 
-class i2s_slave(Module, AutoCSR, AutoDoc):
-    def __init__(self, pads, fifodepth=256):
+
+class S7I2SSlave(Module, AutoCSR, AutoDoc):
+    def __init__(self, pads, fifo_depth=256):
         self.intro = ModuleDoc("""
         Intro
         *******
-        
-        I2S slave creates a slave audio interface instance. Tx and Rx interfaces are inferred
-        based upon the presence or absence of the respective pins in the "pads" argument.
-        
-        The interface is I2S-like, but note the deviation that the bits are justified
-        left without a 1-bit pad after sync edges. This isn't a problem for talking to the LM49352
-        codec this was designed for, as the bit offset is programmable, but this will not work well if
-        are talking to a CODEC without a programmable bit offset!
-        
+
+        I2S slave creates a slave audio interface instance. Tx and Rx interfaces are inferred based
+        upon the presence or absence of the respective pins in the "pads" argument.
+
+        The interface is I2S-like, but note the deviation that the bits are justified left without a
+        1-bit pad after sync edges. This isn't a problem for talking to the LM49352 codec this was
+        designed for, as the bit offset is programmable, but this will not work well if are talking
+        to a CODEC without a programmable bit offset!
+
         System Interface
         =================
-        
+
         Audio interchange is done with the system using 16-bit stereo samples, with the right channel
         mapped to the least significant word of a 32-bit word. Thus each 32-bit word is a single
-        stereo sample. As this is a slave I2S interface, sampling rate and framing is set by the programming 
-        of the audio CODEC chip. A slave situation is preferred because this defers the generation of
-        audio clocks to the CODEC, which has PLLs specialized to generate the correct frequencies for
-        audio sampling rates.
-          
-        `fifodepth` is the depth at which either a read interrupt is fired (guaranteeing at least `fifodepth`
-        stereo samples in the receive FIFO) or a write interrupt is fired (guaranteeing at least `fifodepth`
-        free space in the transmit FIFO). The maximum depth is 512.
-        
+        stereo sample. As this is a slave I2S interface, sampling rate and framing is set by the
+        programming of the audio CODEC chip. A slave situation is preferred because this defers the
+        generation of audio clocks to the CODEC, which has PLLs specialized to generate the correct
+        frequencies for audio sampling rates.
+
+        `fifo_depth` is the depth at which either a read interrupt is fired (guaranteeing at least
+        `fifo_depth` stereo samples in the receive FIFO) or a write interrupt is fired (guaranteeing
+        at least `fifo_depth` free space in the transmit FIFO). The maximum depth is 512.
+
         To receive audio data:
-        
+
         - reset the Rx FIFO, to guarantee all pointers at zero
         - hook the Rx full interrupt with an interrupt handler (optional)
         - if the CODEC is not yet transmitting data, initiate data transmission
         - enable Rx FIFO to run
-        - poll or wait for interrupt; upon interrupt, read `fifodepth` words. Repeat.
+        - poll or wait for interrupt; upon interrupt, read `fifo_depth` words. Repeat.
         - to close the stream, simply clear the Rx FIFO enable bit. The next initiation should call a
-          reset of the FIFO to ensure leftover previous data is cleared from the FIFO. 
-        
+          reset of the FIFO to ensure leftover previous data is cleared from the FIFO.
+
         To transmit audio data:
-        
+
         - reset the Tx FIFO, to guarantee all pointers at zero
         - hook the Tx available interrupt with an interrupt handler (optional)
         - write 512 words of data into the Tx FIFO, filling it to the max
-        - if the CODEC is not yet requesting data and unmuted, unmute and initiate reception 
+        - if the CODEC is not yet requesting data and unmuted, unmute and initiate reception
         - enable the Tx FIFO to run
-        - poll or wait for interrupt; upon interrupt, write `fifodepth` words. Repeat.
-        - to close stream, mute the DAC and stop the request clock. Ideally, this can be completed before
-          the FIFO is emptied, so there is no jarring pop or truncation of data
-        - stop FIFO running. Next initiation should reset the FIFO to ensure leftover previous data in
-          FIFO is cleared. 
-        
+        - poll or wait for interrupt; upon interrupt, write `fifo_depth` words. Repeat.
+        - to close stream, mute the DAC and stop the request clock. Ideally, this can be completed
+        before the FIFO is emptied, so there is no jarring pop or truncation of data
+        - stop FIFO running. Next initiation should reset the FIFO to ensure leftover previous data
+        in FIFO is cleared.
+
         CODEC Interface
         ================
-        
-        The interface assumes we have a sysclk domain running around 100MHz, and that our typical
-        max audio rate is 44.1kHz * 24bits * 2channels = 2.1168MHz audio clock. Thus, the architecture
+
+        The interface assumes we have a sysclk domain running around 100MHz, and that our typical max
+        audio rate is 44.1kHz * 24bits * 2channels = 2.1168MHz audio clock. Thus, the architecture
         treats the audio clock and data as asynchronous inputs that are MultiReg-syncd into the clock
         domain. Probably the slowest sysclk rate this might work with is around 20-25MHz (10x over
         sampling), but at 100MHz things will be quite comfortable.
-        
+
         The upside of the fully asynchronous implementation is that we can leave the I/O unconstrained,
         giving the place/route more latitude to do its job.
-        
+
         Here's the timing format targeted by this I2S interface:
-        
+
             .. wavedrom::
                 :caption: Timing format of the I2S interface
 
                 { "signal" : [
                   { "name": "clk",         "wave": "n....|.......|......" },
                   { "name": "sync",        "wave": "1.0..|....1..|....0." },
-                  { "name": "tx/rx",       "wave": ".====|==x.===|==x.=x", "data": ["L15", "L14", "...", "L1", "L0", "R15", "R14", "...", "R1", "R0", "L15"] },
+                  { "name": "tx/rx",       "wave": ".====|==x.===|==x.=x", "data":
+                  ["L15", "L14", "...", "L1", "L0", "R15", "R14", "...", "R1", "R0", "L15"] },
                 ]}
-        
+
         - Data is updated on the falling edge
         - Data is sampled on the rising edge
-        - Words are MSB-to-LSB, left-justified (**NOTE: this is a deviation from strict I2S, which offsets by 1 from the left**)
+        - Words are MSB-to-LSB, left-justified (**NOTE: this is a deviation from strict I2S, which
+        offsets by 1 from the left**)
         - Sync is an input (FPGA is slave, codec is master): low => left channel, high => right channel
         - Sync can be longer than the wordlen, extra bits are just ignored
         - Tx is data to the codec (SDI pin on LM49352)
         - Rx is data from the codec (SDO pin on LM49352)
-        
+
         """)
 
-        # one cache line is 8 32-bit words, need to always have enough space for one line or else nothing works
-        if fifodepth > 504:
-            fifodepth = 504
+        # One cache line is 8 32-bit words, need to always have enough space for one line or else
+        # nothing works
+        if fifo_depth > 504:
+            fifo_depth = 504
             print("I2S warning: fifo depth greater than 504 selected; truncating to 504")
-        if fifodepth < 8:
-            fifodepth = 8
+        if fifo_depth < 8:
+            fifo_depth = 8
             print("I2S warning: fifo depth less than 8 selected; truncating to 8")
 
-        # connect pins, synchronizers, and edge detectors
+        # Connect pins, synchronizers, and edge detectors
         if hasattr(pads, 'tx'):
             tx_pin = Signal()
             self.comb += pads.tx.eq(tx_pin)
@@ -112,67 +118,67 @@ class i2s_slave(Module, AutoCSR, AutoDoc):
         self.specials += MultiReg(pads.clk, clk_pin)
         clk_d = Signal()
         self.sync += clk_d.eq(clk_pin)
-        rising_edge = Signal()
+        rising_edge  = Signal()
         falling_edge = Signal()
         self.comb += [rising_edge.eq(clk_pin & ~clk_d), falling_edge.eq(~clk_pin & clk_d)]
 
-        # wishbone bus
-        self.bus = wishbone.Interface()
+        # Wishbone bus
+        self.bus = bus = wishbone.Interface()
         rd_ack = Signal()
         wr_ack = Signal()
         self.comb +=[
-            If(self.bus.we,
-               self.bus.ack.eq(wr_ack),
+            If(bus.we,
+               bus.ack.eq(wr_ack),
             ).Else(
-                self.bus.ack.eq(rd_ack),
+                bus.ack.eq(rd_ack),
             )
         ]
 
-        # interrupts
+        # Interrupts
         self.submodules.ev = EventManager()
         if hasattr(pads, 'rx'):
-            self.ev.rx_ready = EventSourcePulse(description="Indicates FIFO is ready to read")  # rising edge triggered
+            self.ev.rx_ready = EventSourcePulse(description="Indicates FIFO is ready to read")  # Rising edge triggered
             self.ev.rx_error = EventSourcePulse(description="Indicates an Rx error has happened (over/underflow)")
         if hasattr(pads, 'tx'):
-            self.ev.tx_ready = EventSourcePulse(description="Indicates enough space available for next Tx quanta of {} words".format(fifodepth))
+            self.ev.tx_ready = EventSourcePulse(description="Indicates enough space available for next Tx quanta of {} words".format(fifo_depth))
             self.ev.tx_error = EventSourcePulse(description="Indicates a Tx error has happened (over/underflow")
         self.ev.finalize()
 
 
         # build the RX subsystem
         if hasattr(pads, 'rx'):
-            rx_rd_d = Signal(32)
-            rx_almostfull = Signal()
+            rx_rd_d        = Signal(32)
+            rx_almostfull  = Signal()
             rx_almostempty = Signal()
-            rx_full = Signal()
-            rx_empty = Signal()
-            rx_rdcount = Signal(9)
-            rx_rderr = Signal()
-            rx_wrerr = Signal()
-            rx_wrcount = Signal(9)
-            rx_rden = Signal()
-            rx_wr_d = Signal(32)
-            rx_wren = Signal()
+            rx_full        = Signal()
+            rx_empty       = Signal()
+            rx_rdcount     = Signal(9)
+            rx_rderr       = Signal()
+            rx_wrerr       = Signal()
+            rx_wrcount     = Signal(9)
+            rx_rden        = Signal()
+            rx_wr_d        = Signal(32)
+            rx_wren        = Signal()
 
             self.rx_ctl = CSRStorage(description="Rx data path control",
                 fields=[
                     CSRField("enable", size=1, description="Enable the receiving data"),
-                    CSRField("reset", size=1, description="Writing `1` resets the FIFO. Reset happens regardless of enable state.", pulse=1)
+                    CSRField("reset",  size=1, description="Writing `1` resets the FIFO. Reset happens regardless of enable state.", pulse=1)
                 ])
             self.rx_stat = CSRStatus(description="Rx data path status",
                 fields=[
-                    CSRField("overflow", size=1, description="Rx overflow"),
+                    CSRField("overflow",  size=1, description="Rx overflow"),
                     CSRField("underflow", size=1, description="Rx underflow"),
-                    CSRField("dataready", size=1, description="{} words of data loaded and ready to read".format(fifodepth)),
-                    CSRField("empty", size=1, description="No data available in FIFO to read"), # next flags probably never used
-                    CSRField("wrcount", size=9, description="Write count"),
-                    CSRField("rdcount", size=9, description="Read count"),
-                    CSRField("fifodepth", size=9, description="FIFO depth as synthesized")
+                    CSRField("dataready", size=1, description="{} words of data loaded and ready to read".format(fifo_depth)),
+                    CSRField("empty",     size=1, description="No data available in FIFO to read"), # next flags probably never used
+                    CSRField("wrcount",   size=9, description="Write count"),
+                    CSRField("rdcount",   size=9, description="Read count"),
+                    CSRField("fifo_depth", size=9, description="FIFO depth as synthesized")
                 ])
-            self.comb += self.rx_stat.fields.fifodepth.eq(fifodepth)
+            self.comb += self.rx_stat.fields.fifo_depth.eq(fifo_depth)
 
             rx_rst_cnt = Signal(3)
-            rx_reset = Signal()
+            rx_reset   = Signal()
             self.sync += [
                 If(self.rx_ctl.fields.reset,
                    rx_rst_cnt.eq(5),  # 5 cycles reset required by design
@@ -186,19 +192,30 @@ class i2s_slave(Module, AutoCSR, AutoDoc):
                     )
                 )
             ]
-            # at a width of 32 bits, an 18kiB fifo is 512 entries deep
+            # At a width of 32 bits, an 18kiB fifo is 512 entries deep
             self.specials += Instance("FIFO_SYNC_MACRO",
-                                p_DEVICE="7SERIES", p_FIFO_SIZE="18Kb", p_DATA_WIDTH=32,
-                                p_ALMOST_EMPTY_OFFSET=8, p_ALMOST_FULL_OFFSET=(512 - fifodepth),
-                                p_DO_REG=0,
-
-                                o_ALMOSTFULL=rx_almostfull, o_ALMOSTEMPTY=rx_almostempty,
-                                o_DO=rx_rd_d, o_EMPTY=rx_empty, o_FULL=rx_full,
-                                o_RDCOUNT=rx_rdcount, o_RDERR=rx_rderr, o_WRCOUNT=rx_wrcount, o_WRERR=rx_wrerr,
-                                i_DI=rx_wr_d, i_CLK=ClockSignal(), i_RDEN=rx_rden & ~rx_reset,
-                                i_WREN=rx_wren & ~rx_reset, i_RST=rx_reset,
+                p_DEVICE              = "7SERIES",
+                p_FIFO_SIZE           = "18Kb",
+                p_DATA_WIDTH          = 32,
+                p_ALMOST_EMPTY_OFFSET = 8,
+                p_ALMOST_FULL_OFFSET  = (512 - fifo_depth),
+                p_DO_REG              = 0,
+                i_CLK         = ClockSignal(),
+                i_RST         = rx_reset,
+                o_ALMOSTFULL  = rx_almostfull,
+                o_ALMOSTEMPTY = rx_almostempty,
+                o_FULL        = rx_full,
+                o_EMPTY       = rx_empty,
+                i_WREN        = rx_wren & ~rx_reset,
+                i_DI          = rx_wr_d,
+                i_RDEN        = rx_rden & ~rx_reset,
+                o_DO          = rx_rd_d,
+                o_RDCOUNT     = rx_rdcount,
+                o_RDERR       = rx_rderr,
+                o_WRCOUNT     = rx_wrcount,
+                o_WRERR       = rx_wrerr,
             )
-            self.comb += [  # wire up the status signals and interrupts
+            self.comb += [  # Wire up the status signals and interrupts
                 self.rx_stat.fields.overflow.eq(rx_wrerr),
                 self.rx_stat.fields.underflow.eq(rx_rderr),
                 self.rx_stat.fields.dataready.eq(rx_almostfull),
@@ -207,19 +224,21 @@ class i2s_slave(Module, AutoCSR, AutoDoc):
                 self.ev.rx_ready.trigger.eq(rx_almostfull),
                 self.ev.rx_error.trigger.eq(rx_wrerr | rx_rderr),
             ]
-            bus_read = Signal()
-            bus_read_d = Signal()
+            bus_read    = Signal()
+            bus_read_d  = Signal()
             rd_ack_pipe = Signal()
-            self.comb += bus_read.eq(self.bus.cyc & self.bus.stb & ~self.bus.we & (self.bus.cti == 0))
-            self.sync += [  # this is the bus responder -- only works for uncached memory regions
+            self.comb += bus_read.eq(bus.cyc & bus.stb & ~bus.we & (bus.cti == 0))
+            self.sync += [  # This is the bus responder -- only works for uncached memory regions
                 bus_read_d.eq(bus_read),
-                If(bus_read & ~bus_read_d, # one response, one cycle
+                If(bus_read & ~bus_read_d, # One response, one cycle
                    rd_ack_pipe.eq(1),
                    If(~rx_empty,
-                      self.bus.dat_r.eq(rx_rd_d),
+                      bus.dat_r.eq(rx_rd_d),
                       rx_rden.eq(1),
                    ).Else(
-                       self.bus.dat_r.eq(0xDEADBEEF),  # don't stall the bus indefinitely if we try to read from an empty fifo...just return garbage
+                       # Don't stall the bus indefinitely if we try to read from an empty fifo...just
+                       # return garbage
+                       bus.dat_r.eq(0xdeadbeef),
                        rx_rden.eq(0),
                    )
                 ).Else(
@@ -232,97 +251,107 @@ class i2s_slave(Module, AutoCSR, AutoDoc):
             rx_cnt = Signal(5)
             self.submodules.rxi2s = rxi2s = FSM(reset_state="IDLE")
             rxi2s.act("IDLE",
-                      NextValue(rx_wr_d, 0),
-                      If(self.rx_ctl.fields.enable,
-                         If(rising_edge & sync_pin, # wait_sync guarantees we start at the beginning of a left frame, and not in the middle
-                            NextState("WAIT_SYNC"),
-                         )
-                      )
+                NextValue(rx_wr_d, 0),
+                If(self.rx_ctl.fields.enable,
+                    # Wait_sync guarantees we start at the beginning of a left frame, and not in
+                    # the middle
+                    If(rising_edge & sync_pin,
+                        NextState("WAIT_SYNC")
+                   )
+                )
             ),
             rxi2s.act("WAIT_SYNC",
-                      If(rising_edge & ~sync_pin,
-                         NextState("LEFT"),
-                         NextValue(rx_cnt, 16),
-                         ),
+                If(rising_edge & ~sync_pin,
+                    NextState("LEFT"),
+                    NextValue(rx_cnt, 16)
+                ),
             )
             rxi2s.act("LEFT",
-                      If(~self.rx_ctl.fields.enable, NextState("IDLE")).Else(
-                          NextValue(rx_wr_d, Cat(rx_pin, rx_wr_d[:-1])),
-                          NextValue(rx_cnt, rx_cnt - 1),
-                          NextState("LEFT_WAIT"),
-                      )
+                If(~self.rx_ctl.fields.enable,
+                    NextState("IDLE")
+                ).Else(
+                    NextValue(rx_wr_d, Cat(rx_pin, rx_wr_d[:-1])),
+                    NextValue(rx_cnt, rx_cnt - 1),
+                    NextState("LEFT_WAIT")
+                )
             )
             rxi2s.act("LEFT_WAIT",
-                      If(~self.rx_ctl.fields.enable, NextState("IDLE")).Else(
-                          If(rising_edge,
-                              If((rx_cnt == 0) & sync_pin,
-                                 NextValue(rx_cnt, 16),
-                                 NextState("RIGHT")
-                              ).Elif(rx_cnt > 0,
-                                NextState("LEFT"),
-                              )
-                          )
-                      )
+                If(~self.rx_ctl.fields.enable,
+                    NextState("IDLE")
+                ).Else(
+                    If(rising_edge,
+                        If((rx_cnt == 0) & sync_pin,
+                            NextValue(rx_cnt, 16),
+                            NextState("RIGHT")
+                        ).Elif(rx_cnt > 0,
+                            NextState("LEFT")
+                        )
+                    )
+                )
             )
             rxi2s.act("RIGHT",
-                      If(~self.rx_ctl.fields.enable, NextState("IDLE")).Else(
-                          NextValue(rx_wr_d, Cat(rx_pin, rx_wr_d[:-1])),
-                          NextValue(rx_cnt, rx_cnt - 1),
-                          NextState("RIGHT_WAIT"),
-                      )
+                If(~self.rx_ctl.fields.enable,
+                    NextState("IDLE")
+                ).Else(
+                    NextValue(rx_wr_d, Cat(rx_pin, rx_wr_d[:-1])),
+                    NextValue(rx_cnt, rx_cnt - 1),
+                    NextState("RIGHT_WAIT")
+                )
             )
             rxi2s.act("RIGHT_WAIT",
-                      If(~self.rx_ctl.fields.enable, NextState("IDLE")).Else(
-                          If(rising_edge,
-                              If((rx_cnt == 0) & ~sync_pin,
-                                 NextValue(rx_cnt, 16),
-                                 NextState("LEFT"),
-                                 rx_wren.eq(1), # pulse rx_wren to write the current data word
-                              ).Elif(rx_cnt > 0,
-                                NextState("RIGHT"),
-                              )
-                          )
-                      )
+                If(~self.rx_ctl.fields.enable,
+                    NextState("IDLE")
+                ).Else(
+                    If(rising_edge,
+                        If((rx_cnt == 0) & ~sync_pin,
+                            NextValue(rx_cnt, 16),
+                            NextState("LEFT"),
+                            rx_wren.eq(1) # Pulse rx_wren to write the current data word
+                        ).Elif(rx_cnt > 0,
+                            NextState("RIGHT")
+                        )
+                    )
+                )
             )
 
 
-        # build the TX subsystem
+        # Build the TX subsystem
         if hasattr(pads, 'tx'):
-            tx_rd_d = Signal(32)
-            tx_almostfull = Signal()
+            tx_rd_d        = Signal(32)
+            tx_almostfull  = Signal()
             tx_almostempty = Signal()
-            tx_full = Signal()
-            tx_empty = Signal()
-            tx_rdcount = Signal(9)
-            tx_rderr = Signal()
-            tx_wrerr = Signal()
-            tx_wrcount = Signal(9)
-            tx_rden = Signal()
-            tx_wr_d = Signal(32)
-            tx_wren = Signal()
+            tx_full        = Signal()
+            tx_empty       = Signal()
+            tx_rdcount     = Signal(9)
+            tx_rderr       = Signal()
+            tx_wrerr       = Signal()
+            tx_wrcount     = Signal(9)
+            tx_rden        = Signal()
+            tx_wr_d        = Signal(32)
+            tx_wren        = Signal()
 
             self.tx_ctl = CSRStorage(description="Tx data path control",
                 fields=[
                     CSRField("enable", size=1, description="Enable the transmission data"),
-                    CSRField("reset", size=1, description="Writing `1` resets the FIFO. Reset happens regardless of enable state.", pulse=1)
+                    CSRField("reset",  size=1, description="Writing `1` resets the FIFO. Reset happens regardless of enable state.", pulse=1)
                 ])
             self.tx_stat = CSRStatus(description="Tx data path status",
                 fields=[
-                    CSRField("overflow", size=1, description="Tx overflow"),
-                    CSRField("underflow", size=1, description="Tx underflow"),
-                    CSRField("free", size=1, description="At least {} words of space free".format(fifodepth)),
+                    CSRField("overflow",   size=1, description="Tx overflow"),
+                    CSRField("underflow",  size=1, description="Tx underflow"),
+                    CSRField("free",       size=1, description="At least {} words of space free".format(fifo_depth)),
                     CSRField("almostfull", size=1, description="Less than 8 words space available"), # the next few flags should be rarely used
-                    CSRField("full", size=1, description="FIFO is full or overfull"),
-                    CSRField("empty", size=1, description="FIFO is empty"),
-                    CSRField("wrcount", size=9, description="Tx write count"),
-                    CSRField("rdcount", size=9, description="Tx read count"),
+                    CSRField("full",       size=1, description="FIFO is full or overfull"),
+                    CSRField("empty",      size=1, description="FIFO is empty"),
+                    CSRField("wrcount",    size=9, description="Tx write count"),
+                    CSRField("rdcount",    size=9, description="Tx read count"),
                 ])
 
             tx_rst_cnt = Signal(3)
-            tx_reset = Signal()
+            tx_reset   = Signal()
             self.sync += [
                 If(self.tx_ctl.fields.reset,
-                   tx_rst_cnt.eq(5),  # 5 cycles reset required by design
+                   tx_rst_cnt.eq(5), # 5 cycles reset required by design
                    tx_reset.eq(1)
                 ).Else(
                     If(tx_rst_cnt == 0,
@@ -333,19 +362,31 @@ class i2s_slave(Module, AutoCSR, AutoDoc):
                     )
                 )
             ]
-            # at a width of 32 bits, an 18kiB fifo is 512 entries deep
+            # At a width of 32 bits, an 18kiB fifo is 512 entries deep
             self.specials += Instance("FIFO_SYNC_MACRO",
-                                p_DEVICE="7SERIES", p_FIFO_SIZE="18Kb", p_DATA_WIDTH=32,
-                                p_ALMOST_EMPTY_OFFSET=fifodepth, p_ALMOST_FULL_OFFSET=8,
-                                p_DO_REG=0,
-
-                                o_ALMOSTFULL=tx_almostfull, o_ALMOSTEMPTY=tx_almostempty,
-                                o_DO=tx_rd_d, o_EMPTY=tx_empty, o_FULL=tx_full,
-                                o_RDCOUNT=tx_rdcount, o_RDERR=tx_rderr, o_WRCOUNT=tx_wrcount, o_WRERR=tx_wrerr,
-                                i_DI=tx_wr_d, i_CLK=ClockSignal(), i_RDEN=tx_rden & ~tx_reset,
-                                i_WREN=tx_wren & ~tx_reset, i_RST=tx_reset,
+                p_DEVICE              = "7SERIES",
+                p_FIFO_SIZE           = "18Kb",
+                p_DATA_WIDTH          = 32,
+                p_ALMOST_EMPTY_OFFSET = fifo_depth,
+                p_ALMOST_FULL_OFFSET  = 8,
+                p_DO_REG              = 0,
+                i_CLK         = ClockSignal(),
+                i_RST         = tx_reset,
+                o_ALMOSTFULL  = tx_almostfull,
+                o_ALMOSTEMPTY = tx_almostempty,
+                o_FULL        = tx_full,
+                o_EMPTY       = tx_empty,
+                i_WREN        = tx_wren & ~tx_reset,
+                i_DI          = tx_wr_d,
+                i_RDEN        = tx_rden & ~tx_reset,
+                o_DO          = tx_rd_d,
+                o_RDCOUNT     = tx_rdcount,
+                o_RDERR       = tx_rderr,
+                o_WRCOUNT     = tx_wrcount,
+                o_WRERR       = tx_wrerr,
             )
-            self.comb += [  # wire up the status signals and interrupts
+
+            self.comb += [  # Wire up the status signals and interrupts
                 self.tx_stat.fields.overflow.eq(tx_wrerr),
                 self.tx_stat.fields.underflow.eq(tx_rderr),
                 self.tx_stat.fields.free.eq(tx_almostempty),
@@ -357,10 +398,12 @@ class i2s_slave(Module, AutoCSR, AutoDoc):
                 self.ev.tx_ready.trigger.eq(tx_almostempty),
                 self.ev.tx_error.trigger.eq(tx_wrerr | tx_rderr),
             ]
-            self.sync += [  # this is the bus responder -- need to check how this interacts with uncached memory region
-                If(self.bus.cyc & self.bus.stb & self.bus.we & ~self.bus.ack,
+            self.sync += [
+                # This is the bus responder -- need to check how this interacts with uncached memory
+                # region
+                If(bus.cyc & bus.stb & bus.we & ~bus.ack,
                    If(~tx_full,
-                      tx_wr_d.eq(self.bus.dat_w),
+                      tx_wr_d.eq(bus.dat_w),
                       tx_wren.eq(1),
                       wr_ack.eq(1),
                    ).Else(
@@ -377,59 +420,67 @@ class i2s_slave(Module, AutoCSR, AutoDoc):
             tx_buf = Signal(32)
             self.submodules.txi2s = txi2s = FSM(reset_state="IDLE")
             txi2s.act("IDLE",
-                      If(self.tx_ctl.fields.enable,
-                         If(falling_edge & sync_pin,
-                            NextState("WAIT_SYNC"),
-                         )
-                      )
+                If(self.tx_ctl.fields.enable,
+                    If(falling_edge & sync_pin,
+                        NextState("WAIT_SYNC"),
+                    )
+                )
             ),
             txi2s.act("WAIT_SYNC",
-                      If(falling_edge & ~sync_pin,
-                         NextState("LEFT"),
-                         NextValue(tx_cnt, 16),
-                         NextValue(tx_buf, tx_rd_d),
-                         tx_rden.eq(1),
-                      ),
+                If(falling_edge & ~sync_pin,
+                    NextState("LEFT"),
+                    NextValue(tx_cnt, 16),
+                    NextValue(tx_buf, tx_rd_d),
+                    tx_rden.eq(1)
+                )
             )
             txi2s.act("LEFT",
-                      If(~self.tx_ctl.fields.enable, NextState("IDLE")).Else(
-                          NextValue(tx_pin, tx_buf[31]),
-                          NextValue(tx_buf, Cat(0, tx_buf[:-1])),
-                          NextValue(tx_cnt, tx_cnt - 1),
-                          NextState("LEFT_WAIT"),
-                      )
+                If(~self.tx_ctl.fields.enable,
+                    NextState("IDLE")
+                ).Else(
+                    NextValue(tx_pin, tx_buf[31]),
+                    NextValue(tx_buf, Cat(0, tx_buf[:-1])),
+                    NextValue(tx_cnt, tx_cnt - 1),
+                    NextState("LEFT_WAIT")
+                )
             )
             txi2s.act("LEFT_WAIT",
-                      If(~self.tx_ctl.fields.enable, NextState("IDLE")).Else(
-                          If(falling_edge,
-                              If((tx_cnt == 0) & sync_pin,
-                                 NextValue(tx_cnt, 16),
-                                 NextState("RIGHT")
-                              ).Elif(tx_cnt > 0,
-                                NextState("LEFT"),
-                              )
-                          )
-                      )
+                If(~self.tx_ctl.fields.enable,
+                    NextState("IDLE")
+                ).Else(
+                    If(falling_edge,
+                        If((tx_cnt == 0) & sync_pin,
+                            NextValue(tx_cnt, 16),
+                            NextState("RIGHT")
+                        ).Elif(tx_cnt > 0,
+                            NextState("LEFT")
+                        )
+                    )
+                )
             )
             txi2s.act("RIGHT",
-                      If(~self.tx_ctl.fields.enable, NextState("IDLE")).Else(
-                          NextValue(tx_pin, tx_buf[31]),
-                          NextValue(tx_buf, Cat(0, tx_buf[:-1])),
-                          NextValue(tx_cnt, tx_cnt - 1),
-                          NextState("RIGHT_WAIT"),
-                      )
+                If(~self.tx_ctl.fields.enable,
+                    NextState("IDLE")
+                ).Else(
+                    NextValue(tx_pin, tx_buf[31]),
+                    NextValue(tx_buf, Cat(0, tx_buf[:-1])),
+                    NextValue(tx_cnt, tx_cnt - 1),
+                    NextState("RIGHT_WAIT")
+                )
             )
             txi2s.act("RIGHT_WAIT",
-                      If(~self.tx_ctl.fields.enable, NextState("IDLE")).Else(
-                          If(falling_edge,
-                              If((tx_cnt == 0) & ~sync_pin,
-                                 NextValue(tx_cnt, 16),
-                                 NextState("LEFT"),
-                                 NextValue(tx_buf, tx_rd_d),
-                                 tx_rden.eq(1),
-                              ).Elif(tx_cnt > 0,
-                                NextState("RIGHT"),
-                              )
-                          )
-                      )
-            )
\ No newline at end of file
+                If(~self.tx_ctl.fields.enable,
+                    NextState("IDLE")
+                ).Else(
+                    If(falling_edge,
+                        If((tx_cnt == 0) & ~sync_pin,
+                            NextValue(tx_cnt, 16),
+                            NextState("LEFT"),
+                            NextValue(tx_buf, tx_rd_d),
+                            tx_rden.eq(1)
+                        ).Elif(tx_cnt > 0,
+                            NextState("RIGHT")
+                        )
+                    )
+                )
+            )
diff --git a/test/test_i2s.py b/test/test_i2s.py
new file mode 100644 (file)
index 0000000..06b6602
--- /dev/null
@@ -0,0 +1,15 @@
+# This file is Copyright (c) 2020 Florent Kermarrec <florent@enjoy-digital.fr>
+# License: BSD
+
+import unittest
+
+from migen import *
+
+from litex.soc.cores.i2s import S7I2SSlave
+
+
+class TestI2S(unittest.TestCase):
+    def test_s7i2sslave_syntax(self):
+        i2s_pads = Record([("rx", 1), ("tx", 1), ("sync", 1), ("clk", 1)])
+        i2s = S7I2SSlave(pads=i2s_pads, fifo_depth=256)
+