From 5a637f83d96fc9f27e93385924e7253a5f5187bc Mon Sep 17 00:00:00 2001 From: Amelie Delaunay Date: Mon, 17 Oct 2022 15:14:13 +0200 Subject: dmaengine: stm32-mdma: memset stm32_mdma_chan_config struct before using it New bool m2m_hw has been added at the end of stm32_mdma_chan_config struct to support the STM32 DMA MDMA chaining. m2m_hw is set true in stm32_mdma_slave_config() if peripheral_size is set, but m2m_hw is never initialized false. To ensure this case, and any further new update of the structure, memset it to 0 before using it. Fixes: 696874322771 ("dmaengine: stm32-mdma: add support to be triggered by STM32 DMA") Signed-off-by: Amelie Delaunay Link: https://lore.kernel.org/r/20221017131413.202567-1-amelie.delaunay@foss.st.com Signed-off-by: Vinod Koul --- drivers/dma/stm32-mdma.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c index e28acbcb53f4..b9d4c843635f 100644 --- a/drivers/dma/stm32-mdma.c +++ b/drivers/dma/stm32-mdma.c @@ -1539,6 +1539,7 @@ static struct dma_chan *stm32_mdma_of_xlate(struct of_phandle_args *dma_spec, return NULL; } + memset(&config, 0, sizeof(config)); config.request = dma_spec->args[0]; config.priority_level = dma_spec->args[1]; config.transfer_config = dma_spec->args[2]; -- cgit v1.2.3 From 0ec8ce07394442d722806fe61b901a5b2b17249d Mon Sep 17 00:00:00 2001 From: Fenghua Yu Date: Fri, 14 Oct 2022 15:25:41 -0700 Subject: dmaengine: idxd: Do not enable user type Work Queue without Shared Virtual Addressing When the idxd_user_drv driver is bound to a Work Queue (WQ) device without IOMMU or with IOMMU Passthrough without Shared Virtual Addressing (SVA), the application gains direct access to physical memory via the device by programming physical address to a submitted descriptor. This allows direct userspace read and write access to arbitrary physical memory. This is inconsistent with the security goals of a good kernel API. Unlike vfio_pci driver, the IDXD char device driver does not provide any ways to pin user pages and translate the address from user VA to IOVA or PA without IOMMU SVA. Therefore the application has no way to instruct the device to perform DMA function. This makes the char device not usable for normal application usage. Since user type WQ without SVA cannot be used for normal application usage and presents the security issue, bind idxd_user_drv driver and enable user type WQ only when SVA is enabled (i.e. user PASID is enabled). Fixes: 448c3de8ac83 ("dmaengine: idxd: create user driver for wq 'device'") Cc: stable@vger.kernel.org Suggested-by: Arjan Van De Ven Signed-off-by: Fenghua Yu Reviewed-by: Dave Jiang Reviewed-by: Jerry Snitselaar Link: https://lore.kernel.org/r/20221014222541.3912195-1-fenghua.yu@intel.com Signed-off-by: Vinod Koul --- drivers/dma/idxd/cdev.c | 18 ++++++++++++++++++ include/uapi/linux/idxd.h | 1 + 2 files changed, 19 insertions(+) (limited to 'drivers') diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c index c2808fd081d6..a9b96b18772f 100644 --- a/drivers/dma/idxd/cdev.c +++ b/drivers/dma/idxd/cdev.c @@ -312,6 +312,24 @@ static int idxd_user_drv_probe(struct idxd_dev *idxd_dev) if (idxd->state != IDXD_DEV_ENABLED) return -ENXIO; + /* + * User type WQ is enabled only when SVA is enabled for two reasons: + * - If no IOMMU or IOMMU Passthrough without SVA, userspace + * can directly access physical address through the WQ. + * - The IDXD cdev driver does not provide any ways to pin + * user pages and translate the address from user VA to IOVA or + * PA without IOMMU SVA. Therefore the application has no way + * to instruct the device to perform DMA function. This makes + * the cdev not usable for normal application usage. + */ + if (!device_user_pasid_enabled(idxd)) { + idxd->cmd_status = IDXD_SCMD_WQ_USER_NO_IOMMU; + dev_dbg(&idxd->pdev->dev, + "User type WQ cannot be enabled without SVA.\n"); + + return -EOPNOTSUPP; + } + mutex_lock(&wq->wq_lock); wq->type = IDXD_WQT_USER; rc = drv_enable_wq(wq); diff --git a/include/uapi/linux/idxd.h b/include/uapi/linux/idxd.h index 095299c75828..2b9e7feba3f3 100644 --- a/include/uapi/linux/idxd.h +++ b/include/uapi/linux/idxd.h @@ -29,6 +29,7 @@ enum idxd_scmd_stat { IDXD_SCMD_WQ_NO_SIZE = 0x800e0000, IDXD_SCMD_WQ_NO_PRIV = 0x800f0000, IDXD_SCMD_WQ_IRQ_ERR = 0x80100000, + IDXD_SCMD_WQ_USER_NO_IOMMU = 0x80110000, }; #define IDXD_SCMD_SOFTERR_MASK 0x80000000 -- cgit v1.2.3 From b3d726cb8497c6b12106fd617d46eef11763ea86 Mon Sep 17 00:00:00 2001 From: Doug Brown Date: Mon, 5 Sep 2022 17:07:09 -0700 Subject: dmaengine: pxa_dma: use platform_get_irq_optional The first IRQ is required, but IRQs 1 through (nb_phy_chans - 1) are optional, because on some platforms (e.g. PXA168) there is a single IRQ shared between all channels. This change inhibits a flood of "IRQ index # not found" messages at startup. Tested on a PXA168-based device. Fixes: 7723f4c5ecdb ("driver core: platform: Add an error message to platform_get_irq*()") Signed-off-by: Doug Brown Link: https://lore.kernel.org/r/20220906000709.52705-1-doug@schmorgal.com Signed-off-by: Vinod Koul --- drivers/dma/pxa_dma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c index e7034f6f3994..22a392fe6d32 100644 --- a/drivers/dma/pxa_dma.c +++ b/drivers/dma/pxa_dma.c @@ -1247,14 +1247,14 @@ static int pxad_init_phys(struct platform_device *op, return -ENOMEM; for (i = 0; i < nb_phy_chans; i++) - if (platform_get_irq(op, i) > 0) + if (platform_get_irq_optional(op, i) > 0) nr_irq++; for (i = 0; i < nb_phy_chans; i++) { phy = &pdev->phys[i]; phy->base = pdev->base; phy->idx = i; - irq = platform_get_irq(op, i); + irq = platform_get_irq_optional(op, i); if ((nr_irq > 1) && (irq > 0)) ret = devm_request_irq(&op->dev, irq, pxad_chan_handler, -- cgit v1.2.3 From e8dbd6445dd6b38c4c50410a86f13158486ee99a Mon Sep 17 00:00:00 2001 From: Xiaochen Shen Date: Sat, 1 Oct 2022 04:15:27 +0800 Subject: dmaengine: idxd: Fix max batch size for Intel IAA >From Intel IAA spec [1], Intel IAA does not support batch processing. Two batch related default values for IAA are incorrect in current code: (1) The max batch size of device is set during device initialization, that indicates batch is supported. It should be always 0 on IAA. (2) The max batch size of work queue is set to WQ_DEFAULT_MAX_BATCH (32) as the default value regardless of Intel DSA or IAA device during work queue setup and cleanup. It should be always 0 on IAA. Fix the issues by setting the max batch size of device and max batch size of work queue to 0 on IAA device, that means batch is not supported. [1]: https://cdrdv2.intel.com/v1/dl/getContent/721858 Fixes: 23084545dbb0 ("dmaengine: idxd: set max_xfer and max_batch for RO device") Fixes: 92452a72ebdf ("dmaengine: idxd: set defaults for wq configs") Fixes: bfe1d56091c1 ("dmaengine: idxd: Init and probe for Intel data accelerators") Signed-off-by: Xiaochen Shen Reviewed-by: Dave Jiang Reviewed-by: Fenghua Yu Link: https://lore.kernel.org/r/20220930201528.18621-2-xiaochen.shen@intel.com Signed-off-by: Vinod Koul --- drivers/dma/idxd/device.c | 6 +++--- drivers/dma/idxd/idxd.h | 32 ++++++++++++++++++++++++++++++++ drivers/dma/idxd/init.c | 4 ++-- drivers/dma/idxd/sysfs.c | 2 +- 4 files changed, 38 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c index 2c1e6f6daa62..670b71927db0 100644 --- a/drivers/dma/idxd/device.c +++ b/drivers/dma/idxd/device.c @@ -390,7 +390,7 @@ static void idxd_wq_disable_cleanup(struct idxd_wq *wq) clear_bit(WQ_FLAG_ATS_DISABLE, &wq->flags); memset(wq->name, 0, WQ_NAME_SIZE); wq->max_xfer_bytes = WQ_DEFAULT_MAX_XFER; - wq->max_batch_size = WQ_DEFAULT_MAX_BATCH; + idxd_wq_set_max_batch_size(idxd->data->type, wq, WQ_DEFAULT_MAX_BATCH); if (wq->opcap_bmap) bitmap_copy(wq->opcap_bmap, idxd->opcap_bmap, IDXD_MAX_OPCAP_BITS); } @@ -869,7 +869,7 @@ static int idxd_wq_config_write(struct idxd_wq *wq) /* bytes 12-15 */ wq->wqcfg->max_xfer_shift = ilog2(wq->max_xfer_bytes); - wq->wqcfg->max_batch_shift = ilog2(wq->max_batch_size); + idxd_wqcfg_set_max_batch_shift(idxd->data->type, wq->wqcfg, ilog2(wq->max_batch_size)); /* bytes 32-63 */ if (idxd->hw.wq_cap.op_config && wq->opcap_bmap) { @@ -1051,7 +1051,7 @@ static int idxd_wq_load_config(struct idxd_wq *wq) wq->priority = wq->wqcfg->priority; wq->max_xfer_bytes = 1ULL << wq->wqcfg->max_xfer_shift; - wq->max_batch_size = 1ULL << wq->wqcfg->max_batch_shift; + idxd_wq_set_max_batch_size(idxd->data->type, wq, 1U << wq->wqcfg->max_batch_shift); for (i = 0; i < WQCFG_STRIDES(idxd); i++) { wqcfg_offset = WQCFG_OFFSET(idxd, wq->id, i); diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h index 1196ab342f01..7ced8d283d98 100644 --- a/drivers/dma/idxd/idxd.h +++ b/drivers/dma/idxd/idxd.h @@ -548,6 +548,38 @@ static inline int idxd_wq_refcount(struct idxd_wq *wq) return wq->client_count; }; +/* + * Intel IAA does not support batch processing. + * The max batch size of device, max batch size of wq and + * max batch shift of wqcfg should be always 0 on IAA. + */ +static inline void idxd_set_max_batch_size(int idxd_type, struct idxd_device *idxd, + u32 max_batch_size) +{ + if (idxd_type == IDXD_TYPE_IAX) + idxd->max_batch_size = 0; + else + idxd->max_batch_size = max_batch_size; +} + +static inline void idxd_wq_set_max_batch_size(int idxd_type, struct idxd_wq *wq, + u32 max_batch_size) +{ + if (idxd_type == IDXD_TYPE_IAX) + wq->max_batch_size = 0; + else + wq->max_batch_size = max_batch_size; +} + +static inline void idxd_wqcfg_set_max_batch_shift(int idxd_type, union wqcfg *wqcfg, + u32 max_batch_shift) +{ + if (idxd_type == IDXD_TYPE_IAX) + wqcfg->max_batch_shift = 0; + else + wqcfg->max_batch_shift = max_batch_shift; +} + int __must_check __idxd_driver_register(struct idxd_device_driver *idxd_drv, struct module *module, const char *mod_name); #define idxd_driver_register(driver) \ diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c index 2b18d512cbfc..09cbf0c179ba 100644 --- a/drivers/dma/idxd/init.c +++ b/drivers/dma/idxd/init.c @@ -183,7 +183,7 @@ static int idxd_setup_wqs(struct idxd_device *idxd) init_completion(&wq->wq_dead); init_completion(&wq->wq_resurrect); wq->max_xfer_bytes = WQ_DEFAULT_MAX_XFER; - wq->max_batch_size = WQ_DEFAULT_MAX_BATCH; + idxd_wq_set_max_batch_size(idxd->data->type, wq, WQ_DEFAULT_MAX_BATCH); wq->enqcmds_retries = IDXD_ENQCMDS_RETRIES; wq->wqcfg = kzalloc_node(idxd->wqcfg_size, GFP_KERNEL, dev_to_node(dev)); if (!wq->wqcfg) { @@ -418,7 +418,7 @@ static void idxd_read_caps(struct idxd_device *idxd) idxd->max_xfer_bytes = 1ULL << idxd->hw.gen_cap.max_xfer_shift; dev_dbg(dev, "max xfer size: %llu bytes\n", idxd->max_xfer_bytes); - idxd->max_batch_size = 1U << idxd->hw.gen_cap.max_batch_shift; + idxd_set_max_batch_size(idxd->data->type, idxd, 1U << idxd->hw.gen_cap.max_batch_shift); dev_dbg(dev, "max batch size: %u\n", idxd->max_batch_size); if (idxd->hw.gen_cap.config_en) set_bit(IDXD_FLAG_CONFIGURABLE, &idxd->flags); diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c index bdaccf9e0436..7269bd54554f 100644 --- a/drivers/dma/idxd/sysfs.c +++ b/drivers/dma/idxd/sysfs.c @@ -1046,7 +1046,7 @@ static ssize_t wq_max_batch_size_store(struct device *dev, struct device_attribu if (batch_size > idxd->max_batch_size) return -EINVAL; - wq->max_batch_size = (u32)batch_size; + idxd_wq_set_max_batch_size(idxd->data->type, wq, (u32)batch_size); return count; } -- cgit v1.2.3 From 0b8c97a1d8c1bb6a853b8bb1778e8fef17b86fc9 Mon Sep 17 00:00:00 2001 From: Fengqian Gao Date: Fri, 30 Sep 2022 11:28:35 +0800 Subject: dmaengine: idxd: fix RO device state error after been disabled/reset When IDXD is not configurable, that means its WQ, engine, and group configurations cannot be changed. But it can be disabled and its state should be set as disabled regardless it's configurable or not. Fix this by setting device state IDXD_DEV_DISABLED for read-only device as well in idxd_device_clear_state(). Fixes: cf4ac3fef338 ("dmaengine: idxd: fix lockdep warning on device driver removal") Signed-off-by: Fengqian Gao Reviewed-by: Xiaochen Shen Reviewed-by: Dave Jiang Reviewed-by: Fenghua Yu Link: https://lore.kernel.org/r/20220930032835.2290-1-fengqian.gao@intel.com Signed-off-by: Vinod Koul --- drivers/dma/idxd/device.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c index 670b71927db0..6f44fa8f78a5 100644 --- a/drivers/dma/idxd/device.c +++ b/drivers/dma/idxd/device.c @@ -730,13 +730,21 @@ static void idxd_device_wqs_clear_state(struct idxd_device *idxd) void idxd_device_clear_state(struct idxd_device *idxd) { - if (!test_bit(IDXD_FLAG_CONFIGURABLE, &idxd->flags)) - return; + /* IDXD is always disabled. Other states are cleared only when IDXD is configurable. */ + if (test_bit(IDXD_FLAG_CONFIGURABLE, &idxd->flags)) { + /* + * Clearing wq state is protected by wq lock. + * So no need to be protected by device lock. + */ + idxd_device_wqs_clear_state(idxd); + + spin_lock(&idxd->dev_lock); + idxd_groups_clear_state(idxd); + idxd_engines_clear_state(idxd); + } else { + spin_lock(&idxd->dev_lock); + } - idxd_device_wqs_clear_state(idxd); - spin_lock(&idxd->dev_lock); - idxd_groups_clear_state(idxd); - idxd_engines_clear_state(idxd); idxd->state = IDXD_DEV_DISABLED; spin_unlock(&idxd->dev_lock); } -- cgit v1.2.3 From 8454f880c24bca0d9d4bfb6ed4a4a5429a4d9b20 Mon Sep 17 00:00:00 2001 From: Martin Povišer Date: Wed, 19 Oct 2022 15:23:23 +0200 Subject: dmaengine: apple-admac: Fix grabbing of channels in of_xlate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The of_xlate callback is supposed to return the channel after already having 'grabbed' it for private use, so fill that in. Fixes: b127315d9a78 ("dmaengine: apple-admac: Add Apple ADMAC driver") Signed-off-by: Martin Povišer Link: https://lore.kernel.org/r/20221019132324.8585-1-povik+lin@cutebit.org Signed-off-by: Vinod Koul --- drivers/dma/apple-admac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/dma/apple-admac.c b/drivers/dma/apple-admac.c index 317ca76ccafd..a2cc520225d3 100644 --- a/drivers/dma/apple-admac.c +++ b/drivers/dma/apple-admac.c @@ -493,7 +493,7 @@ static struct dma_chan *admac_dma_of_xlate(struct of_phandle_args *dma_spec, return NULL; } - return &ad->channels[index].chan; + return dma_get_slave_channel(&ad->channels[index].chan); } static int admac_drain_reports(struct admac_data *ad, int channo) -- cgit v1.2.3 From 081195d17a0c4c636da2b869bd5809d42e8cbb13 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Mon, 24 Oct 2022 21:50:09 +0200 Subject: dmaengine: mv_xor_v2: Fix a resource leak in mv_xor_v2_remove() A clk_prepare_enable() call in the probe is not balanced by a corresponding clk_disable_unprepare() in the remove function. Add the missing call. Fixes: 3cd2c313f1d6 ("dmaengine: mv_xor_v2: Fix clock resource by adding a register clock") Signed-off-by: Christophe JAILLET Link: https://lore.kernel.org/r/e9e3837a680c9bd2438e4db2b83270c6c052d005.1666640987.git.christophe.jaillet@wanadoo.fr Signed-off-by: Vinod Koul --- drivers/dma/mv_xor_v2.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/dma/mv_xor_v2.c b/drivers/dma/mv_xor_v2.c index f629ef6fd3c2..113834e1167b 100644 --- a/drivers/dma/mv_xor_v2.c +++ b/drivers/dma/mv_xor_v2.c @@ -893,6 +893,7 @@ static int mv_xor_v2_remove(struct platform_device *pdev) tasklet_kill(&xor_dev->irq_tasklet); clk_disable_unprepare(xor_dev->clk); + clk_disable_unprepare(xor_dev->reg_clk); return 0; } -- cgit v1.2.3 From ac2b9f34f02052709aea7b34bb2a165e1853eb41 Mon Sep 17 00:00:00 2001 From: Yang Yingliang Date: Thu, 20 Oct 2022 14:28:27 +0800 Subject: dmaengine: ti: k3-udma-glue: fix memory leak when register device fail If device_register() fails, it should call put_device() to give up reference, the name allocated in dev_set_name() can be freed in callback function kobject_cleanup(). Fixes: 5b65781d06ea ("dmaengine: ti: k3-udma-glue: Add support for K3 PKTDMA") Signed-off-by: Yang Yingliang Acked-by: Peter Ujfalusi Link: https://lore.kernel.org/r/20221020062827.2914148-1-yangyingliang@huawei.com Signed-off-by: Vinod Koul --- drivers/dma/ti/k3-udma-glue.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers') diff --git a/drivers/dma/ti/k3-udma-glue.c b/drivers/dma/ti/k3-udma-glue.c index 4fdd9f06b723..4f1aeb81e9c7 100644 --- a/drivers/dma/ti/k3-udma-glue.c +++ b/drivers/dma/ti/k3-udma-glue.c @@ -299,6 +299,7 @@ struct k3_udma_glue_tx_channel *k3_udma_glue_request_tx_chn(struct device *dev, ret = device_register(&tx_chn->common.chan_dev); if (ret) { dev_err(dev, "Channel Device registration failed %d\n", ret); + put_device(&tx_chn->common.chan_dev); tx_chn->common.chan_dev.parent = NULL; goto err; } @@ -917,6 +918,7 @@ k3_udma_glue_request_rx_chn_priv(struct device *dev, const char *name, ret = device_register(&rx_chn->common.chan_dev); if (ret) { dev_err(dev, "Channel Device registration failed %d\n", ret); + put_device(&rx_chn->common.chan_dev); rx_chn->common.chan_dev.parent = NULL; goto err; } @@ -1048,6 +1050,7 @@ k3_udma_glue_request_remote_rx_chn(struct device *dev, const char *name, ret = device_register(&rx_chn->common.chan_dev); if (ret) { dev_err(dev, "Channel Device registration failed %d\n", ret); + put_device(&rx_chn->common.chan_dev); rx_chn->common.chan_dev.parent = NULL; goto err; } -- cgit v1.2.3 From 140fd5e74a1cc7413df88c735fff5ec64d33c1d3 Mon Sep 17 00:00:00 2001 From: Amelie Delaunay Date: Mon, 24 Oct 2022 10:36:11 +0200 Subject: dmaengine: stm32-dma: fix potential race between pause and resume When disabling dma channel, a TCF flag is set and as TCIE is enabled, an interrupt is raised. On a busy system, the interrupt may have latency and the user can ask for dmaengine_resume while stm32-dma driver has not yet managed the complete pause (backup of registers to restore state in resume). To avoid such a case, instead of waiting the interrupt to backup the registers, do it just after disabling the channel and discard Transfer Complete interrupt in case the channel is paused. Fixes: 099a9a94be0e ("dmaengine: stm32-dma: add device_pause/device_resume support") Signed-off-by: Amelie Delaunay Link: https://lore.kernel.org/r/20221024083611.132588-1-amelie.delaunay@foss.st.com Signed-off-by: Vinod Koul --- drivers/dma/stm32-dma.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/stm32-dma.c b/drivers/dma/stm32-dma.c index 4891a1767e5a..37674029cb42 100644 --- a/drivers/dma/stm32-dma.c +++ b/drivers/dma/stm32-dma.c @@ -675,6 +675,8 @@ static void stm32_dma_handle_chan_paused(struct stm32_dma_chan *chan) chan->chan_reg.dma_sndtr = stm32_dma_read(dmadev, STM32_DMA_SNDTR(chan->id)); + chan->status = DMA_PAUSED; + dev_dbg(chan2dev(chan), "vchan %pK: paused\n", &chan->vchan); } @@ -789,9 +791,7 @@ static irqreturn_t stm32_dma_chan_irq(int irq, void *devid) if (status & STM32_DMA_TCI) { stm32_dma_irq_clear(chan, STM32_DMA_TCI); if (scr & STM32_DMA_SCR_TCIE) { - if (chan->status == DMA_PAUSED && !(scr & STM32_DMA_SCR_EN)) - stm32_dma_handle_chan_paused(chan); - else + if (chan->status != DMA_PAUSED) stm32_dma_handle_chan_done(chan, scr); } status &= ~STM32_DMA_TCI; @@ -838,13 +838,11 @@ static int stm32_dma_pause(struct dma_chan *c) return -EPERM; spin_lock_irqsave(&chan->vchan.lock, flags); + ret = stm32_dma_disable_chan(chan); - /* - * A transfer complete flag is set to indicate the end of transfer due to the stream - * interruption, so wait for interrupt - */ if (!ret) - chan->status = DMA_PAUSED; + stm32_dma_handle_chan_paused(chan); + spin_unlock_irqrestore(&chan->vchan.lock, flags); return ret; -- cgit v1.2.3 From f1171bbdd2ba2a50ee64bb198a78c268a5baf5f1 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:35 +0300 Subject: dmaengine: at_hdmac: Fix at_lli struct definition Those hardware registers are all of 32 bits, while dma_addr_t ca be of type u64 or u32 depending on CONFIG_ARCH_DMA_ADDR_T_64BIT. Force u32 to comply with what the hardware expects. Fixes: dc78baa2b90b ("dmaengine: at_hdmac: new driver for the Atmel AHB DMA Controller") Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-2-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac_regs.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac_regs.h b/drivers/dma/at_hdmac_regs.h index 4d1ebc040031..d4d382d74607 100644 --- a/drivers/dma/at_hdmac_regs.h +++ b/drivers/dma/at_hdmac_regs.h @@ -186,13 +186,13 @@ /* LLI == Linked List Item; aka DMA buffer descriptor */ struct at_lli { /* values that are not changed by hardware */ - dma_addr_t saddr; - dma_addr_t daddr; + u32 saddr; + u32 daddr; /* value that may get written back: */ - u32 ctrla; + u32 ctrla; /* more values that are not changed by hardware */ - u32 ctrlb; - dma_addr_t dscr; /* chain to next lli */ + u32 ctrlb; + u32 dscr; /* chain to next lli */ }; /** -- cgit v1.2.3 From 7176a6a8982d311e50a7c1168868d26e65bbba19 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:36 +0300 Subject: dmaengine: at_hdmac: Don't start transactions at tx_submit level tx_submit is supposed to push the current transaction descriptor to a pending queue, waiting for issue_pending() to be called. issue_pending() must start the transfer, not tx_submit(), thus remove atc_dostart() from atc_tx_submit(). Clients of at_xdmac that assume that tx_submit() starts the transfer must be updated and call dma_async_issue_pending() if they miss to call it. The vdbg print was moved to after the lock is released. It is desirable to do the prints without the lock held if possible, and because the if statement disappears there's no reason why to do the print while holding the lock. Fixes: dc78baa2b90b ("dmaengine: at_hdmac: new driver for the Atmel AHB DMA Controller") Reported-by: Peter Rosin Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-3-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index 5a50423b7378..3f71f4d2f467 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -691,19 +691,11 @@ static dma_cookie_t atc_tx_submit(struct dma_async_tx_descriptor *tx) spin_lock_irqsave(&atchan->lock, flags); cookie = dma_cookie_assign(tx); - if (list_empty(&atchan->active_list)) { - dev_vdbg(chan2dev(tx->chan), "tx_submit: started %u\n", - desc->txd.cookie); - atc_dostart(atchan, desc); - list_add_tail(&desc->desc_node, &atchan->active_list); - } else { - dev_vdbg(chan2dev(tx->chan), "tx_submit: queued %u\n", - desc->txd.cookie); - list_add_tail(&desc->desc_node, &atchan->queue); - } - + list_add_tail(&desc->desc_node, &atchan->queue); spin_unlock_irqrestore(&atchan->lock, flags); + dev_vdbg(chan2dev(tx->chan), "tx_submit: queued %u\n", + desc->txd.cookie); return cookie; } -- cgit v1.2.3 From 8a47221fc28417ff8a32a4f92d4448a56c3cf7e1 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:37 +0300 Subject: dmaengine: at_hdmac: Start transfer for cyclic channels in issue_pending Cyclic channels must too call issue_pending in order to start a transfer. Start the transfer in issue_pending regardless of the type of channel. This wrongly worked before, because in the past the transfer was started at tx_submit level when only a desc in the transfer list. Fixes: 53830cc75974 ("dmaengine: at_hdmac: add cyclic DMA operation support") Reported-by: Peter Rosin Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-4-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index 3f71f4d2f467..e9d0c3632868 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -1536,10 +1536,6 @@ static void atc_issue_pending(struct dma_chan *chan) dev_vdbg(chan2dev(chan), "issue_pending\n"); - /* Not needed for cyclic transfers */ - if (atc_chan_is_cyclic(atchan)) - return; - atc_advance_work(atchan); } -- cgit v1.2.3 From fcd37565efdaffeac179d0f0ce980ac79bfdf569 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:38 +0300 Subject: dmaengine: at_hdmac: Fix premature completion of desc in issue_pending Multiple calls to atc_issue_pending() could result in a premature completion of a descriptor from the atchan->active list, as the method always completed the first active descriptor from the list. Instead, issue_pending() should just take the first transaction descriptor from the pending queue, move it to active_list and start the transfer. Fixes: dc78baa2b90b ("dmaengine: at_hdmac: new driver for the Atmel AHB DMA Controller") Reported-by: Peter Rosin Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-5-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index e9d0c3632868..cb5522417db6 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -1527,16 +1527,26 @@ atc_tx_status(struct dma_chan *chan, } /** - * atc_issue_pending - try to finish work + * atc_issue_pending - takes the first transaction descriptor in the pending + * queue and starts the transfer. * @chan: target DMA channel */ static void atc_issue_pending(struct dma_chan *chan) { - struct at_dma_chan *atchan = to_at_dma_chan(chan); + struct at_dma_chan *atchan = to_at_dma_chan(chan); + struct at_desc *desc; + unsigned long flags; dev_vdbg(chan2dev(chan), "issue_pending\n"); - atc_advance_work(atchan); + spin_lock_irqsave(&atchan->lock, flags); + if (atc_chan_is_enabled(atchan) || list_empty(&atchan->queue)) + return spin_unlock_irqrestore(&atchan->lock, flags); + + desc = atc_first_queued(atchan); + list_move_tail(&desc->desc_node, &atchan->active_list); + atc_dostart(atchan, desc); + spin_unlock_irqrestore(&atchan->lock, flags); } /** -- cgit v1.2.3 From f645f85ae1104f8bd882f962ac0a69a1070076dd Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:39 +0300 Subject: dmaengine: at_hdmac: Do not call the complete callback on device_terminate_all The method was wrong because it violated the dmaengine API. For aborted transfers the complete callback should not be called. Fix the behavior and do not call the complete callback on device_terminate_all. Fixes: 808347f6a317 ("dmaengine: at_hdmac: add DMA slave transfers") Reported-by: Peter Rosin Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-6-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index cb5522417db6..11816484843e 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -1437,11 +1437,8 @@ static int atc_terminate_all(struct dma_chan *chan) struct at_dma_chan *atchan = to_at_dma_chan(chan); struct at_dma *atdma = to_at_dma(chan->device); int chan_id = atchan->chan_common.chan_id; - struct at_desc *desc, *_desc; unsigned long flags; - LIST_HEAD(list); - dev_vdbg(chan2dev(chan), "%s\n", __func__); /* @@ -1460,15 +1457,11 @@ static int atc_terminate_all(struct dma_chan *chan) cpu_relax(); /* active_list entries will end up before queued entries */ - list_splice_init(&atchan->queue, &list); - list_splice_init(&atchan->active_list, &list); + list_splice_tail_init(&atchan->queue, &atchan->free_list); + list_splice_tail_init(&atchan->active_list, &atchan->free_list); spin_unlock_irqrestore(&atchan->lock, flags); - /* Flush all pending and queued descriptors */ - list_for_each_entry_safe(desc, _desc, &list, desc_node) - atc_chain_complete(atchan, desc); - clear_bit(ATC_IS_PAUSED, &atchan->status); /* if channel dedicated to cyclic operations, free it */ clear_bit(ATC_IS_CYCLIC, &atchan->status); -- cgit v1.2.3 From 6e5ad28d16f082efeae3d0bd2e31f24bed218019 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:40 +0300 Subject: dmaengine: at_hdmac: Protect atchan->status with the channel lock Now that the complete callback call was removed from device_terminate_all(), we can protect the atchan->status with the channel lock. The atomic bitops on atchan->status do not substitute proper locking on the status, as one could still modify the status after the lock was dropped in atc_terminate_all() but before the atomic bitops were executed. Fixes: 078a6506141a ("dmaengine: at_hdmac: Fix deadlocks") Reported-by: Peter Rosin Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-7-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index 11816484843e..deb4c6027436 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -1460,12 +1460,12 @@ static int atc_terminate_all(struct dma_chan *chan) list_splice_tail_init(&atchan->queue, &atchan->free_list); list_splice_tail_init(&atchan->active_list, &atchan->free_list); - spin_unlock_irqrestore(&atchan->lock, flags); - clear_bit(ATC_IS_PAUSED, &atchan->status); /* if channel dedicated to cyclic operations, free it */ clear_bit(ATC_IS_CYCLIC, &atchan->status); + spin_unlock_irqrestore(&atchan->lock, flags); + return 0; } -- cgit v1.2.3 From c6babed879fbe82796a601bf097649e07382db46 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:41 +0300 Subject: dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all() atc_complete_all() had concurrency bugs, thus remove it: 1/ atc_complete_all() in its entirety was buggy, as when the atchan->queue list (the one that contains descriptors that are not yet issued to the hardware) contained descriptors, it fired just the first from the atchan->queue, but moved all the desc from atchan->queue to atchan->active_list and considered them all as fired. This could result in calling the completion of a descriptor that was not yet issued to the hardware. 2/ when in tasklet at atc_advance_work() time, atchan->active_list was queried without holding the lock of the chan. This can result in atchan->active_list concurrency problems between the tasklet and issue_pending(). Fixes: dc78baa2b90b ("dmaengine: at_hdmac: new driver for the Atmel AHB DMA Controller") Reported-by: Peter Rosin Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-8-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 49 ++++--------------------------------------------- 1 file changed, 4 insertions(+), 45 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index deb4c6027436..f1e6fa6af6c2 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -485,42 +485,6 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc) dma_run_dependencies(txd); } -/** - * atc_complete_all - finish work for all transactions - * @atchan: channel to complete transactions for - * - * Eventually submit queued descriptors if any - * - * Assume channel is idle while calling this function - * Called with atchan->lock held and bh disabled - */ -static void atc_complete_all(struct at_dma_chan *atchan) -{ - struct at_desc *desc, *_desc; - LIST_HEAD(list); - unsigned long flags; - - dev_vdbg(chan2dev(&atchan->chan_common), "complete all\n"); - - spin_lock_irqsave(&atchan->lock, flags); - - /* - * Submit queued descriptors ASAP, i.e. before we go through - * the completed ones. - */ - if (!list_empty(&atchan->queue)) - atc_dostart(atchan, atc_first_queued(atchan)); - /* empty active_list now it is completed */ - list_splice_init(&atchan->active_list, &list); - /* empty queue list by moving descriptors (if any) to active_list */ - list_splice_init(&atchan->queue, &atchan->active_list); - - spin_unlock_irqrestore(&atchan->lock, flags); - - list_for_each_entry_safe(desc, _desc, &list, desc_node) - atc_chain_complete(atchan, desc); -} - /** * atc_advance_work - at the end of a transaction, move forward * @atchan: channel where the transaction ended @@ -528,25 +492,20 @@ static void atc_complete_all(struct at_dma_chan *atchan) static void atc_advance_work(struct at_dma_chan *atchan) { unsigned long flags; - int ret; dev_vdbg(chan2dev(&atchan->chan_common), "advance_work\n"); spin_lock_irqsave(&atchan->lock, flags); - ret = atc_chan_is_enabled(atchan); + if (atc_chan_is_enabled(atchan) || list_empty(&atchan->active_list)) + return spin_unlock_irqrestore(&atchan->lock, flags); spin_unlock_irqrestore(&atchan->lock, flags); - if (ret) - return; - - if (list_empty(&atchan->active_list) || - list_is_singular(&atchan->active_list)) - return atc_complete_all(atchan); atc_chain_complete(atchan, atc_first_active(atchan)); /* advance work */ spin_lock_irqsave(&atchan->lock, flags); - atc_dostart(atchan, atc_first_active(atchan)); + if (!list_empty(&atchan->active_list)) + atc_dostart(atchan, atc_first_active(atchan)); spin_unlock_irqrestore(&atchan->lock, flags); } -- cgit v1.2.3 From 06988949df8c3007ad82036d3606d8ae72ed9000 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:42 +0300 Subject: dmaengine: at_hdmac: Fix concurrency over descriptor The descriptor was added to the free_list before calling the callback, which could result in reissuing of the same descriptor and calling of a single callback for both. Move the decriptor to the free list after the callback is invoked. Fixes: dc78baa2b90b ("dmaengine: at_hdmac: new driver for the Atmel AHB DMA Controller") Reported-by: Peter Rosin Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-9-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index f1e6fa6af6c2..2012ecc57826 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -469,11 +469,8 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc) desc->memset_buffer = false; } - /* move children to free_list */ - list_splice_init(&desc->tx_list, &atchan->free_list); - /* move myself to free_list */ - list_move(&desc->desc_node, &atchan->free_list); - + /* Remove transfer node from the active list. */ + list_del_init(&desc->desc_node); spin_unlock_irqrestore(&atchan->lock, flags); dma_descriptor_unmap(txd); @@ -483,6 +480,13 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc) dmaengine_desc_get_callback_invoke(txd, NULL); dma_run_dependencies(txd); + + spin_lock_irqsave(&atchan->lock, flags); + /* move children to free_list */ + list_splice_init(&desc->tx_list, &atchan->free_list); + /* add myself to free_list */ + list_add(&desc->desc_node, &atchan->free_list); + spin_unlock_irqrestore(&atchan->lock, flags); } /** -- cgit v1.2.3 From 6ba826cbb57d675f447b59323204d1473bbd5593 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:43 +0300 Subject: dmaengine: at_hdmac: Free the memset buf without holding the chan lock There's no need to hold the channel lock when freeing the memset buf, as the operation has already completed. Free the memset buf without holding the channel lock. Fixes: 4d112426c344 ("dmaengine: hdmac: Add memset capabilities") Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-10-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index 2012ecc57826..0fb44f622d35 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -462,13 +462,6 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc) if (!atc_chan_is_cyclic(atchan)) dma_cookie_complete(txd); - /* If the transfer was a memset, free our temporary buffer */ - if (desc->memset_buffer) { - dma_pool_free(atdma->memset_pool, desc->memset_vaddr, - desc->memset_paddr); - desc->memset_buffer = false; - } - /* Remove transfer node from the active list. */ list_del_init(&desc->desc_node); spin_unlock_irqrestore(&atchan->lock, flags); @@ -487,6 +480,13 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc) /* add myself to free_list */ list_add(&desc->desc_node, &atchan->free_list); spin_unlock_irqrestore(&atchan->lock, flags); + + /* If the transfer was a memset, free our temporary buffer */ + if (desc->memset_buffer) { + dma_pool_free(atdma->memset_pool, desc->memset_vaddr, + desc->memset_paddr); + desc->memset_buffer = false; + } } /** -- cgit v1.2.3 From 03ed9ba357cc78116164b90b87f45eacab60b561 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:44 +0300 Subject: dmaengine: at_hdmac: Fix concurrency over the active list The tasklet (atc_advance_work()) did not held the channel lock when retrieving the first active descriptor, causing concurrency problems if issue_pending() was called in between. If issue_pending() was called exactly after the lock was released in the tasklet (atc_advance_work()), atc_chain_complete() could complete a descriptor for which the controller has not yet raised an interrupt. Fixes: dc78baa2b90b ("dmaengine: at_hdmac: new driver for the Atmel AHB DMA Controller") Reported-by: Peter Rosin Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-11-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index 0fb44f622d35..b53a9fc15dd9 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -462,8 +462,6 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc) if (!atc_chan_is_cyclic(atchan)) dma_cookie_complete(txd); - /* Remove transfer node from the active list. */ - list_del_init(&desc->desc_node); spin_unlock_irqrestore(&atchan->lock, flags); dma_descriptor_unmap(txd); @@ -495,6 +493,7 @@ atc_chain_complete(struct at_dma_chan *atchan, struct at_desc *desc) */ static void atc_advance_work(struct at_dma_chan *atchan) { + struct at_desc *desc; unsigned long flags; dev_vdbg(chan2dev(&atchan->chan_common), "advance_work\n"); @@ -502,9 +501,12 @@ static void atc_advance_work(struct at_dma_chan *atchan) spin_lock_irqsave(&atchan->lock, flags); if (atc_chan_is_enabled(atchan) || list_empty(&atchan->active_list)) return spin_unlock_irqrestore(&atchan->lock, flags); - spin_unlock_irqrestore(&atchan->lock, flags); - atc_chain_complete(atchan, atc_first_active(atchan)); + desc = atc_first_active(atchan); + /* Remove the transfer node from the active list. */ + list_del_init(&desc->desc_node); + spin_unlock_irqrestore(&atchan->lock, flags); + atc_chain_complete(atchan, desc); /* advance work */ spin_lock_irqsave(&atchan->lock, flags); -- cgit v1.2.3 From ba2423633ba646e1df20e30cb3cf35495c16f173 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:45 +0300 Subject: dmaengine: at_hdmac: Fix descriptor handling when issuing it to hardware As it was before, the descriptor was issued to the hardware without adding it to the active (issued) list. This could result in a completion of other descriptor, or/and in the descriptor never being completed. Fixes: dc78baa2b90b ("dmaengine: at_hdmac: new driver for the Atmel AHB DMA Controller") Reported-by: Peter Rosin Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-12-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index b53a9fc15dd9..9e5a30396c1c 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -510,8 +510,11 @@ static void atc_advance_work(struct at_dma_chan *atchan) /* advance work */ spin_lock_irqsave(&atchan->lock, flags); - if (!list_empty(&atchan->active_list)) - atc_dostart(atchan, atc_first_active(atchan)); + if (!list_empty(&atchan->active_list)) { + desc = atc_first_queued(atchan); + list_move_tail(&desc->desc_node, &atchan->active_list); + atc_dostart(atchan, desc); + } spin_unlock_irqrestore(&atchan->lock, flags); } @@ -523,6 +526,7 @@ static void atc_advance_work(struct at_dma_chan *atchan) static void atc_handle_error(struct at_dma_chan *atchan) { struct at_desc *bad_desc; + struct at_desc *desc; struct at_desc *child; unsigned long flags; @@ -540,8 +544,11 @@ static void atc_handle_error(struct at_dma_chan *atchan) list_splice_init(&atchan->queue, atchan->active_list.prev); /* Try to restart the controller */ - if (!list_empty(&atchan->active_list)) - atc_dostart(atchan, atc_first_active(atchan)); + if (!list_empty(&atchan->active_list)) { + desc = atc_first_queued(atchan); + list_move_tail(&desc->desc_node, &atchan->active_list); + atc_dostart(atchan, desc); + } /* * KERN_CRITICAL may seem harsh, but since this only happens -- cgit v1.2.3 From ef2cb4f0ce479f77607b04c4b0414bf32f863ee8 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:46 +0300 Subject: dmaengine: at_hdmac: Fix completion of unissued descriptor in case of errors In case the controller detected an error, the code took the chance to move all the queued (submitted) descriptors to the active (issued) list. This was wrong as if there were any descriptors in the submitted list they were moved to the issued list without actually issuing them to the controller, thus a completion could be raised without even fireing the descriptor. Fixes: dc78baa2b90b ("dmaengine: at_hdmac: new driver for the Atmel AHB DMA Controller") Reported-by: Peter Rosin Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-13-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index 9e5a30396c1c..80eeb4fb88ef 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -539,10 +539,6 @@ static void atc_handle_error(struct at_dma_chan *atchan) bad_desc = atc_first_active(atchan); list_del_init(&bad_desc->desc_node); - /* As we are stopped, take advantage to push queued descriptors - * in active_list */ - list_splice_init(&atchan->queue, atchan->active_list.prev); - /* Try to restart the controller */ if (!list_empty(&atchan->active_list)) { desc = atc_first_queued(atchan); -- cgit v1.2.3 From 580ee84405c27d6ed419abe4d2b3de1968abdafd Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:47 +0300 Subject: dmaengine: at_hdmac: Don't allow CPU to reorder channel enable at_hdmac uses __raw_writel for register writes. In the absence of a barrier, the CPU may reorder the register operations. Introduce a write memory barrier so that the CPU does not reorder the channel enable, thus the start of the transfer, without making sure that all the pre-required register fields are already written. Fixes: dc78baa2b90b ("dmaengine: at_hdmac: new driver for the Atmel AHB DMA Controller") Reported-by: Peter Rosin Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Link: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-14-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index 80eeb4fb88ef..968a5aba47cd 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -256,6 +256,8 @@ static void atc_dostart(struct at_dma_chan *atchan, struct at_desc *first) ATC_SPIP_BOUNDARY(first->boundary)); channel_writel(atchan, DPIP, ATC_DPIP_HOLE(first->dst_hole) | ATC_DPIP_BOUNDARY(first->boundary)); + /* Don't allow CPU to reorder channel enable. */ + wmb(); dma_writel(atdma, CHER, atchan->mask); vdbg_dump_regs(atchan); -- cgit v1.2.3 From 28cbe5a0a46a6637adbda52337d7b2777fc04027 Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:48 +0300 Subject: dmaengine: at_hdmac: Fix impossible condition The iterator can not be greater than ATC_MAX_DSCR_TRIALS, as the for loop will stop when i == ATC_MAX_DSCR_TRIALS. While here, use the common "i" name for the iterator. Fixes: 93dce3a6434f ("dmaengine: at_hdmac: fix residue computation") Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-15-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index 968a5aba47cd..afcbad3e1718 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -318,7 +318,8 @@ static int atc_get_bytes_left(struct dma_chan *chan, dma_cookie_t cookie) struct at_desc *desc_first = atc_first_active(atchan); struct at_desc *desc; int ret; - u32 ctrla, dscr, trials; + u32 ctrla, dscr; + unsigned int i; /* * If the cookie doesn't match to the currently running transfer then @@ -388,7 +389,7 @@ static int atc_get_bytes_left(struct dma_chan *chan, dma_cookie_t cookie) dscr = channel_readl(atchan, DSCR); rmb(); /* ensure DSCR is read before CTRLA */ ctrla = channel_readl(atchan, CTRLA); - for (trials = 0; trials < ATC_MAX_DSCR_TRIALS; ++trials) { + for (i = 0; i < ATC_MAX_DSCR_TRIALS; ++i) { u32 new_dscr; rmb(); /* ensure DSCR is read after CTRLA */ @@ -414,7 +415,7 @@ static int atc_get_bytes_left(struct dma_chan *chan, dma_cookie_t cookie) rmb(); /* ensure DSCR is read before CTRLA */ ctrla = channel_readl(atchan, CTRLA); } - if (unlikely(trials >= ATC_MAX_DSCR_TRIALS)) + if (unlikely(i == ATC_MAX_DSCR_TRIALS)) return -ETIMEDOUT; /* for the first descriptor we can be more accurate */ -- cgit v1.2.3 From c47e6403fa099f200868d6b106701cb42d181d2b Mon Sep 17 00:00:00 2001 From: Tudor Ambarus Date: Tue, 25 Oct 2022 12:02:49 +0300 Subject: dmaengine: at_hdmac: Check return code of dma_async_device_register dma_async_device_register() can fail, check the return code and display an error. Fixes: dc78baa2b90b ("dmaengine: at_hdmac: new driver for the Atmel AHB DMA Controller") Signed-off-by: Tudor Ambarus Cc: stable@vger.kernel.org Acked-by: Nicolas Ferre Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Link: https://lore.kernel.org/r/20221025090306.297886-16-tudor.ambarus@microchip.com Signed-off-by: Vinod Koul --- drivers/dma/at_hdmac.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index afcbad3e1718..858bd64f1313 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -1928,7 +1928,11 @@ static int __init at_dma_probe(struct platform_device *pdev) dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask) ? "slave " : "", plat_dat->nr_channels); - dma_async_device_register(&atdma->dma_common); + err = dma_async_device_register(&atdma->dma_common); + if (err) { + dev_err(&pdev->dev, "Unable to register: %d.\n", err); + goto err_dma_async_device_register; + } /* * Do not return an error if the dmac node is not present in order to @@ -1948,6 +1952,7 @@ static int __init at_dma_probe(struct platform_device *pdev) err_of_dma_controller_register: dma_async_device_unregister(&atdma->dma_common); +err_dma_async_device_register: dma_pool_destroy(atdma->memset_pool); err_memset_pool_create: dma_pool_destroy(atdma->dma_desc_pool); -- cgit v1.2.3