From patchwork Sat Dec 22 07:28:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 10741435 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 894E5746 for ; Sat, 22 Dec 2018 18:19:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0A0262893C for ; Sat, 22 Dec 2018 18:19:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id F28A6289E5; Sat, 22 Dec 2018 18:19:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6DDEF2893C for ; Sat, 22 Dec 2018 18:19:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2391094AbeLVSTX (ORCPT ); Sat, 22 Dec 2018 13:19:23 -0500 Received: from mailout3.hostsharing.net ([176.9.242.54]:50493 "EHLO mailout3.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2391097AbeLVSTW (ORCPT ); Sat, 22 Dec 2018 13:19:22 -0500 Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.hostsharing.net", Issuer "COMODO RSA Domain Validation Secure Server CA" (not verified)) by mailout3.hostsharing.net (Postfix) with ESMTPS id D6571101E693E; Sat, 22 Dec 2018 08:42:21 +0100 (CET) Received: from localhost (unknown [89.246.108.87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 30E066031F05; Sat, 22 Dec 2018 08:42:21 +0100 (CET) X-Mailbox-Line: From 72be86ba05f9dde6cebeb4763da831f0d2f642d0 Mon Sep 17 00:00:00 2001 Message-Id: <72be86ba05f9dde6cebeb4763da831f0d2f642d0.1545462045.git.lukas@wunner.de> In-Reply-To: References: From: Lukas Wunner Date: Sat, 22 Dec 2018 08:28:45 +0100 Subject: [PATCH 1/5] dmaengine: bcm2835: Fix interrupt race on RT To: Vinod Koul , Eric Anholt , Stefan Wahren Cc: Frank Pavlic , Martin Sperl , Florian Meier , dmaengine@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, Tiejun Chen , linux-rt-users@vger.kernel.org Sender: dmaengine-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP If IRQ handlers are threaded (either because CONFIG_PREEMPT_RT_BASE is enabled or "threadirqs" was passed on the command line) and if system load is sufficiently high that wakeup latency of IRQ threads degrades, SPI DMA transactions on the BCM2835 occasionally break like this: ks8851 spi0.0: SPI transfer timed out bcm2835-dma 3f007000.dma: DMA transfer could not be terminated ks8851 spi0.0 eth2: ks8851_rdfifo: spi_sync() failed The root cause is an assumption made by the DMA driver which is documented in a code comment in bcm2835_dma_terminate_all(): /* * Stop DMA activity: we assume the callback will not be called * after bcm_dma_abort() returns (even if it does, it will see * c->desc is NULL and exit.) */ That assumption falls apart if the IRQ handler bcm2835_dma_callback() is threaded: A client may terminate a descriptor and issue a new one before the IRQ handler had a chance to run. In fact the IRQ handler may miss an *arbitrary* number of descriptors. The result is the following race condition: 1. A descriptor finishes, its interrupt is deferred to the IRQ thread. 2. A client calls dma_terminate_async() which sets channel->desc = NULL. 3. The client issues a new descriptor. Because channel->desc is NULL, bcm2835_dma_issue_pending() immediately starts the descriptor. 4. Finally the IRQ thread runs and writes BCM2835_DMA_INT to the CS register to acknowledge the interrupt. This clears the ACTIVE flag, so the newly issued descriptor is paused in the middle of the transaction. Because channel->desc is not NULL, the IRQ thread finalizes the descriptor and tries to start the next one. I see two possible solutions: The first is to call synchronize_irq() in bcm2835_dma_issue_pending() to wait until the IRQ thread has finished before issuing a new descriptor. The downside of this approach is unnecessary latency if clients desire rapidly terminating and re-issuing descriptors and don't have any use for an IRQ callback. (The SPI TX DMA channel is a case in point.) A better alternative is to make the IRQ thread recognize that it has missed descriptors and avoid finalizing the newly issued descriptor. Therefore, only finalize a descriptor if the END flag is set in the CS register. Clear the flag when starting a new descriptor. Perform both operations under the vchan lock. That way, there is practically no impact on latency and throughput if the client doesn't care for the interrupt: Only minimal additional overhead is introduced as one further MMIO read is necessary per interrupt. That MMIO read is needed to write the same value to the CS register that it currently has, thereby preserving the ACTIVE flag while clearing the INT and END flags. Note that the transaction may finish between reading and writing the CS register, and in that case the write changes the ACTIVE flag from 0 to 1. But that's harmless, the chip will notice that NEXTCONBK is 0 and self-clear the ACTIVE flag again. Fixes: 96286b576690 ("dmaengine: Add support for BCM2835") Signed-off-by: Lukas Wunner Cc: stable@vger.kernel.org # v3.14+ Cc: Frank Pavlic Cc: Martin Sperl Cc: Florian Meier --- drivers/dma/bcm2835-dma.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c index 1a44c8086d77..e94f41c56975 100644 --- a/drivers/dma/bcm2835-dma.c +++ b/drivers/dma/bcm2835-dma.c @@ -456,7 +456,8 @@ static void bcm2835_dma_start_desc(struct bcm2835_chan *c) c->desc = d = to_bcm2835_dma_desc(&vd->tx); writel(d->cb_list[0].paddr, c->chan_base + BCM2835_DMA_ADDR); - writel(BCM2835_DMA_ACTIVE, c->chan_base + BCM2835_DMA_CS); + writel(BCM2835_DMA_ACTIVE | BCM2835_DMA_END, + c->chan_base + BCM2835_DMA_CS); } static irqreturn_t bcm2835_dma_callback(int irq, void *data) @@ -464,6 +465,7 @@ static irqreturn_t bcm2835_dma_callback(int irq, void *data) struct bcm2835_chan *c = data; struct bcm2835_desc *d; unsigned long flags; + u32 cs; /* check the shared interrupt */ if (c->irq_flags & IRQF_SHARED) { @@ -477,11 +479,17 @@ static irqreturn_t bcm2835_dma_callback(int irq, void *data) spin_lock_irqsave(&c->vc.lock, flags); /* Acknowledge interrupt */ - writel(BCM2835_DMA_INT, c->chan_base + BCM2835_DMA_CS); + cs = readl(c->chan_base + BCM2835_DMA_CS); + writel(cs, c->chan_base + BCM2835_DMA_CS); d = c->desc; - if (d) { + /* + * If this IRQ handler is threaded, clients may terminate descriptors + * and issue new ones before the IRQ handler runs. Avoid finalizing + * such a newly issued descriptor by checking the END flag. + */ + if (d && cs & BCM2835_DMA_END) { if (d->cyclic) { /* call the cyclic callback */ vchan_cyclic_callback(&d->vd); @@ -789,11 +797,7 @@ static int bcm2835_dma_terminate_all(struct dma_chan *chan) list_del_init(&c->node); spin_unlock(&d->lock); - /* - * Stop DMA activity: we assume the callback will not be called - * after bcm_dma_abort() returns (even if it does, it will see - * c->desc is NULL and exit.) - */ + /* stop DMA activity */ if (c->desc) { vchan_terminate_vdesc(&c->desc->vd); c->desc = NULL; From patchwork Sat Dec 22 07:28:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 10741433 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1CDA113A4 for ; Sat, 22 Dec 2018 18:19:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9F9702893B for ; Sat, 22 Dec 2018 18:19:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 93CF6289E5; Sat, 22 Dec 2018 18:19:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B8AB52893B for ; Sat, 22 Dec 2018 18:19:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2391325AbeLVSTV (ORCPT ); Sat, 22 Dec 2018 13:19:21 -0500 Received: from mailout3.hostsharing.net ([176.9.242.54]:34459 "EHLO mailout3.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2391094AbeLVSTU (ORCPT ); Sat, 22 Dec 2018 13:19:20 -0500 Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.hostsharing.net", Issuer "COMODO RSA Domain Validation Secure Server CA" (not verified)) by mailout3.hostsharing.net (Postfix) with ESMTPS id 9D226101E693F; Sat, 22 Dec 2018 08:43:28 +0100 (CET) Received: from localhost (unknown [89.246.108.87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 337146031F05; Sat, 22 Dec 2018 08:43:28 +0100 (CET) X-Mailbox-Line: From 0eb99caa68fa697af01b90e2711d30ea3545062f Mon Sep 17 00:00:00 2001 Message-Id: <0eb99caa68fa697af01b90e2711d30ea3545062f.1545462045.git.lukas@wunner.de> In-Reply-To: References: From: Lukas Wunner Date: Sat, 22 Dec 2018 08:28:45 +0100 Subject: [PATCH 2/5] dmaengine: bcm2835: Fix abort of transactions To: Vinod Koul , Eric Anholt , Stefan Wahren Cc: Frank Pavlic , Martin Sperl , Florian Meier , dmaengine@vger.kernel.org, linux-rpi-kernel@lists.infradead.org Sender: dmaengine-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP There are multiple issues with bcm2835_dma_abort() (which is called on termination of a transaction): * The algorithm to abort the transaction first pauses the channel by clearing the ACTIVE flag in the CS register, then waits for the PAUSED flag to clear. Page 49 of the spec documents the latter as follows: "Indicates if the DMA is currently paused and not transferring data. This will occur if the active bit has been cleared [...]" https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf So the function is entering an infinite loop because it is waiting for PAUSED to clear which is always set due to the function having cleared the ACTIVE flag. The only thing that's saving it from itself is the upper bound of 10000 loop iterations. The code comment says that the intention is to "wait for any current AXI transfer to complete", so the author probably wanted to check the WAITING_FOR_OUTSTANDING_WRITES flag instead. Amend the function accordingly. * The CS register is only read at the beginning of the function. It needs to be read again after pausing the channel and before checking for outstanding writes, otherwise writes which were issued between the register read at the beginning of the function and pausing the channel may not be waited for. * The function seeks to abort the transfer by writing 0 to the NEXTCONBK register and setting the ABORT and ACTIVE flags. Thereby, the 0 in NEXTCONBK is sought to be loaded into the CONBLK_AD register. However experimentation has shown this approach to not work: The CONBLK_AD register remains the same as before and the CS register contains 0x00000030 (PAUSED | DREQ_STOPS_DMA). In other words, the control block is not aborted but merely paused and it will be resumed once the next DMA transaction is started. That is absolutely not the desired behavior. A simpler approach is to set the channel's RESET flag instead. This reliably zeroes the NEXTCONBK as well as the CS register. It requires less code and only a single MMIO write. This is also what popular user space DMA drivers do, e.g.: https://github.com/metachris/RPIO/blob/master/source/c_pwm/pwm.c Note that the spec is contradictory whether the NEXTCONBK register is writeable at all. On the one hand, page 41 claims: "The value loaded into the NEXTCONBK register can be overwritten so that the linked list of Control Block data structures can be dynamically altered. However it is only safe to do this when the DMA is paused." On the other hand, page 40 specifies: "Only three registers in each channel's register set are directly writeable (CS, CONBLK_AD and DEBUG). The other registers (TI, SOURCE_AD, DEST_AD, TXFR_LEN, STRIDE & NEXTCONBK), are automatically loaded from a Control Block data structure held in external memory." Fixes: 96286b576690 ("dmaengine: Add support for BCM2835") Signed-off-by: Lukas Wunner Cc: stable@vger.kernel.org # v3.14+ Cc: Frank Pavlic Cc: Martin Sperl Cc: Florian Meier --- drivers/dma/bcm2835-dma.c | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c index e94f41c56975..17bc7304db3a 100644 --- a/drivers/dma/bcm2835-dma.c +++ b/drivers/dma/bcm2835-dma.c @@ -419,25 +419,11 @@ static int bcm2835_dma_abort(void __iomem *chan_base) writel(0, chan_base + BCM2835_DMA_CS); /* Wait for any current AXI transfer to complete */ - while ((cs & BCM2835_DMA_ISPAUSED) && --timeout) { + while ((readl(chan_base + BCM2835_DMA_CS) & + BCM2835_DMA_WAITING_FOR_WRITES) && --timeout) cpu_relax(); - cs = readl(chan_base + BCM2835_DMA_CS); - } - - /* We'll un-pause when we set of our next DMA */ - if (!timeout) - return -ETIMEDOUT; - - if (!(cs & BCM2835_DMA_ACTIVE)) - return 0; - - /* Terminate the control block chain */ - writel(0, chan_base + BCM2835_DMA_NEXTCB); - - /* Abort the whole DMA */ - writel(BCM2835_DMA_ABORT | BCM2835_DMA_ACTIVE, - chan_base + BCM2835_DMA_CS); + writel(BCM2835_DMA_RESET, chan_base + BCM2835_DMA_CS); return 0; } @@ -787,7 +773,6 @@ static int bcm2835_dma_terminate_all(struct dma_chan *chan) struct bcm2835_chan *c = to_bcm2835_dma_chan(chan); struct bcm2835_dmadev *d = to_bcm2835_dma_dev(c->vc.chan.device); unsigned long flags; - int timeout = 10000; LIST_HEAD(head); spin_lock_irqsave(&c->vc.lock, flags); @@ -802,18 +787,6 @@ static int bcm2835_dma_terminate_all(struct dma_chan *chan) vchan_terminate_vdesc(&c->desc->vd); c->desc = NULL; bcm2835_dma_abort(c->chan_base); - - /* Wait for stopping */ - while (--timeout) { - if (!(readl(c->chan_base + BCM2835_DMA_CS) & - BCM2835_DMA_ACTIVE)) - break; - - cpu_relax(); - } - - if (!timeout) - dev_err(d->ddev.dev, "DMA transfer could not be terminated\n"); } vchan_get_all_descriptors(&c->vc, &head); From patchwork Sat Dec 22 07:28:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 10741437 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 58A3213A4 for ; Sat, 22 Dec 2018 18:22:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 465ED28A31 for ; Sat, 22 Dec 2018 18:22:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3581728A33; Sat, 22 Dec 2018 18:22:04 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5267A28A31 for ; Sat, 22 Dec 2018 18:21:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388774AbeLVSV7 (ORCPT ); Sat, 22 Dec 2018 13:21:59 -0500 Received: from mailout2.hostsharing.net ([83.223.90.233]:53723 "EHLO mailout2.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731816AbeLVSV6 (ORCPT ); Sat, 22 Dec 2018 13:21:58 -0500 Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.hostsharing.net", Issuer "COMODO RSA Domain Validation Secure Server CA" (not verified)) by mailout2.hostsharing.net (Postfix) with ESMTPS id D33DC10189B81; Sat, 22 Dec 2018 08:45:08 +0100 (CET) Received: from localhost (unknown [89.246.108.87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 734136031F05; Sat, 22 Dec 2018 08:45:08 +0100 (CET) X-Mailbox-Line: From 961e947f7f5f95a1dab079fbea0c04199eb3d60f Mon Sep 17 00:00:00 2001 Message-Id: <961e947f7f5f95a1dab079fbea0c04199eb3d60f.1545462045.git.lukas@wunner.de> In-Reply-To: References: From: Lukas Wunner Date: Sat, 22 Dec 2018 08:28:45 +0100 Subject: [PATCH 3/5] dmaengine: bcm2835: Clean up abort of transactions To: Vinod Koul , Eric Anholt , Stefan Wahren Cc: Frank Pavlic , Martin Sperl , Florian Meier , dmaengine@vger.kernel.org, linux-rpi-kernel@lists.infradead.org Sender: dmaengine-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP bcm2835_dma_abort() returns an int but bcm2835_dma_terminate_all() (its sole caller) does not evaluate the return value. Change the return type to void. Also, the "cs" variable is dispensable, so remove it. Its type seems wrong anyway, "unsigned long" is 64-bit on arm64, yet the CS register is 32-bit. Signed-off-by: Lukas Wunner Cc: Frank Pavlic Cc: Martin Sperl Cc: Florian Meier --- drivers/dma/bcm2835-dma.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c index 17bc7304db3a..a3ecb7fd4fc2 100644 --- a/drivers/dma/bcm2835-dma.c +++ b/drivers/dma/bcm2835-dma.c @@ -406,14 +406,12 @@ static void bcm2835_dma_fill_cb_chain_with_sg( } } -static int bcm2835_dma_abort(void __iomem *chan_base) +static void bcm2835_dma_abort(void __iomem *chan_base) { - unsigned long cs; long int timeout = 10000; - cs = readl(chan_base + BCM2835_DMA_CS); - if (!(cs & BCM2835_DMA_ACTIVE)) - return 0; + if (!(readl(chan_base + BCM2835_DMA_CS) & BCM2835_DMA_ACTIVE)) + return; /* Write 0 to the active bit - Pause the DMA */ writel(0, chan_base + BCM2835_DMA_CS); @@ -424,7 +422,6 @@ static int bcm2835_dma_abort(void __iomem *chan_base) cpu_relax(); writel(BCM2835_DMA_RESET, chan_base + BCM2835_DMA_CS); - return 0; } static void bcm2835_dma_start_desc(struct bcm2835_chan *c) From patchwork Sat Dec 22 07:28:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 10741439 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B0B9113A4 for ; Sat, 22 Dec 2018 18:24:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9E3D828A31 for ; Sat, 22 Dec 2018 18:24:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 928E328A33; Sat, 22 Dec 2018 18:24:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EB3C428A31 for ; Sat, 22 Dec 2018 18:24:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388371AbeLVSYT (ORCPT ); Sat, 22 Dec 2018 13:24:19 -0500 Received: from mailout3.hostsharing.net ([176.9.242.54]:55925 "EHLO mailout3.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387436AbeLVSYT (ORCPT ); Sat, 22 Dec 2018 13:24:19 -0500 Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.hostsharing.net", Issuer "COMODO RSA Domain Validation Secure Server CA" (not verified)) by mailout3.hostsharing.net (Postfix) with ESMTPS id 083E2101E6B0B; Sat, 22 Dec 2018 08:46:19 +0100 (CET) Received: from localhost (unknown [89.246.108.87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 8B6E76031F05; Sat, 22 Dec 2018 08:46:18 +0100 (CET) X-Mailbox-Line: From 05c9d6350d5d9174317e36bd0033113eb1077cc3 Mon Sep 17 00:00:00 2001 Message-Id: <05c9d6350d5d9174317e36bd0033113eb1077cc3.1545462045.git.lukas@wunner.de> In-Reply-To: References: From: Lukas Wunner Date: Sat, 22 Dec 2018 08:28:45 +0100 Subject: [PATCH 4/5] dmaengine: bcm2835: Enforce control block alignment To: Vinod Koul , Eric Anholt , Stefan Wahren Cc: Frank Pavlic , Martin Sperl , Florian Meier , dmaengine@vger.kernel.org, linux-rpi-kernel@lists.infradead.org Sender: dmaengine-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Per section 4.2.1.1 of the BCM2835 ARM Peripherals spec, control blocks "must start at a 256 bit aligned address": https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf This rule is currently satisfied only by accident because struct bcm2835_dma_cb has a size of 256 bit and the DMA pool API happens to allocate blocks consecutively. It seems safer to be explicit and tell the DMA pool allocator about the required alignment. Signed-off-by: Lukas Wunner Cc: Frank Pavlic Cc: Martin Sperl Cc: Florian Meier --- drivers/dma/bcm2835-dma.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c index a3ecb7fd4fc2..e424e232a3d0 100644 --- a/drivers/dma/bcm2835-dma.c +++ b/drivers/dma/bcm2835-dma.c @@ -498,8 +498,12 @@ static int bcm2835_dma_alloc_chan_resources(struct dma_chan *chan) dev_dbg(dev, "Allocating DMA channel %d\n", c->ch); + /* + * Control blocks are 256 bit in length and must start at a 256 bit + * (32 byte) aligned address (BCM2835 ARM Peripherals, sec. 4.2.1.1). + */ c->cb_pool = dma_pool_create(dev_name(dev), dev, - sizeof(struct bcm2835_dma_cb), 0, 0); + sizeof(struct bcm2835_dma_cb), 32, 0); if (!c->cb_pool) { dev_err(dev, "unable to allocate descriptor pool\n"); return -ENOMEM; From patchwork Sat Dec 22 07:28:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 10741387 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 129921399 for ; Sat, 22 Dec 2018 17:17:00 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E9A8528A1F for ; Sat, 22 Dec 2018 17:16:59 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DD60C28A24; Sat, 22 Dec 2018 17:16:59 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 47CD128A1F for ; Sat, 22 Dec 2018 17:16:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388040AbeLVRQ7 (ORCPT ); Sat, 22 Dec 2018 12:16:59 -0500 Received: from mailout2.hostsharing.net ([83.223.90.233]:39293 "EHLO mailout2.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725846AbeLVRQ6 (ORCPT ); Sat, 22 Dec 2018 12:16:58 -0500 X-Greylist: delayed 300 seconds by postgrey-1.27 at vger.kernel.org; Sat, 22 Dec 2018 12:16:58 EST Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.hostsharing.net", Issuer "COMODO RSA Domain Validation Secure Server CA" (not verified)) by mailout2.hostsharing.net (Postfix) with ESMTPS id 9EAD510189B38; Sat, 22 Dec 2018 08:47:51 +0100 (CET) Received: from localhost (unknown [89.246.108.87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 401396031F05; Sat, 22 Dec 2018 08:47:51 +0100 (CET) X-Mailbox-Line: From 72817c247506d84dd7b2955303a27274c72b5ef5 Mon Sep 17 00:00:00 2001 Message-Id: <72817c247506d84dd7b2955303a27274c72b5ef5.1545462045.git.lukas@wunner.de> In-Reply-To: References: From: Lukas Wunner Date: Sat, 22 Dec 2018 08:28:45 +0100 Subject: [PATCH 5/5] dmaengine: bcm2835: Remove dead code To: Vinod Koul , Eric Anholt , Stefan Wahren Cc: Frank Pavlic , Martin Sperl , Florian Meier , dmaengine@vger.kernel.org, linux-rpi-kernel@lists.infradead.org Sender: dmaengine-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The BCM2835 DMA driver deletes a channel from a list upon termination without having added it to a list first. Moreover that operation is protected by a spinlock which isn't taken anywhere else. These appear to be remnants of an older version of the driver which accidentally got mainlined. Remove the dead code. While at it remove an outdated comment claiming the driver only supports cyclic transactions. The driver has been supporting other transaction types for more than two years. Signed-off-by: Lukas Wunner Cc: Frank Pavlic Cc: Martin Sperl Cc: Florian Meier --- drivers/dma/bcm2835-dma.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c index e424e232a3d0..633be2ee7f33 100644 --- a/drivers/dma/bcm2835-dma.c +++ b/drivers/dma/bcm2835-dma.c @@ -2,9 +2,6 @@ /* * BCM2835 DMA engine support * - * This driver only supports cyclic DMA transfers - * as needed for the I2S module. - * * Author: Florian Meier * Copyright 2013 * @@ -42,7 +39,6 @@ struct bcm2835_dmadev { struct dma_device ddev; - spinlock_t lock; void __iomem *base; struct device_dma_parameters dma_parms; }; @@ -64,7 +60,6 @@ struct bcm2835_cb_entry { struct bcm2835_chan { struct virt_dma_chan vc; - struct list_head node; struct dma_slave_config cfg; unsigned int dreq; @@ -772,17 +767,11 @@ static int bcm2835_dma_slave_config(struct dma_chan *chan, static int bcm2835_dma_terminate_all(struct dma_chan *chan) { struct bcm2835_chan *c = to_bcm2835_dma_chan(chan); - struct bcm2835_dmadev *d = to_bcm2835_dma_dev(c->vc.chan.device); unsigned long flags; LIST_HEAD(head); spin_lock_irqsave(&c->vc.lock, flags); - /* Prevent this channel being scheduled */ - spin_lock(&d->lock); - list_del_init(&c->node); - spin_unlock(&d->lock); - /* stop DMA activity */ if (c->desc) { vchan_terminate_vdesc(&c->desc->vd); @@ -815,7 +804,6 @@ static int bcm2835_dma_chan_init(struct bcm2835_dmadev *d, int chan_id, c->vc.desc_free = bcm2835_dma_desc_free; vchan_init(&c->vc, &d->ddev); - INIT_LIST_HEAD(&c->node); c->chan_base = BCM2835_DMA_CHANIO(d->base, chan_id); c->ch = chan_id; @@ -918,7 +906,6 @@ static int bcm2835_dma_probe(struct platform_device *pdev) od->ddev.residue_granularity = DMA_RESIDUE_GRANULARITY_BURST; od->ddev.dev = &pdev->dev; INIT_LIST_HEAD(&od->ddev.channels); - spin_lock_init(&od->lock); platform_set_drvdata(pdev, od);