From 616bf84db1a4220aea5c08705628b4c3e71601b8 Mon Sep 17 00:00:00 2001 From: Vlad Lungu Date: Thu, 3 Mar 2016 11:46:51 +0200 Subject: spi: intel: fix write failures at low speeds When configured at a frequency of 2MHz, a transaction writing 3 bytes and reading 1 byte fails silently (last byte cannot be read back from the device). Enabling CONFIG_SPI_DEBUG fixes the issue. Scope traces show that the transaction (from /CS assert to /CS deassert) takes 14us and there is activity on the MOSI line after /CS is deasserted. A transaction writing 2 bytes and reading 3 bytes take 22us. The issue is due to the fact that completed() deasserts /CS after the driver has put 3 bytes in the TXFIFO and taken 1 byte from RXFIFO. Just because the last byte made it to the TXFIFO, it doesn't mean that it was put on the MOSI line. The fix: For a transaction sending T bytes and expecting R bytes, let N=max(T,R). Send exactly N bytes and wait for exactly N bytes (or an error). This way, we are sure that all the bytes were sent to the target device. Also: Stop calling pull_data() after every byte sent, it might take a while for a byte to show up in RXFIFO. If RFS bit is set, stop sending bytes (will be really useful with a bigger RFT). Flushing RXFIFO in spi_intel_transceive() is not needed anymore. Change-Id: Ifb06a12b03e3e20d6ace4d9f3a20fc11ec3bb010 Signed-off-by: Vlad Lungu --- drivers/spi/spi_intel.c | 46 +++++++++++++++++++--------------------------- drivers/spi/spi_intel.h | 7 ++++--- 2 files changed, 23 insertions(+), 30 deletions(-) diff --git a/drivers/spi/spi_intel.c b/drivers/spi/spi_intel.c index 94796b686..5c06c2b2d 100644 --- a/drivers/spi/spi_intel.c +++ b/drivers/spi/spi_intel.c @@ -133,16 +133,11 @@ static void completed(struct device *dev, uint32_t error) struct spi_intel_config *info = dev->config->config_info; struct spi_intel_data *spi = dev->driver_data; - if (!((spi->tx_buf == spi->tx_buf_end && !spi->rx_buf) || - (spi->rx_buf == spi->rx_buf_end && !spi->tx_buf) || - (spi->tx_buf == spi->tx_buf_end && - spi->rx_buf == spi->rx_buf_end) || - error)) { + /* if received == trans_len, then transmitted == trans_len */ + if (!(spi->received == spi->trans_len) && !error) { return; } - spi->tx_buf = spi->rx_buf = spi->tx_buf_end = spi->rx_buf_end = NULL; - spi->t_len = spi->r_buf_len = 0; spi->error = error; _spi_control_cs(dev, 0); @@ -163,15 +158,15 @@ static void pull_data(struct device *dev) while (read_sssr(info->regs) & INTEL_SPI_SSSR_RNE) { data = (uint8_t) read_ssdr(info->regs); cnt++; + spi->received++; - if (spi->rx_buf < spi->rx_buf_end) { + if ((spi->received - 1) < spi->r_buf_len) { *(uint8_t *)(spi->rx_buf) = data; spi->rx_buf++; } } - DBG("Pulled: %d (total: %d)\n", - cnt, spi->r_buf_len - (spi->rx_buf_end - spi->rx_buf)); + DBG("Pulled: %d (total: %d)\n", cnt, spi->received); } static void push_data(struct device *dev) @@ -180,12 +175,16 @@ static void push_data(struct device *dev) struct spi_intel_data *spi = dev->driver_data; uint32_t cnt = 0; uint8_t data; + uint32_t status; - while (read_sssr(info->regs) & INTEL_SPI_SSSR_TNF) { - if (spi->tx_buf < spi->tx_buf_end) { + while ((status = read_sssr(info->regs)) & INTEL_SPI_SSSR_TNF) { + if (status & INTEL_SPI_SSSR_RFS) { + break; + } + if (spi->tx_buf && (spi->transmitted < spi->t_buf_len)) { data = *(uint8_t *)(spi->tx_buf); spi->tx_buf++; - } else if (spi->t_len + cnt < spi->r_buf_len) { + } else if (spi->transmitted < spi->trans_len) { data = 0; } else { /* Nothing to push anymore for now */ @@ -195,14 +194,12 @@ static void push_data(struct device *dev) cnt++; DBG("Pushing 1 byte (total: %d)\n", cnt); write_ssdr(data, info->regs); - - pull_data(dev); + spi->transmitted++; } - spi->t_len += cnt; - DBG("Pushed: %d (total: %d)\n", cnt, spi->t_len); + DBG("Pushed: %d (total: %d)\n", cnt, spi->transmitted); - if (spi->tx_buf == spi->tx_buf_end) { + if (spi->transmitted == spi->trans_len) { clear_bit_sscr1_tie(info->regs); } } @@ -262,9 +259,6 @@ static int spi_intel_configure(struct device *dev, /* Configuring the rate */ write_dds_rate(INTEL_SPI_DSS_RATE(config->max_sys_freq), info->regs); - spi->tx_buf = spi->tx_buf_end = spi->rx_buf = spi->rx_buf_end = NULL; - spi->t_len = spi->r_buf_len = 0; - return DEV_OK; } @@ -284,16 +278,14 @@ static int spi_intel_transceive(struct device *dev, return DEV_USED; } - /* Flushing recv fifo */ - spi->rx_buf = spi->rx_buf_end = NULL; - pull_data(dev); - /* Set buffers info */ spi->tx_buf = tx_buf; - spi->tx_buf_end = tx_buf + tx_buf_len; spi->rx_buf = rx_buf; - spi->rx_buf_end = rx_buf + rx_buf_len; + spi->t_buf_len = tx_buf_len; spi->r_buf_len = rx_buf_len; + spi->transmitted = 0; + spi->received = 0; + spi->trans_len = max(tx_buf_len, rx_buf_len); _spi_control_cs(dev, 1); diff --git a/drivers/spi/spi_intel.h b/drivers/spi/spi_intel.h index 533410aae..ae453131a 100644 --- a/drivers/spi/spi_intel.h +++ b/drivers/spi/spi_intel.h @@ -52,11 +52,12 @@ struct spi_intel_data { uint32_t sscr0; uint32_t sscr1; uint8_t *tx_buf; - uint8_t *tx_buf_end; uint8_t *rx_buf; - uint8_t *rx_buf_end; + uint32_t t_buf_len; uint32_t r_buf_len; - uint32_t t_len; + uint32_t transmitted; + uint32_t received; + uint32_t trans_len; }; /* Registers */ -- cgit v1.2.3