From 6aa800ca65b1788e8c2e4ffd1e57a4c4355fe0ed Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Wed, 17 Feb 2016 14:28:48 +0100 Subject: [PATCH 01/11] spi: imx: use proper dev_* functions for driver messages Add a struct device * member to the private driver data and use it to print messages using dev_* functions rather than pr_*. Signed-off-by: Sascha Hauer Signed-off-by: Mark Brown --- drivers/spi/spi-imx.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index d98c33cb64f9..7ac206b8cc93 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -86,6 +86,7 @@ struct spi_imx_devtype_data { struct spi_imx_data { struct spi_bitbang bitbang; + struct device *dev; struct completion xfer_done; void __iomem *base; @@ -250,14 +251,15 @@ static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi, #define MX51_ECSPI_TESTREG_LBC BIT(31) /* MX51 eCSPI */ -static unsigned int mx51_ecspi_clkdiv(unsigned int fin, unsigned int fspi, - unsigned int *fres) +static unsigned int mx51_ecspi_clkdiv(struct spi_imx_data *spi_imx, + unsigned int fspi, unsigned int *fres) { /* * there are two 4-bit dividers, the pre-divider divides by * $pre, the post-divider by 2^$post */ unsigned int pre, post; + unsigned int fin = spi_imx->spi_clk; if (unlikely(fspi > fin)) return 0; @@ -270,14 +272,14 @@ static unsigned int mx51_ecspi_clkdiv(unsigned int fin, unsigned int fspi, post = max(4U, post) - 4; if (unlikely(post > 0xf)) { - pr_err("%s: cannot set clock freq: %u (base freq: %u)\n", - __func__, fspi, fin); + dev_err(spi_imx->dev, "cannot set clock freq: %u (base freq: %u)\n", + fspi, fin); return 0xff; } pre = DIV_ROUND_UP(fin, fspi << post) - 1; - pr_debug("%s: fin: %u, fspi: %u, post: %u, pre: %u\n", + dev_dbg(spi_imx->dev, "%s: fin: %u, fspi: %u, post: %u, pre: %u\n", __func__, fin, fspi, post, pre); /* Resulting frequency for the SCLK line. */ @@ -330,7 +332,7 @@ static int __maybe_unused mx51_ecspi_config(struct spi_imx_data *spi_imx, ctrl |= MX51_ECSPI_CTRL_MODE_MASK; /* set clock speed */ - ctrl |= mx51_ecspi_clkdiv(spi_imx->spi_clk, config->speed_hz, &clk); + ctrl |= mx51_ecspi_clkdiv(spi_imx, config->speed_hz, &clk); /* set chip select to use */ ctrl |= MX51_ECSPI_CTRL_CS(config->cs); @@ -977,18 +979,14 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, timeout = wait_for_completion_timeout(&spi_imx->dma_tx_completion, IMX_DMA_TIMEOUT); if (!timeout) { - pr_warn("%s %s: I/O Error in DMA TX\n", - dev_driver_string(&master->dev), - dev_name(&master->dev)); + dev_err(spi_imx->dev, "I/O Error in DMA TX\n"); dmaengine_terminate_all(master->dma_tx); dmaengine_terminate_all(master->dma_rx); } else { timeout = wait_for_completion_timeout( &spi_imx->dma_rx_completion, IMX_DMA_TIMEOUT); if (!timeout) { - pr_warn("%s %s: I/O Error in DMA RX\n", - dev_driver_string(&master->dev), - dev_name(&master->dev)); + dev_err(spi_imx->dev, "I/O Error in DMA RX\n"); spi_imx->devtype_data->reset(spi_imx); dmaengine_terminate_all(master->dma_rx); } @@ -1009,9 +1007,7 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, return ret; no_dma: - pr_warn_once("%s %s: DMA not available, falling back to PIO\n", - dev_driver_string(&master->dev), - dev_name(&master->dev)); + dev_warn_once(spi_imx->dev, "DMA not available, falling back to PIO\n"); return -EAGAIN; } @@ -1141,6 +1137,7 @@ static int spi_imx_probe(struct platform_device *pdev) spi_imx = spi_master_get_devdata(master); spi_imx->bitbang.master = master; + spi_imx->dev = &pdev->dev; spi_imx->devtype_data = of_id ? of_id->data : (struct spi_imx_devtype_data *)pdev->id_entry->driver_data; From 4bfe927a11b18db447706507654aef83c9ef15c1 Mon Sep 17 00:00:00 2001 From: Anton Bondarenko Date: Fri, 19 Feb 2016 08:43:03 +0100 Subject: [PATCH 02/11] spi: imx: replace fixed timeout with calculated Fixed timeout value can fire while transaction is ongoing. This may happen because there are no strict requirements on SPI transaction duration. Dynamic timeout value is generated based on SCLK and transaction size. There is also 4 * SCLK delay between TX bursts related to HW internal CS change. Signed-off-by: Anton Bondarenko Signed-off-by: Sascha Hauer Signed-off-by: Mark Brown --- drivers/spi/spi-imx.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index c5eeb846d9fd..6497fc9c2735 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -56,7 +56,6 @@ /* The maximum bytes that a sdma BD can transfer.*/ #define MAX_SDMA_BD_BYTES (1 << 15) -#define IMX_DMA_TIMEOUT (msecs_to_jiffies(3000)) struct spi_imx_config { unsigned int speed_hz; unsigned int bpw; @@ -93,6 +92,7 @@ struct spi_imx_data { struct clk *clk_per; struct clk *clk_ipg; unsigned long spi_clk; + unsigned int spi_bus_clk; unsigned int count; void (*tx)(struct spi_imx_data *); @@ -333,6 +333,7 @@ static int __maybe_unused mx51_ecspi_config(struct spi_imx_data *spi_imx, /* set clock speed */ ctrl |= mx51_ecspi_clkdiv(spi_imx, config->speed_hz, &clk); + spi_imx->spi_bus_clk = clk; /* set chip select to use */ ctrl |= MX51_ECSPI_CTRL_CS(config->cs); @@ -915,11 +916,26 @@ static void spi_imx_dma_tx_callback(void *cookie) complete(&spi_imx->dma_tx_completion); } +static int spi_imx_calculate_timeout(struct spi_imx_data *spi_imx, int size) +{ + unsigned long timeout = 0; + + /* Time with actual data transfer and CS change delay related to HW */ + timeout = (8 + 4) * size / spi_imx->spi_bus_clk; + + /* Add extra second for scheduler related activities */ + timeout += 1; + + /* Double calculated timeout */ + return msecs_to_jiffies(2 * timeout * MSEC_PER_SEC); +} + static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, struct spi_transfer *transfer) { struct dma_async_tx_descriptor *desc_tx = NULL, *desc_rx = NULL; int ret; + unsigned long transfer_timeout; unsigned long timeout; struct spi_master *master = spi_imx->bitbang.master; struct sg_table *tx = &transfer->tx_sg, *rx = &transfer->rx_sg; @@ -966,16 +982,18 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, dma_async_issue_pending(master->dma_tx); spi_imx->devtype_data->trigger(spi_imx); + transfer_timeout = spi_imx_calculate_timeout(spi_imx, transfer->len); + /* Wait SDMA to finish the data transfer.*/ timeout = wait_for_completion_timeout(&spi_imx->dma_tx_completion, - IMX_DMA_TIMEOUT); + transfer_timeout); if (!timeout) { dev_err(spi_imx->dev, "I/O Error in DMA TX\n"); dmaengine_terminate_all(master->dma_tx); dmaengine_terminate_all(master->dma_rx); } else { timeout = wait_for_completion_timeout( - &spi_imx->dma_rx_completion, IMX_DMA_TIMEOUT); + &spi_imx->dma_rx_completion, transfer_timeout); if (!timeout) { dev_err(spi_imx->dev, "I/O Error in DMA RX\n"); spi_imx->devtype_data->reset(spi_imx); From 99f1cf1c0c2ccdfa251a55cd28e3004963bf6e1a Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Tue, 23 Feb 2016 10:23:50 +0100 Subject: [PATCH 03/11] spi: imx: drop fallback to PIO At the moment the driver decides to fallback to PIO mode the buffers are already mapped for DMA. It's a bug to access them with the CPU afterwards, so we cannot just fallback to PIO mode. It should not be necessary anyway, since we only use DMA when we verified that it's possible in the fist place, so when prep_slave_sg fails it's a bug, either in the SDMA driver or in the can_dma implementation. Signed-off-by: Sascha Hauer Signed-off-by: Mark Brown --- drivers/spi/spi-imx.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 6497fc9c2735..a61b1b140523 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -945,7 +945,7 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, tx->sgl, tx->nents, DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK); if (!desc_tx) - goto tx_nodma; + return -EINVAL; desc_tx->callback = spi_imx_dma_tx_callback; desc_tx->callback_param = (void *)spi_imx; @@ -956,8 +956,10 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, desc_rx = dmaengine_prep_slave_sg(master->dma_rx, rx->sgl, rx->nents, DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT | DMA_CTRL_ACK); - if (!desc_rx) - goto rx_nodma; + if (!desc_rx) { + dmaengine_terminate_all(master->dma_tx); + return -EINVAL; + } desc_rx->callback = spi_imx_dma_rx_callback; desc_rx->callback_param = (void *)spi_imx; @@ -1010,12 +1012,6 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, ret = transfer->len; return ret; - -rx_nodma: - dmaengine_terminate_all(master->dma_tx); -tx_nodma: - dev_warn_once(spi_imx->dev, "DMA not available, falling back to PIO\n"); - return -EAGAIN; } static int spi_imx_pio_transfer(struct spi_device *spi, @@ -1042,15 +1038,12 @@ static int spi_imx_pio_transfer(struct spi_device *spi, static int spi_imx_transfer(struct spi_device *spi, struct spi_transfer *transfer) { - int ret; struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master); if (spi_imx->bitbang.master->can_dma && spi_imx_can_dma(spi_imx->bitbang.master, spi, transfer)) { spi_imx->usedma = true; - ret = spi_imx_dma_transfer(spi_imx, transfer); - if (ret != -EAGAIN) - return ret; + return spi_imx_dma_transfer(spi_imx, transfer); } spi_imx->usedma = false; From c008a800716256300c8a62e781a8897a8d3bce1e Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Wed, 24 Feb 2016 09:20:26 +0100 Subject: [PATCH 04/11] spi: imx: initialize usedma earlier The SoC specific config function does not know if DMA will be used or not. This information will be useful to configure the SPI controller correctly for DMA in following patches, so initialize the usedma variable before calling into the SoC specific config function. Signed-off-by: Sascha Hauer Signed-off-by: Mark Brown --- drivers/spi/spi-imx.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index a61b1b140523..5792918aa751 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -815,6 +815,11 @@ static int spi_imx_setupxfer(struct spi_device *spi, spi_imx->tx = spi_imx_buf_tx_u32; } + if (spi_imx_can_dma(spi_imx->bitbang.master, spi, t)) + spi_imx->usedma = 1; + else + spi_imx->usedma = 0; + spi_imx->devtype_data->config(spi_imx, &config); return 0; @@ -1040,14 +1045,10 @@ static int spi_imx_transfer(struct spi_device *spi, { struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master); - if (spi_imx->bitbang.master->can_dma && - spi_imx_can_dma(spi_imx->bitbang.master, spi, transfer)) { - spi_imx->usedma = true; + if (spi_imx->usedma) return spi_imx_dma_transfer(spi_imx, transfer); - } - spi_imx->usedma = false; - - return spi_imx_pio_transfer(spi, transfer); + else + return spi_imx_pio_transfer(spi, transfer); } static int spi_imx_setup(struct spi_device *spi) From 2b0fd069ec159d8234c730855eb892104ae2edc9 Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Wed, 24 Feb 2016 09:20:27 +0100 Subject: [PATCH 05/11] spi: imx: drop unnecessary read/modify/write When the MX51_ECSPI_DMA is configured we control every single bit of the register, so there's no need to read/modify/write it. Instead just write the value we want to have in the register. Also, drop unnecessary check if we are actually doing DMA. The values written to the register have no effect in PIO mode and value written there during the last DMA transfer is still in the register, so we can equally well always write a value. Signed-off-by: Sascha Hauer Signed-off-by: Mark Brown --- drivers/spi/spi-imx.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 5792918aa751..ec033043734a 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -240,9 +240,9 @@ static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi, #define MX51_ECSPI_DMA_RXT_WML_OFFSET 24 #define MX51_ECSPI_DMA_RXT_WML_MASK (0x3F << 24) -#define MX51_ECSPI_DMA_TEDEN_OFFSET 7 -#define MX51_ECSPI_DMA_RXDEN_OFFSET 23 -#define MX51_ECSPI_DMA_RXTDEN_OFFSET 31 +#define MX51_ECSPI_DMA_TEDEN (1 << 7) +#define MX51_ECSPI_DMA_RXDEN (1 << 23) +#define MX51_ECSPI_DMA_RXTDEN (1 << 31) #define MX51_ECSPI_STAT 0x18 #define MX51_ECSPI_STAT_RR (1 << 3) @@ -318,8 +318,7 @@ static void __maybe_unused mx51_ecspi_trigger(struct spi_imx_data *spi_imx) static int __maybe_unused mx51_ecspi_config(struct spi_imx_data *spi_imx, struct spi_imx_config *config) { - u32 ctrl = MX51_ECSPI_CTRL_ENABLE, cfg = 0, dma = 0; - u32 tx_wml_cfg, rx_wml_cfg, rxt_wml_cfg; + u32 ctrl = MX51_ECSPI_CTRL_ENABLE, cfg = 0; u32 clk = config->speed_hz, delay, reg; /* @@ -392,22 +391,12 @@ static int __maybe_unused mx51_ecspi_config(struct spi_imx_data *spi_imx, * Configure the DMA register: setup the watermark * and enable DMA request. */ - if (spi_imx->dma_is_inited) { - dma = readl(spi_imx->base + MX51_ECSPI_DMA); - rx_wml_cfg = spi_imx->wml << MX51_ECSPI_DMA_RX_WML_OFFSET; - tx_wml_cfg = spi_imx->wml << MX51_ECSPI_DMA_TX_WML_OFFSET; - rxt_wml_cfg = spi_imx->wml << MX51_ECSPI_DMA_RXT_WML_OFFSET; - dma = (dma & ~MX51_ECSPI_DMA_TX_WML_MASK - & ~MX51_ECSPI_DMA_RX_WML_MASK - & ~MX51_ECSPI_DMA_RXT_WML_MASK) - | rx_wml_cfg | tx_wml_cfg | rxt_wml_cfg - |(1 << MX51_ECSPI_DMA_TEDEN_OFFSET) - |(1 << MX51_ECSPI_DMA_RXDEN_OFFSET) - |(1 << MX51_ECSPI_DMA_RXTDEN_OFFSET); - - writel(dma, spi_imx->base + MX51_ECSPI_DMA); - } + writel(spi_imx->wml << MX51_ECSPI_DMA_RX_WML_OFFSET | + spi_imx->wml << MX51_ECSPI_DMA_TX_WML_OFFSET | + spi_imx->wml << MX51_ECSPI_DMA_RXT_WML_OFFSET | + MX51_ECSPI_DMA_TEDEN | MX51_ECSPI_DMA_RXDEN | + MX51_ECSPI_DMA_RXTDEN, spi_imx->base + MX51_ECSPI_DMA); return 0; } From 882f329f31cb7317811fe084e0b0642954649bf5 Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Wed, 24 Feb 2016 09:20:28 +0100 Subject: [PATCH 06/11] spi: imx: drop unncessary dma_is_inited variable There's no need for an extra dma_is_inited variable when we can equally well check for the existence of a DMA channel. Signed-off-by: Sascha Hauer Signed-off-by: Mark Brown --- drivers/spi/spi-imx.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index ec033043734a..567a2426e314 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -102,7 +102,6 @@ struct spi_imx_data { unsigned int txfifo; /* number of words pushed in tx FIFO */ /* DMA */ - unsigned int dma_is_inited; unsigned int dma_finished; bool usedma; u32 wml; @@ -205,7 +204,7 @@ static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi, { struct spi_imx_data *spi_imx = spi_master_get_devdata(master); - if (spi_imx->dma_is_inited && transfer->len >= spi_imx->wml && + if (master->dma_rx && transfer->len >= spi_imx->wml && (transfer->len % spi_imx->wml) == 0) return true; return false; @@ -827,8 +826,6 @@ static void spi_imx_sdma_exit(struct spi_imx_data *spi_imx) dma_release_channel(master->dma_tx); master->dma_tx = NULL; } - - spi_imx->dma_is_inited = 0; } static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx, @@ -888,7 +885,6 @@ static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx, master->max_dma_len = MAX_SDMA_BD_BYTES; spi_imx->bitbang.master->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX; - spi_imx->dma_is_inited = 1; return 0; err: From f12ae171c7ce8633a6dd6dea4c50fd6cb72f433a Mon Sep 17 00:00:00 2001 From: Anton Bondarenko Date: Wed, 24 Feb 2016 09:20:29 +0100 Subject: [PATCH 07/11] spi: imx: add support for all SPI word width for DMA DMA transfer for SPI was limited to up to 8 bits word size until now. Sync in SPI burst size and DMA bus width is necessary to correctly support 16 and 32 BPW. Signed-off-by: Anton Bondarenko Signed-off-by: Sascha Hauer Signed-off-by: Mark Brown --- drivers/spi/spi-imx.c | 118 ++++++++++++++++++++++++++++++++---------- 1 file changed, 91 insertions(+), 27 deletions(-) diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 567a2426e314..15b23f0b83e8 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -89,11 +89,15 @@ struct spi_imx_data { struct completion xfer_done; void __iomem *base; + unsigned long base_phys; + struct clk *clk_per; struct clk *clk_ipg; unsigned long spi_clk; unsigned int spi_bus_clk; + unsigned int bytes_per_word; + unsigned int count; void (*tx)(struct spi_imx_data *); void (*rx)(struct spi_imx_data *); @@ -199,15 +203,35 @@ static unsigned int spi_imx_clkdiv_2(unsigned int fin, return 7; } +static int spi_imx_bytes_per_word(const int bpw) +{ + return DIV_ROUND_UP(bpw, BITS_PER_BYTE); +} + static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi, struct spi_transfer *transfer) { struct spi_imx_data *spi_imx = spi_master_get_devdata(master); + unsigned int bpw = transfer->bits_per_word; - if (master->dma_rx && transfer->len >= spi_imx->wml && - (transfer->len % spi_imx->wml) == 0) - return true; - return false; + if (!master->dma_rx) + return false; + + if (!bpw) + bpw = spi->bits_per_word; + + bpw = spi_imx_bytes_per_word(bpw); + + if (bpw != 1 && bpw != 2 && bpw != 4) + return false; + + if (transfer->len < spi_imx->wml * bpw) + return false; + + if (transfer->len % (spi_imx->wml * bpw)) + return false; + + return true; } #define MX51_ECSPI_CTRL 0x08 @@ -775,11 +799,63 @@ static irqreturn_t spi_imx_isr(int irq, void *dev_id) return IRQ_HANDLED; } +static int spi_imx_dma_configure(struct spi_master *master, + int bytes_per_word) +{ + int ret; + enum dma_slave_buswidth buswidth; + struct dma_slave_config rx = {}, tx = {}; + struct spi_imx_data *spi_imx = spi_master_get_devdata(master); + + if (bytes_per_word == spi_imx->bytes_per_word) + /* Same as last time */ + return 0; + + switch (bytes_per_word) { + case 4: + buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES; + break; + case 2: + buswidth = DMA_SLAVE_BUSWIDTH_2_BYTES; + break; + case 1: + buswidth = DMA_SLAVE_BUSWIDTH_1_BYTE; + break; + default: + return -EINVAL; + } + + tx.direction = DMA_MEM_TO_DEV; + tx.dst_addr = spi_imx->base_phys + MXC_CSPITXDATA; + tx.dst_addr_width = buswidth; + tx.dst_maxburst = spi_imx->wml; + ret = dmaengine_slave_config(master->dma_tx, &tx); + if (ret) { + dev_err(spi_imx->dev, "TX dma configuration failed with %d\n", ret); + return ret; + } + + rx.direction = DMA_DEV_TO_MEM; + rx.src_addr = spi_imx->base_phys + MXC_CSPIRXDATA; + rx.src_addr_width = buswidth; + rx.src_maxburst = spi_imx->wml; + ret = dmaengine_slave_config(master->dma_rx, &rx); + if (ret) { + dev_err(spi_imx->dev, "RX dma configuration failed with %d\n", ret); + return ret; + } + + spi_imx->bytes_per_word = bytes_per_word; + + return 0; +} + static int spi_imx_setupxfer(struct spi_device *spi, struct spi_transfer *t) { struct spi_imx_data *spi_imx = spi_master_get_devdata(spi->master); struct spi_imx_config config; + int ret; config.bpw = t ? t->bits_per_word : spi->bits_per_word; config.speed_hz = t ? t->speed_hz : spi->max_speed_hz; @@ -808,6 +884,13 @@ static int spi_imx_setupxfer(struct spi_device *spi, else spi_imx->usedma = 0; + if (spi_imx->usedma) { + ret = spi_imx_dma_configure(spi->master, + spi_imx_bytes_per_word(config.bpw)); + if (ret) + return ret; + } + spi_imx->devtype_data->config(spi_imx, &config); return 0; @@ -829,10 +912,8 @@ static void spi_imx_sdma_exit(struct spi_imx_data *spi_imx) } static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx, - struct spi_master *master, - const struct resource *res) + struct spi_master *master) { - struct dma_slave_config slave_config = {}; int ret; /* use pio mode for i.mx6dl chip TKT238285 */ @@ -850,16 +931,6 @@ static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx, goto err; } - slave_config.direction = DMA_MEM_TO_DEV; - slave_config.dst_addr = res->start + MXC_CSPITXDATA; - slave_config.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; - slave_config.dst_maxburst = spi_imx->wml; - ret = dmaengine_slave_config(master->dma_tx, &slave_config); - if (ret) { - dev_err(dev, "error in TX dma configuration.\n"); - goto err; - } - /* Prepare for RX : */ master->dma_rx = dma_request_slave_channel_reason(dev, "rx"); if (IS_ERR(master->dma_rx)) { @@ -869,15 +940,7 @@ static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx, goto err; } - slave_config.direction = DMA_DEV_TO_MEM; - slave_config.src_addr = res->start + MXC_CSPIRXDATA; - slave_config.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; - slave_config.src_maxburst = spi_imx->wml; - ret = dmaengine_slave_config(master->dma_rx, &slave_config); - if (ret) { - dev_err(dev, "error in RX dma configuration.\n"); - goto err; - } + spi_imx_dma_configure(master, 1); init_completion(&spi_imx->dma_rx_completion); init_completion(&spi_imx->dma_tx_completion); @@ -1164,6 +1227,7 @@ static int spi_imx_probe(struct platform_device *pdev) ret = PTR_ERR(spi_imx->base); goto out_master_put; } + spi_imx->base_phys = res->start; irq = platform_get_irq(pdev, 0); if (irq < 0) { @@ -1204,7 +1268,7 @@ static int spi_imx_probe(struct platform_device *pdev) * other chips. */ if (is_imx51_ecspi(spi_imx)) { - ret = spi_imx_sdma_init(&pdev->dev, spi_imx, master, res); + ret = spi_imx_sdma_init(&pdev->dev, spi_imx, master); if (ret == -EPROBE_DEFER) goto out_clk_put; From c34de7168cd201ab757b11bfc7899953948d7753 Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Wed, 24 Feb 2016 09:20:30 +0100 Subject: [PATCH 08/11] spi: imx: remove unnecessary bit clearing in mx51_ecspi_config This reverts patch 1476253cef (spi: imx: fix ecspi mode setup) The patch tried to fix something by clearing bits in the cfg variable, but cfg is initialized to zero on function entry. There are no bits to clear. Signed-off-by: Sascha Hauer Signed-off-by: Mark Brown --- drivers/spi/spi-imx.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 15b23f0b83e8..287ec0cb6992 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -366,20 +366,13 @@ static int __maybe_unused mx51_ecspi_config(struct spi_imx_data *spi_imx, if (config->mode & SPI_CPHA) cfg |= MX51_ECSPI_CONFIG_SCLKPHA(config->cs); - else - cfg &= ~MX51_ECSPI_CONFIG_SCLKPHA(config->cs); if (config->mode & SPI_CPOL) { cfg |= MX51_ECSPI_CONFIG_SCLKPOL(config->cs); cfg |= MX51_ECSPI_CONFIG_SCLKCTL(config->cs); - } else { - cfg &= ~MX51_ECSPI_CONFIG_SCLKPOL(config->cs); - cfg &= ~MX51_ECSPI_CONFIG_SCLKCTL(config->cs); } if (config->mode & SPI_CS_HIGH) cfg |= MX51_ECSPI_CONFIG_SSBPOL(config->cs); - else - cfg &= ~MX51_ECSPI_CONFIG_SSBPOL(config->cs); /* CTRL register always go first to bring out controller from reset */ writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); From d629c2a0051f27cc17ac9dc36bbeef8174b48aae Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Wed, 24 Feb 2016 09:20:31 +0100 Subject: [PATCH 09/11] spi: imx: make some register defines simpler The watermark levels in the DMA register are write only, the driver should never have to read them back from the hardware. Replace the current _MASK and _OFFSET defines with defines taking the watermark level directly. Signed-off-by: Sascha Hauer Signed-off-by: Mark Brown --- drivers/spi/spi-imx.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 287ec0cb6992..96e32d409820 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -256,12 +256,9 @@ static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi, #define MX51_ECSPI_INT_RREN (1 << 3) #define MX51_ECSPI_DMA 0x14 -#define MX51_ECSPI_DMA_TX_WML_OFFSET 0 -#define MX51_ECSPI_DMA_TX_WML_MASK 0x3F -#define MX51_ECSPI_DMA_RX_WML_OFFSET 16 -#define MX51_ECSPI_DMA_RX_WML_MASK (0x3F << 16) -#define MX51_ECSPI_DMA_RXT_WML_OFFSET 24 -#define MX51_ECSPI_DMA_RXT_WML_MASK (0x3F << 24) +#define MX51_ECSPI_DMA_TX_WML(wml) ((wml) & 0x3f) +#define MX51_ECSPI_DMA_RX_WML(wml) (((wml) & 0x3f) << 16) +#define MX51_ECSPI_DMA_RXT_WML(wml) (((wml) & 0x3f) << 24) #define MX51_ECSPI_DMA_TEDEN (1 << 7) #define MX51_ECSPI_DMA_RXDEN (1 << 23) @@ -408,9 +405,9 @@ static int __maybe_unused mx51_ecspi_config(struct spi_imx_data *spi_imx, * and enable DMA request. */ - writel(spi_imx->wml << MX51_ECSPI_DMA_RX_WML_OFFSET | - spi_imx->wml << MX51_ECSPI_DMA_TX_WML_OFFSET | - spi_imx->wml << MX51_ECSPI_DMA_RXT_WML_OFFSET | + writel(MX51_ECSPI_DMA_RX_WML(spi_imx->wml) | + MX51_ECSPI_DMA_TX_WML(spi_imx->wml) | + MX51_ECSPI_DMA_RXT_WML(spi_imx->wml) | MX51_ECSPI_DMA_TEDEN | MX51_ECSPI_DMA_RXDEN | MX51_ECSPI_DMA_RXTDEN, spi_imx->base + MX51_ECSPI_DMA); From b03c3884ca8c5ad3e6d14af6c8df0e3650c67482 Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Wed, 24 Feb 2016 09:20:32 +0100 Subject: [PATCH 10/11] spi: imx: set MX51_ECSPI_CTRL_SMC bit in setup function Now that the config function knows whether we are doing DMA or not we can do the necessary register setup in the config function and no longer have to do this in the trigger function. With this the trigger function becomes a no-op for DMA, so instead of testing if we are doing DMA or not in the trigger function we simply no longer call it in the DMA case. Signed-off-by: Sascha Hauer Signed-off-by: Mark Brown --- drivers/spi/spi-imx.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 96e32d409820..91890b2d0978 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -106,7 +106,6 @@ struct spi_imx_data { unsigned int txfifo; /* number of words pushed in tx FIFO */ /* DMA */ - unsigned int dma_finished; bool usedma; u32 wml; struct completion dma_rx_completion; @@ -324,14 +323,10 @@ static void __maybe_unused mx51_ecspi_intctrl(struct spi_imx_data *spi_imx, int static void __maybe_unused mx51_ecspi_trigger(struct spi_imx_data *spi_imx) { - u32 reg = readl(spi_imx->base + MX51_ECSPI_CTRL); + u32 reg; - if (!spi_imx->usedma) - reg |= MX51_ECSPI_CTRL_XCH; - else if (!spi_imx->dma_finished) - reg |= MX51_ECSPI_CTRL_SMC; - else - reg &= ~MX51_ECSPI_CTRL_SMC; + reg = readl(spi_imx->base + MX51_ECSPI_CTRL); + reg |= MX51_ECSPI_CTRL_XCH; writel(reg, spi_imx->base + MX51_ECSPI_CTRL); } @@ -371,6 +366,9 @@ static int __maybe_unused mx51_ecspi_config(struct spi_imx_data *spi_imx, if (config->mode & SPI_CS_HIGH) cfg |= MX51_ECSPI_CONFIG_SSBPOL(config->cs); + if (spi_imx->usedma) + ctrl |= MX51_ECSPI_CTRL_SMC; + /* CTRL register always go first to bring out controller from reset */ writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); @@ -1012,9 +1010,6 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, reinit_completion(&spi_imx->dma_rx_completion); reinit_completion(&spi_imx->dma_tx_completion); - /* Trigger the cspi module. */ - spi_imx->dma_finished = 0; - /* * Set these order to avoid potential RX overflow. The overflow may * happen if we enable SPI HW before starting RX DMA due to rescheduling @@ -1025,7 +1020,6 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, */ dma_async_issue_pending(master->dma_rx); dma_async_issue_pending(master->dma_tx); - spi_imx->devtype_data->trigger(spi_imx); transfer_timeout = spi_imx_calculate_timeout(spi_imx, transfer->len); @@ -1046,9 +1040,6 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, } } - spi_imx->dma_finished = 1; - spi_imx->devtype_data->trigger(spi_imx); - if (!timeout) ret = -ETIMEDOUT; else From 6b6192c04bf48ba5bec72287c2e5e9ae28217f6b Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Wed, 24 Feb 2016 09:20:33 +0100 Subject: [PATCH 11/11] spi: imx: drop bogus tests for rx/tx bufs in DMA transfer The driver tries to be clever by only setting up DMA channels when the corresponding sg tables are non NULL. The sg tables are embedded structs in struct spi_transfer, so they are guaranteed to be non NULL which makes the if(tx)/if(rx) tests completely bogus. The driver even sets the SPI_MASTER_MUST_RX / SPI_MASTER_MUST_TX flags which makes sure the sg tables are not only present but also non empty. Drop the tests and make the DMA path easier to follow. Signed-off-by: Sascha Hauer Signed-off-by: Mark Brown --- drivers/spi/spi-imx.c | 86 ++++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 50 deletions(-) diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 91890b2d0978..e7a19be87c38 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -974,51 +974,40 @@ static int spi_imx_calculate_timeout(struct spi_imx_data *spi_imx, int size) static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, struct spi_transfer *transfer) { - struct dma_async_tx_descriptor *desc_tx = NULL, *desc_rx = NULL; - int ret; + struct dma_async_tx_descriptor *desc_tx, *desc_rx; unsigned long transfer_timeout; unsigned long timeout; struct spi_master *master = spi_imx->bitbang.master; struct sg_table *tx = &transfer->tx_sg, *rx = &transfer->rx_sg; - if (tx) { - desc_tx = dmaengine_prep_slave_sg(master->dma_tx, - tx->sgl, tx->nents, DMA_MEM_TO_DEV, - DMA_PREP_INTERRUPT | DMA_CTRL_ACK); - if (!desc_tx) - return -EINVAL; - - desc_tx->callback = spi_imx_dma_tx_callback; - desc_tx->callback_param = (void *)spi_imx; - dmaengine_submit(desc_tx); - } - - if (rx) { - desc_rx = dmaengine_prep_slave_sg(master->dma_rx, - rx->sgl, rx->nents, DMA_DEV_TO_MEM, - DMA_PREP_INTERRUPT | DMA_CTRL_ACK); - if (!desc_rx) { - dmaengine_terminate_all(master->dma_tx); - return -EINVAL; - } - - desc_rx->callback = spi_imx_dma_rx_callback; - desc_rx->callback_param = (void *)spi_imx; - dmaengine_submit(desc_rx); - } - - reinit_completion(&spi_imx->dma_rx_completion); - reinit_completion(&spi_imx->dma_tx_completion); - /* - * Set these order to avoid potential RX overflow. The overflow may - * happen if we enable SPI HW before starting RX DMA due to rescheduling - * for another task and/or interrupt. - * So RX DMA enabled first to make sure data would be read out from FIFO - * ASAP. TX DMA enabled next to start filling TX FIFO with new data. - * And finaly SPI HW enabled to start actual data transfer. + * The TX DMA setup starts the transfer, so make sure RX is configured + * before TX. */ + desc_rx = dmaengine_prep_slave_sg(master->dma_rx, + rx->sgl, rx->nents, DMA_DEV_TO_MEM, + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); + if (!desc_rx) + return -EINVAL; + + desc_rx->callback = spi_imx_dma_rx_callback; + desc_rx->callback_param = (void *)spi_imx; + dmaengine_submit(desc_rx); + reinit_completion(&spi_imx->dma_rx_completion); dma_async_issue_pending(master->dma_rx); + + desc_tx = dmaengine_prep_slave_sg(master->dma_tx, + tx->sgl, tx->nents, DMA_MEM_TO_DEV, + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); + if (!desc_tx) { + dmaengine_terminate_all(master->dma_tx); + return -EINVAL; + } + + desc_tx->callback = spi_imx_dma_tx_callback; + desc_tx->callback_param = (void *)spi_imx; + dmaengine_submit(desc_tx); + reinit_completion(&spi_imx->dma_tx_completion); dma_async_issue_pending(master->dma_tx); transfer_timeout = spi_imx_calculate_timeout(spi_imx, transfer->len); @@ -1030,22 +1019,19 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, dev_err(spi_imx->dev, "I/O Error in DMA TX\n"); dmaengine_terminate_all(master->dma_tx); dmaengine_terminate_all(master->dma_rx); - } else { - timeout = wait_for_completion_timeout( - &spi_imx->dma_rx_completion, transfer_timeout); - if (!timeout) { - dev_err(spi_imx->dev, "I/O Error in DMA RX\n"); - spi_imx->devtype_data->reset(spi_imx); - dmaengine_terminate_all(master->dma_rx); - } + return -ETIMEDOUT; } - if (!timeout) - ret = -ETIMEDOUT; - else - ret = transfer->len; + timeout = wait_for_completion_timeout(&spi_imx->dma_rx_completion, + transfer_timeout); + if (!timeout) { + dev_err(&master->dev, "I/O Error in DMA RX\n"); + spi_imx->devtype_data->reset(spi_imx); + dmaengine_terminate_all(master->dma_rx); + return -ETIMEDOUT; + } - return ret; + return transfer->len; } static int spi_imx_pio_transfer(struct spi_device *spi,