Microchip-MPLAB-Harmony / core

Harmony 3 Core
https://onlinedocs.microchip.com/v2/keyword-lookup?keyword=MH3_core&redirect=true
Other
17 stars 12 forks source link

Concurrency issue with drv_at25df.c #8

Closed lucapascarella closed 4 years ago

lucapascarella commented 4 years ago

The _drvat25df.c has a concurrency issue when the SPI interrupt is generated too fast.

Since DRV_AT25DFHandler called by _SPIEventHandler is invoked by the SPI interrupt routine as soon as the transmission is terminated there is an unexpected possibility that the gDrvAT25DFObj.state is modified after the execution of DRV_AT25DFHandler. In this scenario, __DRV_AT25DFHandler handles the default state because _DRV_AT25DF_STATE_READ_CMDADDR is not implemented.

This issue happens frequently when _DRV_AT25DFRead is called in an RTOS context.

A temporary workaround is to set the state to _gDrvAT25DFObj.state = DRV_AT25DF_STATE_READDATA; before call __DRV_AT25DFWriteMemoryAddress in _DRV_AT25DFRead.

`bool DRV_AT25DF_Read( const DRV_HANDLE handle, void* rxData, uint32_t rxDataLength, uint32_t address ) { bool isRequestAccepted = false;

if((handle == DRV_HANDLE_INVALID) || (handle > 0) || (rxData == NULL) \
        || (rxDataLength == 0) || (gDrvAT25DFObj.transferStatus == DRV_AT25DF_TRANSFER_STATUS_BUSY))
{
    return isRequestAccepted;
}

if ((address + rxDataLength) > gDrvAT25DFObj.flashSize)
{
    /* Writing past the flash size results in an error */
    return isRequestAccepted;
}

gDrvAT25DFObj.transferStatus = DRV_AT25DF_TRANSFER_STATUS_BUSY;

/* save the request */
gDrvAT25DFObj.nPendingBytes = rxDataLength;
gDrvAT25DFObj.bufferAddr = rxData;
gDrvAT25DFObj.memoryAddr = address;

// gDrvAT25DFObj.state = DRV_AT25DF_STATE_READ_CMD_ADDR; // Remove this line
gDrvAT25DFObj.state = DRV_AT25DF_STATE_READ_DATA; // Add this line

if (_DRV_AT25DF_WriteMemoryAddress(DRV_SST26_CMD_HIGH_SPEED_READ, address, 1) == true)
{
    // ==> This code may be executed after calling _DRV_AT25DF_Handler() <==
    //gDrvAT25DFObj.state = DRV_AT25DF_STATE_READ_DATA; // Remove this line
    isRequestAccepted = true;
}
else
{
    gDrvAT25DFObj.transferStatus = DRV_AT25DF_TRANSFER_STATUS_ERROR;
}

return isRequestAccepted;

}`

vishalnxt commented 4 years ago

@lucapascarella , Thanks for reporting the issue. It will be fixed in the next release.

dsettu commented 4 years ago

@lucapascarella , This issue is fixed now. Please check this out and close. thank you.