espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-08-14 06:37:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

NcpSpi, Spi-slave: Ensure OT APIs are not called from ISR context (#1806)

Browse Source 
This commit changes the `spi-slave.h` platform functions by modifying
how the callbacks for a completed SPI transaction are used/invoked.

There are two callbacks for when a SPI transaction completes: The
`TransactionCompleteCallback` can be called from ISR context and it
prepares  the next transaction based on current internal `NcpSpi`
state quickly. This callback returns a `bool` to indicate to  platform
spi-slave driver if further processing is required. If `true` is
returned, the platform spi-slave driver is expected to invoke the
newly introduced callback `TransactionProcessCallback` from the same
OS context that all OpenThread APIs and callbacks are called. This
basically, divides the processing into two parts, one part from ISR
and second follow up from OpenThread context.

This change is to address an issue with current model where OpenThread
APIs (e.g., posting a tasklet) could be called from an ISR context
(which was unsafe and could possibly lead to dropping/missing a posted
tasklet).
This commit is contained in:
Abtin Keshavarzian
2017-05-25 14:41:35 -07:00
committed by Jonathan Hui
parent 9073d0d99a
commit c904910f64
4 changed files with 63 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
examples/platforms/posix/spi-stubs.c
+4 -2
View File
@@ -36,9 +36,11 @@
// Spi-slave stubs
otError otPlatSpiSlaveEnable(otPlatSpiSlaveTransactionCompleteCallback aCallback, void *aContext)
otError otPlatSpiSlaveEnable(otPlatSpiSlaveTransactionCompleteCallback aCompleteCallback,
otPlatSpiSlaveTransactionProcessCallback aProcessCallback, void *aContext)
{
(void)aCallback;
(void)aCompleteCallback;
(void)aProcessCallback;
(void)aContext;
fprintf(stderr, "\nNo SPI support for posix platform.");
include/openthread/platform/spi-slave.h
+25 -5
View File
@@ -63,6 +63,11 @@ extern "C" {
* Note that this function is always called at the end of a transaction, even if `otPlatSpiSlavePrepareTransaction()`
* has not yet been called. In such cases, `aOutputBufLen` and `aInputBufLen` will be zero.
*
* This callback can be called from ISR context. The return value from this function indicates if any further
* processing is required. If `TRUE` is returned the platform spi-slave driver implementation must invoke the
* transaction process callback (`aProcessCallback` set in `otPlatSpiSlaveEnable()`) which unlike this callback must be
* called from the same OS context that any other OpenThread API/callback is called.
*
* @param[in] aContext Context pointer passed into `otPlatSpiSlaveEnable()`.
* @param[in] aOutputBuf Value of `aOutputBuf` from last call to `otPlatSpiSlavePrepareTransaction()`.
* @param[in] aOutputBufLen Value of `aOutputBufLen` from last call to `otPlatSpiSlavePrepareTransaction()`.
@@ -70,12 +75,25 @@ extern "C" {
* @param[in] aInputBufLen Value of aInputBufLen from last call to `otPlatSpiSlavePrepareTransaction()`
* @param[in] aTransactionLength Length of the completed transaction, in bytes.
*
* @returns TRUE if after this call returns the platform should invoke the the process callback `aProcessCallback`,
* FALSE if there is nothing to process and no need to invoke the process callback.
*/
typedef void (*otPlatSpiSlaveTransactionCompleteCallback)(void *aContext, uint8_t *aOutputBuf, uint16_t aOutputBufLen,
typedef bool (*otPlatSpiSlaveTransactionCompleteCallback)(void *aContext, uint8_t *aOutputBuf, uint16_t aOutputBufLen,
uint8_t *aInputBuf, uint16_t aInputBufLen,
uint16_t aTransactionLength);
/**
* Invoked after a transaction complete callback is called and returns `TRUE` to do any further processing required.
* Unlike `otPlatSpiSlaveTransactionCompleteCallback` which can be called from any OS context (e.g., ISR), this
* callback MUST be called from the same OS context as any other OpenThread API/callback.
*
* @param[in] aContext Context pointer passed into `otPlatSpiSlaveEnable()`.
*
*/
typedef void (*otPlatSpiSlaveTransactionProcessCallback)(void *aContext);
/**
* Initialize the SPI slave interface.
@@ -84,15 +102,17 @@ typedef void (*otPlatSpiSlaveTransactionCompleteCallback)(void *aContext, uint8_
* If `otPlatSPISlavePrepareTransaction() is not called before the master begins a transaction, the resulting SPI
* transaction will send all `0xFF` bytes and discard all received bytes.
*
* @param[in] aCallback Pointer to transaction complete callback.
* @param[in] aContext Context pointer to be passed to transaction complete callback.
* @param[in] aCompleteCallback Pointer to transaction complete callback.
* @param[in] aProcessCallback Pointer to process callback.
* @param[in] aContext Context pointer to be passed to callbacks.
*
* @retval OT_ERROR_NONE Successfully enabled the SPI Slave interface.
* @retval OT_ERROR_ALREADY SPI Slave interface is already enabled.
* @retval OT_ERROR_FAILED Failed to enable the SPI Slave interface.
*
*/
otError otPlatSpiSlaveEnable(otPlatSpiSlaveTransactionCompleteCallback aCallback, void *aContext);
otError otPlatSpiSlaveEnable(otPlatSpiSlaveTransactionCompleteCallback aCompleteCallback,
otPlatSpiSlaveTransactionProcessCallback aProcessCallback, void *aContext);
/**
* Shutdown and disable the SPI slave interface.
@@ -122,7 +142,7 @@ void otPlatSpiSlaveDisable(void);
* clocks out 30 bytes, the value 30 is passed to the transaction complete callback.
*
* If a `NULL` pointer is passed in as `aOutputBuf` or `aInputBuf` it means that that buffer pointer should not change
* from its previous/current value. In this case, the corresponding length argument should be ignored. For example,
* from its previous/current value. In this case, the corresponding length argument should be ignored. For example,
* `otPlatSpiSlavePrepareTransaction(NULL, 0, aInputBuf, aInputLen, false)` changes the input buffer pointer and its
* length but keeps the output buffer pointer same as before.
*
src/ncp/ncp_spi.cpp
+29 -14
View File
@@ -109,7 +109,6 @@ NcpSpi::NcpSpi(otInstance *aInstance) :
mTxState(kTxStateIdle),
mHandlingRxFrame(false),
mResetFlag(true),
mHandleRxFrameTask(aInstance->mIp6.mTaskletScheduler, &NcpSpi::HandleRxFrame, this),
mPrepareTxFrameTask(aInstance->mIp6.mTaskletScheduler, &NcpSpi::PrepareTxFrame, this),
mSendFrameLen(0)
{
@@ -127,7 +126,7 @@ NcpSpi::NcpSpi(otInstance *aInstance) :
spi_header_set_accept_len(mEmptySendFrameFullAccept, sizeof(mReceiveFrame) - kSpiHeaderLength);
spi_header_set_accept_len(mEmptySendFrameZeroAccept, 0);
otPlatSpiSlaveEnable(&NcpSpi::SpiTransactionComplete, this);
otPlatSpiSlaveEnable(&NcpSpi::SpiTransactionComplete, &NcpSpi::SpiTransactionProcess, this);
// We signal an interrupt on this first transaction to
// make sure that the host processor knows that our
@@ -137,14 +136,14 @@ NcpSpi::NcpSpi(otInstance *aInstance) :
true);
}
void NcpSpi::SpiTransactionComplete(void *aContext, uint8_t *aOutputBuf, uint16_t aOutputBufLen, uint8_t *aInputBuf,
bool NcpSpi::SpiTransactionComplete(void *aContext, uint8_t *aOutputBuf, uint16_t aOutputBufLen, uint8_t *aInputBuf,
uint16_t aInputBufLen, uint16_t aTransactionLength)
{
static_cast<NcpSpi*>(aContext)->SpiTransactionComplete(aOutputBuf, aOutputBufLen, aInputBuf, aInputBufLen,
return static_cast<NcpSpi *>(aContext)->SpiTransactionComplete(aOutputBuf, aOutputBufLen, aInputBuf, aInputBufLen,
aTransactionLength);
}
void NcpSpi::SpiTransactionComplete(uint8_t *aOutputBuf, uint16_t aOutputBufLen, uint8_t *aInputBuf,
bool NcpSpi::SpiTransactionComplete(uint8_t *aOutputBuf, uint16_t aOutputBufLen, uint8_t *aInputBuf,
uint16_t aInputBufLen, uint16_t aTransactionLength)
{
// This may be executed from an interrupt context.
@@ -154,6 +153,7 @@ void NcpSpi::SpiTransactionComplete(uint8_t *aOutputBuf, uint16_t aOutputBufLen,
uint16_t rx_accept_len = 0;
uint16_t tx_data_len = 0;
uint16_t tx_accept_len = 0;
bool shouldProcess = false;
// TODO: Check `PATTERN` bits of `HDR` and ignore frame if not set.
// Holding off on implementing this so as to not cause immediate
@@ -180,7 +180,7 @@ void NcpSpi::SpiTransactionComplete(uint8_t *aOutputBuf, uint16_t aOutputBufLen,
(rx_data_len <= rx_accept_len))
{
mHandlingRxFrame = true;
mHandleRxFrameTask.Post();
shouldProcess = true;
}
if ((mTxState == kTxStateSending) &&
@@ -189,7 +189,7 @@ void NcpSpi::SpiTransactionComplete(uint8_t *aOutputBuf, uint16_t aOutputBufLen,
(tx_data_len <= tx_accept_len))
{
mTxState = kTxStateHandlingSendDone;
mPrepareTxFrameTask.Post();
shouldProcess = true;
}
}
@@ -226,7 +226,28 @@ void NcpSpi::SpiTransactionComplete(uint8_t *aOutputBuf, uint16_t aOutputBufLen,
spi_header_set_accept_len(mSendFrame, sizeof(mReceiveFrame) - kSpiHeaderLength);
}
otPlatSpiSlavePrepareTransaction(aOutputBuf, aOutputBufLen, aInputBuf, aInputBufLen, (mTxState == kTxStateSending));
otPlatSpiSlavePrepareTransaction(aOutputBuf, aOutputBufLen, aInputBuf, aInputBufLen,
(mTxState == kTxStateSending));
return shouldProcess;
}
void NcpSpi::SpiTransactionProcess(void *aContext)
{
static_cast<NcpSpi *>(aContext)->SpiTransactionProcess();
}
void NcpSpi::SpiTransactionProcess(void)
{
if (mTxState == kTxStateHandlingSendDone)
{
mPrepareTxFrameTask.Post();
}
if (mHandlingRxFrame)
{
HandleRxFrame();
}
}
void NcpSpi::TxFrameBufferHasData(void *aContext, NcpFrameBuffer *aNcpFrameBuffer)
@@ -323,11 +344,6 @@ void NcpSpi::PrepareTxFrame(void)
}
}
void NcpSpi::HandleRxFrame(void *aContext)
{
static_cast<NcpSpi *>(aContext)->HandleRxFrame();
}
void NcpSpi::HandleRxFrame(void)
{
// Pass the received frame to base class to process.
@@ -363,7 +379,6 @@ void NcpSpi::HandleRxFrame(void)
// is OK as everything will be set up properly from callback when
// the current transaction is completed.
}
}
} // namespace ot
src/ncp/ncp_spi.hpp
+5 -5
View File
@@ -77,16 +77,17 @@ private:
kTxStateHandlingSendDone // The frame was sent successfully, waiting to prepare the next one (if any).
};
static void SpiTransactionComplete(void *context, uint8_t *aOutputBuf, uint16_t aOutputBufLen, uint8_t *aInputBuf,
static bool SpiTransactionComplete(void *aContext, uint8_t *aOutputBuf, uint16_t aOutputBufLen, uint8_t *aInputBuf,
uint16_t aInputBufLen, uint16_t aTransactionLength);
void SpiTransactionComplete(uint8_t *aOutputBuf, uint16_t aOutputBufLen, uint8_t *aInputBuf, uint16_t aInputBufLen,
bool SpiTransactionComplete(uint8_t *aOutputBuf, uint16_t aOutputBufLen, uint8_t *aInputBuf, uint16_t aInputBufLen,
uint16_t aTransactionLength);
static void HandleRxFrame(void *context);
void HandleRxFrame(void);
static void SpiTransactionProcess(void *aContext);
void SpiTransactionProcess(void);
static void PrepareTxFrame(void *context);
void PrepareTxFrame(void);
void HandleRxFrame(void);
static void TxFrameBufferHasData(void *aContext, NcpFrameBuffer *aNcpFrameBuffer);
@@ -96,7 +97,6 @@ private:
volatile bool mHandlingRxFrame;
volatile bool mResetFlag;
Tasklet mHandleRxFrameTask;
Tasklet mPrepareTxFrameTask;
uint16_t mSendFrameLen;