From c06abca69218ac42fa58d1ba7a7b0d9bab5f1b18 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Tue, 6 Nov 2018 03:40:25 +0000 Subject: Revert "dmaengine: imx-sdma: Use GFP_NOWAIT for dma allocations" This reverts commit c1199875d327, as this depends on another commit that is going to be reverted. Signed-off-by: Lucas Stach Signed-off-by: Robin Gong Cc: stable Signed-off-by: Vinod Koul --- drivers/dma/imx-sdma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index b4ec2d20e661..3bca5e0c715f 100644 --- a/drivers/dma/imx-sdma.c +++ b/drivers/dma/imx-sdma.c @@ -1194,8 +1194,8 @@ static int sdma_alloc_bd(struct sdma_desc *desc) { int ret = 0; - desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_NOWAIT, - &desc->bd_phys); + desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_ATOMIC, + &desc->bd_phys); if (!desc->bd) { ret = -ENOMEM; goto out; -- cgit v1.2.3-59-g8ed1b From ebb853b1bd5f659b92c71dc6a9de44cfc37c78c0 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Tue, 6 Nov 2018 03:40:28 +0000 Subject: Revert "dmaengine: imx-sdma: alloclate bd memory from dma pool" This reverts commit fe5b85c656bc. The SDMA engine needs the descriptors to be contiguous in memory. As the dma pool API is only able to provide a single descriptor per alloc invocation there is no guarantee that multiple descriptors satisfy this requirement. Also the code in question is broken as it only allocates memory for a single descriptor, without looking at the number of descriptors required for the transfer, leading to out-of-bounds accesses when the descriptors are written. Signed-off-by: Lucas Stach Signed-off-by: Robin Gong Cc: stable Signed-off-by: Vinod Koul --- drivers/dma/imx-sdma.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index 3bca5e0c715f..8d2fec8b16cc 100644 --- a/drivers/dma/imx-sdma.c +++ b/drivers/dma/imx-sdma.c @@ -24,7 +24,6 @@ #include #include #include -#include #include #include #include @@ -376,7 +375,6 @@ struct sdma_channel { u32 shp_addr, per_addr; enum dma_status status; struct imx_dma_data data; - struct dma_pool *bd_pool; }; #define IMX_DMA_SG_LOOP BIT(0) @@ -1192,10 +1190,11 @@ out: static int sdma_alloc_bd(struct sdma_desc *desc) { + u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor); int ret = 0; - desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_ATOMIC, - &desc->bd_phys); + desc->bd = dma_zalloc_coherent(NULL, bd_size, &desc->bd_phys, + GFP_ATOMIC); if (!desc->bd) { ret = -ENOMEM; goto out; @@ -1206,7 +1205,9 @@ out: static void sdma_free_bd(struct sdma_desc *desc) { - dma_pool_free(desc->sdmac->bd_pool, desc->bd, desc->bd_phys); + u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor); + + dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys); } static void sdma_desc_free(struct virt_dma_desc *vd) @@ -1272,10 +1273,6 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan) if (ret) goto disable_clk_ahb; - sdmac->bd_pool = dma_pool_create("bd_pool", chan->device->dev, - sizeof(struct sdma_buffer_descriptor), - 32, 0); - return 0; disable_clk_ahb: @@ -1304,9 +1301,6 @@ static void sdma_free_chan_resources(struct dma_chan *chan) clk_disable(sdma->clk_ipg); clk_disable(sdma->clk_ahb); - - dma_pool_destroy(sdmac->bd_pool); - sdmac->bd_pool = NULL; } static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac, -- cgit v1.2.3-59-g8ed1b From b8603d2a5795c42f78998e70dc792336e0dc20c9 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Tue, 6 Nov 2018 03:40:33 +0000 Subject: dmaengine: imx-sdma: implement channel termination via worker The dmaengine documentation states that device_terminate_all may be asynchronous and need not wait for the active transfers to stop. This allows us to move most of the functionality currently implemented in the sdma channel termination function to run in a worker, outside of any atomic context. Moving this out of atomic context has two benefits: we can now sleep while waiting for the channel to terminate, instead of busy waiting and the freeing of the dma descriptors happens with IRQs enabled, getting rid of a warning in the dma mapping code. As the termination is now async, we need to implement the device_synchronize dma engine function which simply waits for the worker to finish its execution. Signed-off-by: Lucas Stach Signed-off-by: Robin Gong Cc: stable Signed-off-by: Vinod Koul --- drivers/dma/imx-sdma.c | 51 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index 8d2fec8b16cc..03d46f1b0826 100644 --- a/drivers/dma/imx-sdma.c +++ b/drivers/dma/imx-sdma.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include @@ -375,6 +376,7 @@ struct sdma_channel { u32 shp_addr, per_addr; enum dma_status status; struct imx_dma_data data; + struct work_struct terminate_worker; }; #define IMX_DMA_SG_LOOP BIT(0) @@ -1025,31 +1027,49 @@ static int sdma_disable_channel(struct dma_chan *chan) return 0; } - -static int sdma_disable_channel_with_delay(struct dma_chan *chan) +static void sdma_channel_terminate_work(struct work_struct *work) { - struct sdma_channel *sdmac = to_sdma_chan(chan); + struct sdma_channel *sdmac = container_of(work, struct sdma_channel, + terminate_worker); unsigned long flags; LIST_HEAD(head); - sdma_disable_channel(chan); - spin_lock_irqsave(&sdmac->vc.lock, flags); - vchan_get_all_descriptors(&sdmac->vc, &head); - sdmac->desc = NULL; - spin_unlock_irqrestore(&sdmac->vc.lock, flags); - vchan_dma_desc_free_list(&sdmac->vc, &head); - /* * According to NXP R&D team a delay of one BD SDMA cost time * (maximum is 1ms) should be added after disable of the channel * bit, to ensure SDMA core has really been stopped after SDMA * clients call .device_terminate_all. */ - mdelay(1); + usleep_range(1000, 2000); + + spin_lock_irqsave(&sdmac->vc.lock, flags); + vchan_get_all_descriptors(&sdmac->vc, &head); + sdmac->desc = NULL; + spin_unlock_irqrestore(&sdmac->vc.lock, flags); + vchan_dma_desc_free_list(&sdmac->vc, &head); +} + +static int sdma_disable_channel_async(struct dma_chan *chan) +{ + struct sdma_channel *sdmac = to_sdma_chan(chan); + + sdma_disable_channel(chan); + + if (sdmac->desc) + schedule_work(&sdmac->terminate_worker); return 0; } +static void sdma_channel_synchronize(struct dma_chan *chan) +{ + struct sdma_channel *sdmac = to_sdma_chan(chan); + + vchan_synchronize(&sdmac->vc); + + flush_work(&sdmac->terminate_worker); +} + static void sdma_set_watermarklevel_for_p2p(struct sdma_channel *sdmac) { struct sdma_engine *sdma = sdmac->sdma; @@ -1287,7 +1307,9 @@ static void sdma_free_chan_resources(struct dma_chan *chan) struct sdma_channel *sdmac = to_sdma_chan(chan); struct sdma_engine *sdma = sdmac->sdma; - sdma_disable_channel_with_delay(chan); + sdma_disable_channel_async(chan); + + sdma_channel_synchronize(chan); if (sdmac->event_id0) sdma_event_disable(sdmac, sdmac->event_id0); @@ -1993,6 +2015,8 @@ static int sdma_probe(struct platform_device *pdev) sdmac->channel = i; sdmac->vc.desc_free = sdma_desc_free; + INIT_WORK(&sdmac->terminate_worker, + sdma_channel_terminate_work); /* * Add the channel to the DMAC list. Do not add channel 0 though * because we need it internally in the SDMA driver. This also means @@ -2044,7 +2068,8 @@ static int sdma_probe(struct platform_device *pdev) sdma->dma_device.device_prep_slave_sg = sdma_prep_slave_sg; sdma->dma_device.device_prep_dma_cyclic = sdma_prep_dma_cyclic; sdma->dma_device.device_config = sdma_config; - sdma->dma_device.device_terminate_all = sdma_disable_channel_with_delay; + sdma->dma_device.device_terminate_all = sdma_disable_channel_async; + sdma->dma_device.device_synchronize = sdma_channel_synchronize; sdma->dma_device.src_addr_widths = SDMA_DMA_BUSWIDTHS; sdma->dma_device.dst_addr_widths = SDMA_DMA_BUSWIDTHS; sdma->dma_device.directions = SDMA_DMA_DIRECTIONS; -- cgit v1.2.3-59-g8ed1b From 64068853bc77786d1a28abb4087d6a3e93aedbe2 Mon Sep 17 00:00:00 2001 From: Lucas Stach Date: Tue, 6 Nov 2018 03:40:37 +0000 Subject: dmaengine: imx-sdma: use GFP_NOWAIT for dma descriptor allocations DMA buffer descriptors aren't allocated from atomic context, so they can use the less heavyweigth GFP_NOWAIT. Signed-off-by: Lucas Stach Signed-off-by: Robin Gong Cc: stable Signed-off-by: Vinod Koul --- drivers/dma/imx-sdma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index 03d46f1b0826..cb1b44d78a1f 100644 --- a/drivers/dma/imx-sdma.c +++ b/drivers/dma/imx-sdma.c @@ -1214,7 +1214,7 @@ static int sdma_alloc_bd(struct sdma_desc *desc) int ret = 0; desc->bd = dma_zalloc_coherent(NULL, bd_size, &desc->bd_phys, - GFP_ATOMIC); + GFP_NOWAIT); if (!desc->bd) { ret = -ENOMEM; goto out; -- cgit v1.2.3-59-g8ed1b From 59861547ec9a9736e7882f6fb0c096a720ff811a Mon Sep 17 00:00:00 2001 From: Bin Liu Date: Mon, 12 Nov 2018 09:43:22 -0600 Subject: dmaengine: cppi41: delete channel from pending list when stop channel The driver defines three states for a cppi channel. - idle: .chan_busy == 0 && not in .pending list - pending: .chan_busy == 0 && in .pending list - busy: .chan_busy == 1 && not in .pending list There are cases in which the cppi channel could be in the pending state when cppi41_dma_issue_pending() is called after cppi41_runtime_suspend() is called. cppi41_stop_chan() has a bug for these cases to set channels to idle state. It only checks the .chan_busy flag, but not the .pending list, then later when cppi41_runtime_resume() is called the channels in .pending list will be transitioned to busy state. Removing channels from the .pending list solves the problem. Fixes: 975faaeb9985 ("dma: cppi41: start tear down only if channel is busy") Cc: stable@vger.kernel.org # v3.15+ Signed-off-by: Bin Liu Reviewed-by: Peter Ujfalusi Signed-off-by: Vinod Koul --- drivers/dma/ti/cppi41.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/dma/ti/cppi41.c b/drivers/dma/ti/cppi41.c index 1497da367710..e507ec36c0d3 100644 --- a/drivers/dma/ti/cppi41.c +++ b/drivers/dma/ti/cppi41.c @@ -723,8 +723,22 @@ static int cppi41_stop_chan(struct dma_chan *chan) desc_phys = lower_32_bits(c->desc_phys); desc_num = (desc_phys - cdd->descs_phys) / sizeof(struct cppi41_desc); - if (!cdd->chan_busy[desc_num]) + if (!cdd->chan_busy[desc_num]) { + struct cppi41_channel *cc, *_ct; + + /* + * channels might still be in the pendling list if + * cppi41_dma_issue_pending() is called after + * cppi41_runtime_suspend() is called + */ + list_for_each_entry_safe(cc, _ct, &cdd->pending, node) { + if (cc != c) + continue; + list_del(&cc->node); + break; + } return 0; + } ret = cppi41_tear_down_chan(c); if (ret) -- cgit v1.2.3-59-g8ed1b From ffe843b18211301ad25893eba09f402c19d12304 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 5 Dec 2018 18:33:59 +0200 Subject: dmaengine: dw: Fix FIFO size for Intel Merrifield Intel Merrifield has a reduced size of FIFO used in iDMA 32-bit controller, i.e. 512 bytes instead of 1024. Fix this by partitioning it as 64 bytes per channel. Note, in the future we might switch to 'fifo-size' property instead of hard coded value. Fixes: 199244d69458 ("dmaengine: dw: add support of iDMA 32-bit hardware") Signed-off-by: Andy Shevchenko Cc: stable@vger.kernel.org Signed-off-by: Vinod Koul --- drivers/dma/dw/core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c index d0c3e50b39fb..1fc488e90f36 100644 --- a/drivers/dma/dw/core.c +++ b/drivers/dma/dw/core.c @@ -1059,12 +1059,12 @@ static void dwc_issue_pending(struct dma_chan *chan) /* * Program FIFO size of channels. * - * By default full FIFO (1024 bytes) is assigned to channel 0. Here we + * By default full FIFO (512 bytes) is assigned to channel 0. Here we * slice FIFO on equal parts between channels. */ static void idma32_fifo_partition(struct dw_dma *dw) { - u64 value = IDMA32C_FP_PSIZE_CH0(128) | IDMA32C_FP_PSIZE_CH1(128) | + u64 value = IDMA32C_FP_PSIZE_CH0(64) | IDMA32C_FP_PSIZE_CH1(64) | IDMA32C_FP_UPDATE; u64 fifo_partition = 0; @@ -1077,7 +1077,7 @@ static void idma32_fifo_partition(struct dw_dma *dw) /* Fill FIFO_PARTITION high bits (Channels 2..3, 6..7) */ fifo_partition |= value << 32; - /* Program FIFO Partition registers - 128 bytes for each channel */ + /* Program FIFO Partition registers - 64 bytes per channel */ idma32_writeq(dw, FIFO_PARTITION1, fifo_partition); idma32_writeq(dw, FIFO_PARTITION0, fifo_partition); } -- cgit v1.2.3-59-g8ed1b