i.MX SPI DMA cleanup
Subsystems (2)
Sascha Hauer
maintainerPATCH v2 1/1 i.MX SPI DMA cleanup
Second round for the i.MX SPI DMA patches. No changes to code, only updated commit messages.
Mark, you already applied patch 1/12 and 2/12. I'm resending them here anyway because I updated the commit message as you requested. Feel free to ignore 1/12 and 2/12 or replace them with this updated version.
Changes since v1:
- Add missing Signed-off-by to Patch 3
- Drop device tree change as it's already applied by Shawn
- Add patch subject when referring to commits
----------------------------------------------------------------
Anton Bondarenko (3):
spi: imx: allow only WML aligned transfers to use DMA
spi: imx: replace fixed timeout with calculated
spi: imx: add support for all SPI word width for DMA
Sascha Hauer (9):
spi: imx: use proper dev_* functions for driver messages
spi: imx: drop fallback to PIO
spi: imx: initialize usedma earlier
spi: imx: drop unnecessary read/modify/write
spi: imx: drop unncessary dma_is_inited variable
spi: imx: remove unnecessary bit clearing in mx51_ecspi_config
spi: imx: make some register defines simpler
spi: imx: set MX51_ECSPI_CTRL_SMC bit in setup function
spi: imx: drop bogus tests for rx/tx bufs in DMA transfer
drivers/spi/spi-imx.c | 350 ++++++++++++++++++++++++++------------------------ 1 file changed, 181 insertions(+), 169 deletions(-)
Sascha Hauer
maintainerPATCH 1/12 spi: imx: allow only WML aligned transfers to use DMA
From: Anton Bondarenko <[email protected]>
RX DMA tail data handling doesn't work correctly in many cases with current implementation. It happens because SPI core was setup to generates both RX and RX TAIL events. And RX TAIL event does not work correctly. This can be easily verified by sending SPI transaction with size modulus WML(32 in our case) not equal 0.
Also this removes the change introduced in f6ee9b582 (spi: imx: Fix small DMA transfers) since this only fixes usecases with transfer size from 33 to 128 bytes and doesn't fix 129 bytes and bigger.
This is output from transaction with len 138 bytes in loopback mode at 10Mhz:
TX0000: a3 97 a2 55 53 be f1 fc f9 79 6b 52 14 13 e9 e2
TX0010: 2d 51 8e 1f 56 08 57 27 a7 05 d4 d0 52 82 77 75
TX0020: 1b 99 4a ed 58 3d 6a 52 36 d5 24 4a 68 8e ad 95
TX0030: 5f 3c 35 b5 c4 8c dd 6c 11 32 3d e2 b4 b4 59 cf
TX0040: ce 23 3d 27 df a7 f9 96 fc 1e e0 66 2c 0e 7b 8c
TX0050: ca 30 42 8f bc 9f 7b ce d1 b8 b1 87 ec 8a d6 bb
TX0060: 2e 15 63 0e 3c dc a4 3a 7a 06 20 a7 93 1b 34 dd
TX0070: 4c f5 ec 88 96 68 d6 68 a0 09 6f 8e 93 47 c9 41
TX0080: db ac cf 97 89 f3 51 05 79 71
RX0000: a3 97 a2 55 53 be f1 fc f9 79 6b 52 14 13 e9 e2
RX0010: 2d 51 8e 1f 56 08 57 27 a7 05 d4 d0 52 82 77 75
RX0020: 1b 99 4a ed 58 3d 6a 52 36 d5 24 4a 68 8e ad 95
RX0030: 5f 3c 35 00 00 b5 00 00 00 c4 00 00 8c 00 00 dd
RX0040: 6c 11 32 3d e2 b4 b4 59 cf ce 23 3d 27 df a7 f9
RX0050: 96 fc 1e e0 66 2c 0e 7b 8c ca 30 42 8f 1f 1f bc
RX0060: 9f 7b ce d1 b8 b1 87 ec 8a d6 bb 2e 15 63 0e ed
RX0070: ed 3c 58 58 58 dc 3d 3d a4 6a 6a 3a 52 52 7a 36
RX0080: 06 20 a7 93 1b 34 dd 4c f5 ec
Zeros at offset 33 and 34 caused by reading empty RX FIFO which not possible if DMA RX read was triggered by RX event. This mean DMA was triggered by RX TAIL event.
--- drivers/spi/spi-imx.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-)
drivers/spi/spi-imx.c +2 -15
@@ -204,8 +204,8 @@ 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 *sizeof (u32)) +if (spi_imx->dma_is_inited && transfer->len >= spi_imx->wml && + (transfer->len % spi_imx->wml) ==0 )return true;return false; }
@@ -919,8 +919,6 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx,struct dma_async_tx_descriptor *desc_tx =NULL , *desc_rx =NULL ;int ret;unsigned long timeout; -u32 dma; -int left;struct spi_master *master = spi_imx->bitbang.master;struct sg_table *tx = &transfer->tx_sg, *rx = &transfer->rx_sg;
@@ -954,13 +952,6 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx,/* Trigger the cspi module. */ spi_imx->dma_finished =0 ; - dma =readl (spi_imx->base +MX51_ECSPI_DMA ); - dma = dma & (~MX51_ECSPI_DMA_RXT_WML_MASK ); -/* Change RX_DMA_LENGTH trigger dma fetch tail data */ - left = transfer->len % spi_imx->wml; -if (left) -writel (dma | (left <<MX51_ECSPI_DMA_RXT_WML_OFFSET ), - spi_imx->base +MX51_ECSPI_DMA );/* * Set these order to avoid potential RX overflow. The overflow may * happen if we enable SPI HW before starting RX DMA due to rescheduling
@@ -992,10 +983,6 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, spi_imx->devtype_data->reset (spi_imx);dmaengine_terminate_all (master->dma_rx); } - dma &= ~MX51_ECSPI_DMA_RXT_WML_MASK ; -writel (dma | - spi_imx->wml <<MX51_ECSPI_DMA_RXT_WML_OFFSET , - spi_imx->base +MX51_ECSPI_DMA ); } spi_imx->dma_finished =1 ;
Signature · 1 line
2.7.0
Sascha Hauer
maintainerPATCH 2/12 spi: imx: use proper dev_* functions for driver messages
In reply to Sascha HauerAdd a struct device * member to the private driver data and use it to print messages using dev_* functions rather than pr_*.
--- drivers/spi/spi-imx.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-)
drivers/spi/spi-imx.c +12 -15
@@ -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);
@@ -968,18 +970,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); }
@@ -996,9 +994,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 ; }
@@ -1128,6 +1124,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;
Signature · 1 line
2.7.0
Sascha Hauer
maintainerPATCH 3/12 spi: imx: replace fixed timeout with calculated
In reply to Sascha HauerFrom: Anton Bondarenko <[email protected]>
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.
--- drivers/spi/spi-imx.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)
drivers/spi/spi-imx.c +21 -3
@@ -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);
Signature · 1 line
2.7.0
Sascha Hauer
maintainerPATCH 4/12 spi: imx: drop fallback to PIO
In reply to Sascha HauerAt 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.
--- drivers/spi/spi-imx.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
drivers/spi/spi-imx.c +3 -10
@@ -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 no_dma; +return -EINVAL ; desc_tx->callback = spi_imx_dma_tx_callback; desc_tx->callback_param = (void *)spi_imx;
@@ -957,7 +957,7 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, rx->sgl, rx->nents,DMA_DEV_TO_MEM , DMA_PREP_INTERRUPT | DMA_CTRL_ACK);if (!desc_rx) - goto no_dma; +return -EINVAL ; desc_rx->callback = spi_imx_dma_rx_callback; desc_rx->callback_param = (void *)spi_imx;
@@ -1010,10 +1010,6 @@ static int spi_imx_dma_transfer(struct spi_imx_data *spi_imx, ret = transfer->len;return ret; - -no_dma: -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,
@@ -1040,15 +1036,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;
Signature · 1 line
2.7.0
Mark Brown
maintainerPATCH 4/12 spi: imx: drop fallback to PIO
Quoted 3 lines · Sascha Hauer
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.
This doesn't apply against current code, please check and resend.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160220/a63b0592/attachment.sig>
Sascha Hauer
maintainerPATCH 5/12 spi: imx: initialize usedma earlier
In reply to Sascha HauerSo that the SoC specific config function can know if we will do DMA or not. Will be needed in following patches.
--- drivers/spi/spi-imx.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
drivers/spi/spi-imx.c +8 -7
@@ -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 ;
@@ -1038,14 +1043,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)
Signature · 1 line
2.7.0
PATCH 5/12 spi: imx: initialize usedma earlier
Hi,
Quoted 2 lines · On Fri, 19 Feb 2016 08:43:05 +0100 Sascha Hauer wrote:
So that the SoC specific config function can know if we will do
It's not quite obvious that this partial sentence is a continuation of the subject header, even more since it starts with a capital letter...
Lothar Wa?mann
Signature · 9 lines
___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________
Sascha Hauer
maintainerPATCH 6/12 spi: imx: drop unnecessary read/modify/write
In reply to Sascha HauerWhen 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.
--- drivers/spi/spi-imx.c | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-)
drivers/spi/spi-imx.c +10 -21
@@ -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 ; }
Signature · 1 line
2.7.0
Sascha Hauer
maintainerPATCH 7/12 spi: imx: drop unncessary dma_is_inited variable
In reply to Sascha HauerThere's no need for an extra dma_is_inited variable when we can equally well check for the existence of a DMA channel.
--- drivers/spi/spi-imx.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/spi/spi-imx.c +1 -5
@@ -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:
Signature · 1 line
2.7.0
Sascha Hauer
maintainerPATCH 8/12 spi: imx: add support for all SPI word width for DMA
In reply to Sascha HauerFrom: Anton Bondarenko <[email protected]>
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.
--- drivers/spi/spi-imx.c | 118 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 91 insertions(+), 27 deletions(-)
drivers/spi/spi-imx.c +91 -27
@@ -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) +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; -if (master->dma_rx && transfer->len >= spi_imx->wml && - (transfer->len % spi_imx->wml) ==0 ) -return true; -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);
@@ -1162,6 +1225,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 ) {
@@ -1202,7 +1266,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;
Signature · 1 line
2.7.0
Sascha Hauer
maintainerPATCH 9/12 spi: imx: remove unnecessary bit clearing in mx51_ecspi_config
In reply to Sascha HauerThis 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.
--- drivers/spi/spi-imx.c | 7 ------- 1 file changed, 7 deletions(-)
drivers/spi/spi-imx.c +0 -7
@@ -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 );
Signature · 1 line
2.7.0
Sascha Hauer
maintainerPATCH 10/12 spi: imx: make some register defines simpler
In reply to Sascha HauerThe 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.
--- drivers/spi/spi-imx.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
drivers/spi/spi-imx.c +6 -9
@@ -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 );
Signature · 1 line
2.7.0
Sascha Hauer
maintainerPATCH 11/12 spi: imx: set MX51_ECSPI_CTRL_SMC bit in setup function
In reply to Sascha HauerNow 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.
--- drivers/spi/spi-imx.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-)
drivers/spi/spi-imx.c +6 -15
@@ -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, intstatic 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 );
@@ -1010,9 +1008,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
@@ -1023,7 +1018,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);
@@ -1044,9 +1038,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
Signature · 1 line
2.7.0
Sascha Hauer
maintainerPATCH 12/12 spi: imx: drop bogus tests for rx/tx bufs in DMA transfer
In reply to Sascha HauerThe 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.
--- drivers/spi/spi-imx.c | 78 +++++++++++++++++++++------------------------------ 1 file changed, 32 insertions(+), 46 deletions(-)
drivers/spi/spi-imx.c +32 -46
@@ -975,48 +975,37 @@ 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; -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); - } +/* +* 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 ; -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) -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_rx->callback = spi_imx_dma_rx_callback; - desc_rx->callback_param = (void *)spi_imx; -dmaengine_submit (desc_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) +return -EINVAL ; -reinit_completion (&spi_imx->dma_rx_completion); + 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); - -/* -* 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. -*/ -dma_async_issue_pending (master->dma_rx);dma_async_issue_pending (master->dma_tx); transfer_timeout =spi_imx_calculate_timeout (spi_imx, transfer->len);
@@ -1028,22 +1017,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,
Signature · 1 line
2.7.0