Missing critical section in SX1272WriteBuffer and SX1272ReadBuffer

[RomanJasmann]

The complete SPI transaction in SX1272WriteBuffer and SX1272ReadBuffer in sx1272.c is not protected by a critical section. SpiInOut in spi-board.c does protect the SPI transfer by a critical section, but the critical section does not include the writing of the NSS signal. Consider the following situation:

1. SX1272WriteBuffer is called.
2. The NSS signal is set to 0.
3. In between GpioWrite( &SX1272.Spi.Nss, 0 ); and SpiInOut( &SX1272.Spi, addr | 0x80 );, a radio interrupt happens and the interrupt handler also calls SX1272WriteBuffer or SX1272ReadBuffer. At the end of SX1272WriteBuffer or SX1272ReadBuffer, the NSS signal is set to 1.
4. The interrupt handler returns
5. The SPI transfer SpiInOut( &SX1272.Spi, addr | 0x80 ); fails because the NSS pin was set to 1 in the interrupt handler.

void SX1272WriteBuffer( uint32_t addr, uint8_t *buffer, uint8_t size )
{
    uint8_t i;

    //NSS = 0;
    GpioWrite( &SX1272.Spi.Nss, 0 );

    SpiInOut( &SX1272.Spi, addr | 0x80 );
    for( i = 0; i < size; i++ )
    {
        SpiInOut( &SX1272.Spi, buffer[i] );
    }

    //NSS = 1;
    GpioWrite( &SX1272.Spi.Nss, 1 );
}

void SX1272ReadBuffer( uint32_t addr, uint8_t *buffer, uint8_t size )
{
    uint8_t i;

    //NSS = 0;
    GpioWrite( &SX1272.Spi.Nss, 0 );

    SpiInOut( &SX1272.Spi, addr & 0x7F );

    for( i = 0; i < size; i++ )
    {
        buffer[i] = SpiInOut( &SX1272.Spi, 0 );
    }

    //NSS = 1;
    GpioWrite( &SX1272.Spi.Nss, 1 );
}

uint16_t SpiInOut( Spi_t *obj, uint16_t outData )
{
    uint8_t rxData = 0;

    if( ( obj == NULL ) || ( SpiHandle[obj->SpiId].Instance ) == NULL )
    {
        assert_param( LMN_STATUS_ERROR );
    }

    __HAL_SPI_ENABLE( &SpiHandle[obj->SpiId] );

    CRITICAL_SECTION_BEGIN( );

    HAL_SPI_TransmitReceive( &SpiHandle[obj->SpiId], ( uint8_t* )&outData, &rxData, 1, HAL_MAX_DELAY );

    CRITICAL_SECTION_END( );

    return( rxData );
}

Fix:

void SX1272WriteBuffer( uint32_t addr, uint8_t *buffer, uint8_t size )
{
    uint8_t i;

    CRITICAL_SECTION_BEGIN( );

    //NSS = 0;
    GpioWrite( &SX1272.Spi.Nss, 0 );

    SpiInOut( &SX1272.Spi, addr | 0x80 );
    for( i = 0; i < size; i++ )
    {
        SpiInOut( &SX1272.Spi, buffer[i] );
    }

    //NSS = 1;
    GpioWrite( &SX1272.Spi.Nss, 1 );

    CRITICAL_SECTION_END( );
}

void SX1272ReadBuffer( uint32_t addr, uint8_t *buffer, uint8_t size )
{
    uint8_t i;

    CRITICAL_SECTION_BEGIN( );

    //NSS = 0;
    GpioWrite( &SX1272.Spi.Nss, 0 );

    SpiInOut( &SX1272.Spi, addr & 0x7F );

    for( i = 0; i < size; i++ )
    {
        buffer[i] = SpiInOut( &SX1272.Spi, 0 );
    }

    //NSS = 1;
    GpioWrite( &SX1272.Spi.Nss, 1 );

    CRITICAL_SECTION_END( );
}

uint16_t SpiInOut( Spi_t *obj, uint16_t outData )
{
    uint8_t rxData = 0;

    if( ( obj == NULL ) || ( SpiHandle[obj->SpiId].Instance ) == NULL )
    {
        assert_param( LMN_STATUS_ERROR );
    }

    __HAL_SPI_ENABLE( &SpiHandle[obj->SpiId] );

    HAL_SPI_TransmitReceive( &SpiHandle[obj->SpiId], ( uint8_t* )&outData, &rxData, 1, HAL_MAX_DELAY );

    return( rxData );
}