✨ [rtl] add optional SPI data FIFO

[stnolting]

This PR adds an optional data FIFO to the SPI module (idea by @akaeba from #378). A new top generic is added to configure the SPI FIFO size: IO_TRNG_FIFO, default = 0; has to be zero or a power of two. Furthermore, all FIFO status signals are added to the SPI control register and a fine grained interrupt configuration is added:

• interrupt if SPI PHY completes the current transmission (default, behavior of pre-PR version)
• interrupt if SPI TX FIFO becomes empty
• interrupt if SPI TX FIFO becomes less than half full

✔️ The changes from this PR are fully backwards compatible.

[akaeba]

Hi Stephan, i adjusted the driver see #382. Nevertheless there are some bugs inside. For the case IO_SPI_FIFO = 0 is all fine. But if i set IO_SPI_FIFO=32 and run following code example:

while ( 1 ) {
  memset(uint8MemBuf, 0, sizeof(uint8MemBuf)/sizeof(uint8MemBuf[0]));
  uint8MemBuf[0] = 0x03;
  neorv32_spi_rw(&g_neorv32_spi, uint8MemBuf, 0, sizeof(uint8MemBuf)/sizeof(uint8MemBuf[0]), 1);
  while ( neorv32_spi_rw_busy(&g_neorv32_spi) ) {
    __asm("nop");
  }
  while ( 1 ) {
    __asm("nop");
  }
}

Then Performs the SPI core with disabled CSn an additional Access.

One other Hint, if the FIFO=0, should tx_fifo.empty the same like busy. This relaxes the SW because you need to to change the config from tx_fifo.empty IRQ to Idle IRQ. Because it's the same.

[stnolting]

Hi Stephan, i adjusted the driver see #382.

Great! I'll have a look later.

But if i set IO_SPI_FIFO=32 and run following code example: [...] Then Performs the SPI core with disabled CSn an additional Access.

I do not understand your code yet but I will have a closer look at this. 😉

One other Hint, if the FIFO=0, should tx_fifo.empty the same like busy. This relaxes the SW because you need to to change the config from tx_fifo.empty IRQ to Idle IRQ. Because it's the same.

You are right, but I was thinking about this the other way around: basically, software should only check the SPI control register's busy flag. If FIFO_SIZE = 0 this flag indicates (when set) that the SPI engine is currently doing a transmission (full backwards compatible). If FIFO_SIZE > 0 this flag is set when the SPI engine is busy OR when the TX FIFO has not run empty yet. So even software that was not written for handling the FIFO can still rely on the bus flag.

The FIFO status signals should only be used by functions (or IRQ handlers) that were explicitly designed to handle "block transfers" (actually using the FIFO).

[akaeba]

I do not understand your code yet but I will have a closer look at this. 😉

I try to write 10 Bytes to the SPI, the core performs also this transaction. But Later when all is done, i see one additional transfer. Thanks for checking :)

[akaeba]

The FIFO status signals should only be used by functions (or IRQ handlers) that were explicitly designed to handle "block transfers" (actually using the FIFO).

Okay, i think there i need to explain a little more. If you have FIFO=0, and setup the SPI IRQ=3d, then the Core will not produce IRQs. My segustions is: that in that case the busy irq is also produced even if tx empty IRQ is selected.

[stnolting]

I try to write 10 Bytes to the SPI, the core performs also this transaction. But Later when all is done, i see one additional transfer. Thanks for checking :)

I just simulated the code from #382. There are exactly ten transfers:

SPI signals in light blue. There are 8 transfers between the two cursors; in this time CS (spi_csn_o) is low. The "start" signal just beneath the light blue "busy" signal shows 8 spikes - so 8 transfers are getting triggered in total.

[stnolting]

If you have FIFO=0, and setup the SPI IRQ=3d, then the Core will not produce IRQs.

Right. I need to modify the SPI control register so that writing interrupt configurations 10 and 11 (both requiring the FIFO) always falls back to 00 (just checking the busy state of the SPI engine itself) if FIFO_SIZE = 0.

My segustions is: that in that case the busy irq is also produced even if tx empty IRQ is selected.

Good point, but I think having the fall-back option should also resolve this.

Btw, I think software should check if the SPI FIFO is implemented at all (there are 4 bits in the control register that show log2(FIFO_SIZE)) before configuring the SPI module (especially the IRQ configuration).

[akaeba]

I just simulated the code from #382. There are exactly ten transfers:

I have an idea, I need to check.

[akaeba]

Good point, but I think having the fall-back option should also resolve this.

I'll try 😉

[akaeba]

Hi Stephan, with f9d8ece is the driver operable. the root cause was the interruption of the data write loop in the '_rw' function by the ISR. Now is the driver operable.

[stnolting]

Awesome!

self->uint32Fifo = (uint32_t) neorv32_spi_get_fifo_depth(); // acquire FIFO depth in elements

That looks really good! 😉

I will test your code in a simulation later today. If everything looks fine we can merge #382 I think, right?

[akaeba]

Perfect, thanks for adding the FIFO. 😉

[stnolting]

Your modification of demo_spi_irq works as expected, so I think we can merge that. 👍

However, using the "FIFO gets empty" interrupt is not really efficient here as only a single word is written to the FIFO on each IRQ. A more sophisticated approach should make use of "double buffering" utilizing half of the FIFO. But this is something we could add in the future... 😉

thanks for adding the FIFO

You're welcome! And thanks for the idea!

[akaeba]

"FIFO gets empty" interrupt is not really efficient here as only a single word is written

I think not directly, lets make a small deep dive. In this function:

neorv32/sw/example/demo_spi_irq/drv/neorv32_spi_irq.c

Line 159 in 7eaf2c2

int neorv32_spi_rw(t_neorv32_spi *self, void *spi, uint8_t csn, uint32_t num_elem, uint8_t data_byte) {

neorv32/sw/example/demo_spi_irq/drv/neorv32_spi_irq.c

Line 180 in 7eaf2c2

uint32Buf |= ((uint8_t *) self->ptrSpiBuf)[0];

is only written one data word. This is caused that this function can be interrupted by the ISR. If i try to write more data words then i will mess up the whole packet.

In the ISR there should be written more datawords caused by:

neorv32/sw/example/demo_spi_irq/drv/neorv32_spi_irq.c

Line 95 in 7eaf2c2

for ( ; self->uint32Write<uint32Lim; (self->uint32Write)++ ) {

because there is no return otherwise the data is send. But your right at the beginning fires the controller more IRQs. If the loops goes on the IRQ rate reduces. This is caused that in the higher speed modes the SPI is really fast. If you like, you can try to set-up a lower speed, then you should see the the FIFO is fully filled by the ISR without any new IRQ.

This knowledge would also enables a completly different driver architecture: you bang out the bytes until the SPI return with busy then you leave the ISR. But the FIFO is much better :)

[stnolting]

int neorv32_spi_rw(...)
is only written one data word. This is caused that this function can be interrupted by the ISR. If i try to write more data words then i will mess up the whole packet.

Right. Since this is the setup initial function you could disable the SPI interrupt temporarily to fill up the FIFO as much as possible.

But your right at the beginning fires the controller more IRQs. If the loops goes on the IRQ rate reduces. This is caused that in the higher speed modes the SPI is really fast. If you like, you can try to set-up a lower speed, then you should see the the FIFO is fully filled by the ISR without any new IRQ.

You are right! I keep forgetting that this runs in "highspeed" mode... 😅

Anyway, I think this is ready to be merged! 👍