[sw] add ISR based SPI data flow example

[akaeba]

Hello Stephan,

i added an ISR data based dataflow to the neorv32_spi.c. Concrete i added three functions:

• neorv32_spi_isr
• neorv32_spi_rw
• neorv32_spi_rw_busy

The function neorv32_spi_rw accepts an arbitrary number of bytes and the ISR gets the data from this internal buffer and forwards it to the SPI.

Following minimal example how to use:

#include <string.h>     // memset
#include <neorv32.h>


// common storage element for neorv32 spi
t_neorv32_spi   g_neorv32_spi;


// SPI interrupt handler
void spi_irq_handler(void)
{
    neorv32_spi_isr(&g_neorv32_spi);
}


// main
int main()
{
    uint8_t     uint8MemBuf[10];


    neorv32_rte_setup();    // enable error capture
    neorv32_rte_exception_install(SPI_RTE_ID, spi_irq_handler); // SPI to RTE
    neorv32_cpu_irq_enable(SPI_FIRQ_ENABLE);    // FIRQ6: SPI Interrupt
    neorv32_cpu_eint();                         // enable global interrupts

    neorv32_spi_disable();
    neorv32_spi_setup(0, 0, 0, 0);  // spi mode 0, 8bit
    neorv32_spi_enable();

    /* Data Transfer */
    memset(uint8MemBuf, 0, sizeof(uint8MemBuf)/sizeof(uint8MemBuf[0]));
    uint8MemBuf[0] = 0x3;
    uint8MemBuf[1] = 0x0;
    uint8MemBuf[2] = 0x0;
    uint8MemBuf[3] = 0x0;
    neorv32_spi_rw(&g_neorv32_spi, uint8MemBuf, 0, sizeof(uint8MemBuf)/sizeof(uint8MemBuf[0]), sizeof(uint8MemBuf[0]));

    /* Wait for complete, free for other jobs */
    while ( neorv32_spi_rw_busy(&g_neorv32_spi) ) {
        __asm("nop");
    }

    /* stop program counter */
    while ( 1 ) { }
    return 0;
}

For my project would it be a great help to bang not every single byte out, but send more bytes in one job. What do you think, is there a place for this code inside the NEORV32 sw libs?

Thanks for your help!

BR,
Andreas

[akaeba]

Now i'm a little confused. I tryed this with the builded gcc and there compiled the code fine.

../../../sw/lib/include/neorv32_spi.h:77:41: error: expected ',' or '...' before 'this'
| void neorv32_spi_isr(t_neorv32_spi *this);

[akaeba]

@stnolting: Can this caused by the typedef struct t_neorv32_spi? Otherwise i can also pass a void pointer.

[stnolting]

Great work! I will come back to this later... 👍

Now i'm a little confused. I tryed this with the builded gcc and there compiled the code fine.

../../../sw/lib/include/neorv32_spi.h:77:41: error: expected ',' or '...' before 'this'
| void neorv32_spi_isr(t_neorv32_spi *this);

This is a problem that only occurs in the C++ example program. In C++ this is a reserved keyword. Just rename that pointer and gcc will be happy 😉

[akaeba]

Hello Stephan

This is a problem that only occurs in the C++ example program. In C++ this is a reserved keyword. Just rename that pointer and gcc will be happy 😉

Thanks for the hint. Now the question what do you prefer more, void pointer or pointer with an type? From the perspective of compile checks are pointer with an type better.

Nevertheless now it should compile, please let me check the changes with my setup. I drop a message when i performed this tests.

BR,
Andreas

[stnolting]

I am also using the SPI interrupt in one application to move all transfers "to the background" - but my approach is rather "unpretty"... Your approach looks much cleaner and straightforward!

However, I do not know how to handle this... I think your code might be very helpful for others trying to implement interrupt-driven SPI transfers. However, the core libraries are intended to be a minimal HAL only. Furthermore, if we add core library prototypes for interrupt-driven SPI transfers we should also add those for other modules like TWI, SLINK, UART, ...

I think the best way to handle this is to create a new example project in sw/example. Maybe something like demo_spi_irq?! We could put your minimal code from this PR into that folder to have a ready-to-go example and add your SPI interrupt functions as additional .c and .h files in that folder...? 🤔

What do you think about this?

Thanks for the hint. Now the question what do you prefer more, void pointer or pointer with an type? From the perspective of compile checks are pointer with an type better.

I am no expert here, but I think the typed version seems to be a "safer" variant.

[akaeba]

Hi Stephan,

I am no expert here, but I think the typed version seems to be a "safer" variant.

As first step let me to roll back to the typed version.

demo_spi_irq

yes that we can do

I would prefer to place this three functions closer to the core libs, that this is easily seen. Perhaps two files neorv32_spi_irq.c/h

[stnolting]

I would prefer to place this three functions closer to the core libs, that this is easily seen. Perhaps two files neorv32_spi_irq.c/h

I understand that absolutely 😅

But when we have sw/lib/source/neorv32_spi_irq.c there might be the (legal!) question "why don't we have sw/lib/source/neorv32_twi_irq.c"?

Furthermore, your interrupt-driven SPI functions are tailored to the NEORV32 runtime environment. If you have a setup running an RTOS like FreeRTOS you cannot use the NEORV32 RTE at all but you can still use the HAL from the core libs folder. Having an interrupt call back functions there might be misleading in that scenario.

[akaeba]

Mhh valid points :) Ok then do it in your way demo_spi_irq three files in the demo dir. But the filename neorv32_spi_irq.c/h are okay?

BR,
Andreas

[akaeba]

might be the (legal!) question "why don't we have sw/lib/source/neorv32_twi_irq.c"?

Contributors wellcome :D :D :D Just do it :)

[stnolting]

But the filename neorv32_spi_irq.c/h are okay?

Sounds good!

Don't get me wrong, I really like having out-of-the-box support for IRQ-driven transfers! I just think that if we add something to the core libs it should be uniform across all peripherals... Anyways, mabye we should also add a link to the SPI's data sheet section referring the new IRQ-based example.

might be the (legal!) question "why don't we have sw/lib/source/neorv32_twi_irq.c"?

Contributors welcome :D :D :D Just do it :)

I'd be happy! So feel challenged! 😉😅

[akaeba]

Don't get me wrong

No Worry. All fine ;)

I prepare some commits. I think it would take a while.

[akaeba]

Please wait before merge, i need to check.

[stnolting]

Please wait before merge, i need to check.

Sure. Just click "Ready for Review" when you're done.

[akaeba]

Done.

[stnolting]

Thanks for contributing!