spi-hdlc-adapter: Improved strategy for how SPI transactions are performed. (#1066)

The big change in this commit is a change in the transmission strategy used.
This change isn't fixing any particular bug, but these changes should improve
performance.

Instead of always using a throw-away transaction to figure out the buffer sizes
for the NCP, we now attempt to send any frame we have in the first transaction.
If the NCP has a response that happens to fit in the size of that packet, it
will be processed.

This change also adds a lot more comments better explaining the more
complicated parts of the code.
This commit is contained in:
Robert Quattlebaum
2016-12-13 14:53:17 -08:00
committed by Jonathan Hui
parent dbc1384db2
commit 0b1d47afde
2 changed files with 137 additions and 35 deletions
+1 -1
View File
@@ -6,7 +6,7 @@ it were an HDLC-lite encoded bidirectional asynchronous serial stream.
It uses the SPI protocol outlined in [Appendix A.2][1] of the Spinel
protocol document.
[1]: https://goo.gl/bwHt5r
[1]: https://goo.gl/gt18O4
## Syntax ##
+136 -34
View File
@@ -140,8 +140,8 @@ static int sHdlcOutputFd = -1;
static int sSpiSpeed = 1000000; // in Hz (default: 1MHz)
static uint8_t sSpiMode = 0;
static int sSpiCsDelay = 20;
static int sSpiTransactionDelay = 200;
static int sSpiCsDelay = 20; // in microseconds
static int sSpiTransactionDelay = 200; // in microseconds
static uint16_t sSpiRxPayloadSize;
static uint8_t sSpiRxFrameBuffer[MAX_FRAME_SIZE + SPI_RX_ALIGN_ALLOWANCE_MAX];
@@ -439,18 +439,20 @@ static void debug_spi_header(const char* hint)
static int push_pull_spi(void)
{
int ret;
uint8_t slave_header;
uint16_t slave_max_rx;
uint16_t slave_data_len;
int spi_xfer_bytes = 0;
uint16_t spi_xfer_bytes = 5;
const uint8_t* spiRxFrameBuffer = NULL;
sSpiTxFlowControl = false;
// Fetch the slave's buffer sizes.
// Zero out our max rx and data len
// so that the slave doesn't think
// we are actually trying to transfer
// data.
/// -- FIRST TRANSACTION --------------------------------------------------
// The purpose of the first transaction is to attempt to
// send any transactions we have queued and fetch the slave's
// buffer sizes.
if (sSpiValidFrameCount == 0)
{
spi_header_set_flag_byte(sSpiTxFrameBuffer, SPI_HEADER_RESET_FLAG|SPI_HEADER_PATTERN_VALUE);
@@ -459,20 +461,57 @@ static int push_pull_spi(void)
{
spi_header_set_flag_byte(sSpiTxFrameBuffer, SPI_HEADER_PATTERN_VALUE);
}
// Zero out our max rx and data len
// so that the slave doesn't think
// we are actually trying to transfer
// data.
spi_header_set_accept_len(sSpiTxFrameBuffer, 0);
spi_header_set_data_len(sSpiTxFrameBuffer, 0);
ret = do_spi_xfer(0);
if (sSpiTxIsReady)
{
// Go ahead and try to immediately send a frame if we have it queued up.
spi_header_set_data_len(sSpiTxFrameBuffer, sSpiTxPayloadSize);
if (sSpiTxPayloadSize > spi_xfer_bytes)
{
spi_xfer_bytes = sSpiTxPayloadSize;
}
}
// If we aren't already processing a received frame, we
// can also handle receiving the next frame if its length
// is equal to or less than the size of what we are
// trying to transmit above.
if (sSpiRxPayloadSize == 0) {
spi_header_set_accept_len(sSpiTxFrameBuffer, spi_xfer_bytes);
}
// Perform the first SPI transaction.
ret = do_spi_xfer(spi_xfer_bytes);
if (ret < 0)
{
perror("do_spi_xfer");
// Print out a helpful error message for
// a common error.
if ( (sSpiCsDelay != 0)
&& (errno == EINVAL)
) {
syslog(LOG_ERR, "SPI ioctl failed with EINVAL. Try adding `--spi-cs-delay=0` to command line arguments.");
}
goto bail;
}
// Account for misalignment (0xFF bytes at the start)
spiRxFrameBuffer = get_real_rx_frame_start();
debug_spi_header("push_pull_1");
if (spi_header_get_flag_byte(spiRxFrameBuffer) == 0xFF)
slave_header = spi_header_get_flag_byte(spiRxFrameBuffer);
if ((slave_header == 0xFF) || (slave_header == 0x00))
{
// Device is off or in a bad state.
sSpiTxFlowControl = true;
@@ -484,14 +523,16 @@ static int push_pull_spi(void)
slave_max_rx = spi_header_get_accept_len(spiRxFrameBuffer);
slave_data_len = spi_header_get_data_len(spiRxFrameBuffer);
if ( (slave_max_rx > MAX_FRAME_SIZE)
if ( ((slave_header & SPI_HEADER_PATTERN_MASK) != SPI_HEADER_PATTERN_VALUE)
|| (slave_max_rx > MAX_FRAME_SIZE)
|| (slave_data_len > MAX_FRAME_SIZE)
)
{
sSpiTxFlowControl = true;
syslog(
LOG_INFO,
"Gibberish in header (max_rx:%d, data_len:%d)",
"Gibberish in header (h:0x%02X, max_rx:0x%04X, data_len:0x%04X)",
slave_header,
slave_max_rx,
slave_data_len
);
@@ -500,26 +541,60 @@ static int push_pull_spi(void)
sSpiValidFrameCount++;
if (!sSpiTxIsReady && (slave_data_len == 0))
// Handle received packet, if any.
if ( (sSpiRxPayloadSize == 0)
&& (slave_data_len != 0)
&& (slave_data_len <= spi_header_get_accept_len(sSpiTxFrameBuffer))
) {
// We have received a packet. Set sSpiRxPayloadSize so that
// the packet will eventually get queued up by push_hdlc().
sSpiRxPayloadSize = slave_data_len;
slave_data_len = 0;
}
if (sSpiTxIsReady)
{
// Nothing to do.
// Handle transmitted packet.
if (spi_header_get_data_len(sSpiTxFrameBuffer) <= slave_max_rx)
{
// Outbound packet has been successfully transmitted. Clear
// sSpiTxPayloadSize and sSpiTxIsReady so that pull_hdlc() can
// pull another packet for us to send.
sSpiTxIsReady = false;
sSpiTxPayloadSize = 0;
spi_header_set_data_len(sSpiTxFrameBuffer, 0);
} else {
// The slave Wasn't ready for what we had to
// send them. Turn on rate limiting so that we
// don't waste a ton of CPU bombarding them
// with useless SPI transfers.
sSpiTxFlowControl = true;
}
}
if (slave_data_len == 0)
{
// Nothing else to do.
goto bail;
}
if ( sSpiTxIsReady
&& (sSpiTxPayloadSize <= slave_max_rx)
)
/// -- SECOND TRANSACTION ------------------------------------------------
// The purpose of the second transaction is to attempt to
// fetch any packets that the slave has for us that didn't
// fit in the first transaction.
spi_header_set_flag_byte(sSpiTxFrameBuffer, SPI_HEADER_PATTERN_VALUE);
spi_header_set_accept_len(sSpiTxFrameBuffer, 0);
if (sSpiTxIsReady)
{
spi_xfer_bytes = sSpiTxPayloadSize;
spi_header_set_data_len(sSpiTxFrameBuffer, sSpiTxPayloadSize);
}
else if (sSpiTxIsReady && (sSpiTxPayloadSize > slave_max_rx))
{
// The slave isn't ready for what we have to
// send them. Turn on rate limiting so that we
// don't waste a ton of CPU bombarding them
// with useless SPI transfers.
sSpiTxFlowControl = true;
} else {
spi_xfer_bytes = 0;
}
if ( (slave_data_len != 0)
@@ -533,23 +608,34 @@ static int push_pull_spi(void)
}
}
// Optionally delay a short period to give
// the slave time to get its affairs in order.
usleep((unsigned int)sSpiTransactionDelay);
spi_header_set_flag_byte(sSpiTxFrameBuffer, SPI_HEADER_PATTERN_VALUE);
// This is the real transfer.
// Perform the second SPI transaction.
ret = do_spi_xfer(spi_xfer_bytes);
if (ret < 0)
{
perror("do_spi_xfer");
// Print out a helpful error message for
// a common error.
if ( (sSpiCsDelay != 0)
&& (errno == EINVAL)
) {
syslog(LOG_ERR, "SPI ioctl failed with EINVAL. Try adding `--spi-cs-delay=0` to command line arguments.");
}
goto bail;
}
// Account for misalignment (0xFF bytes at the start)
spiRxFrameBuffer = get_real_rx_frame_start();
debug_spi_header("push_pull_2");
if (spi_header_get_flag_byte(spiRxFrameBuffer) == 0xFF)
slave_header = spi_header_get_flag_byte(spiRxFrameBuffer);
if ((slave_header == 0xFF) || (slave_header == 0x00))
{
// Device is off or in a bad state.
sSpiTxFlowControl = true;
@@ -561,14 +647,16 @@ static int push_pull_spi(void)
slave_max_rx = spi_header_get_accept_len(spiRxFrameBuffer);
slave_data_len = spi_header_get_data_len(spiRxFrameBuffer);
if ( (slave_max_rx > MAX_FRAME_SIZE)
if ( (slave_header != SPI_HEADER_PATTERN_VALUE)
|| (slave_max_rx > MAX_FRAME_SIZE)
|| (slave_data_len > MAX_FRAME_SIZE)
)
{
sSpiTxFlowControl = true;
syslog(
LOG_INFO,
"Gibberish in header (max_rx:%d, data_len:%d)",
"Gibberish in header (h:0x%02X, max_rx:0x%04X, data_len:0x%04X) (2)",
slave_header,
slave_max_rx,
slave_data_len
);
@@ -593,6 +681,7 @@ static int push_pull_spi(void)
// pull another packet for us to send.
sSpiTxIsReady = false;
sSpiTxPayloadSize = 0;
sSpiTxFlowControl = false;
}
bail:
@@ -1496,7 +1585,10 @@ int main(int argc, char *argv[])
}
else if (sSpiTxFlowControl)
{
// We are being rate-limited by the NCP.
// We are being rate-limited by the NCP. This is
// fairly normal behavior. We poll because we
// won't get an interrupt unless the NCP happens
// to be trying to send us something.
timeout_ms = SPI_POLL_PERIOD_MSEC;
if (!did_print_rate_limit_log) {
@@ -1507,14 +1599,18 @@ int main(int argc, char *argv[])
}
else
{
// We have data to send to the slave. Unless we
// are being rate-limited, proceed immediately.
// We have data to send to the slave. Since we
// are not being rate-limited, proceed immediately.
timeout_ms = 0;
did_print_rate_limit_log = false;
}
if (sSpiRxPayloadSize != 0)
{
// We have data that we are waiting to send out
// of the HDLC descriptor, so we need to wait
// for that to clear out before we can do anything
// else.
FD_SET(sHdlcOutputFd, &write_set);
}
@@ -1530,12 +1626,18 @@ int main(int argc, char *argv[])
}
else
{
// The interrupt pin was not asserted,
// so we wait for the interrupt pin to
// be asserted by adding it to the error
// set.
FD_SET(sIntGpioValueFd, &error_set);
}
}
else if (timeout_ms > SPI_POLL_PERIOD_MSEC)
{
// In this case we don't have an interrupt, so
// we revert to SPI polling.
timeout_ms = SPI_POLL_PERIOD_MSEC;
}