Received: by 2002:ac0:946b:0:0:0:0:0 with SMTP id j40csp2691730imj; Mon, 11 Feb 2019 07:05:50 -0800 (PST) X-Google-Smtp-Source: AHgI3Ia0tuf3mUwxkE4TdMlFaKive/1SQTf4WyGMlg+aQ4aWGeHFnyax2+D/vXIoX6NhKlaiRKor X-Received: by 2002:a63:2d6:: with SMTP id 205mr433995pgc.180.1549897550021; Mon, 11 Feb 2019 07:05:50 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1549897550; cv=none; d=google.com; s=arc-20160816; b=YEp+DW+xMfTb1FL9zrYRX0HnoCm6EPcb9a7q6b90xyJgVIQd1V9p++jhlqdlN9d54X c5rrTfxNZvpgPtvlEHG3ePqF+Ldu6sHZ2XZxQHFh0vVT/QIKa+sbeLGJL1XcD3jOMW/p jV7TkKGvjDcNKsYNJtADLw/1PrXGPYPCpUWlVxgXRPQDR2Qoy4txdpueuAgxGtkUDtAG dTmSqlh6wRBH7J/lHpzU0R5oxXvpSqYNUYHO1uzrhiQ4oLxPSA0bL8lSMtqOXg4J5le2 +gDRDwuUwRaNk3CF9IZdsuN3A1O85t25J+/j70aBd9oXYMjzBEidbd5z5EPm8yWNkNjv tnrw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=zjdgfbpwJNbB6gm+skVZLsHjkDfKbjxGo7izhIEMayA=; b=Cyb/7ruMVKOWXtOOLBXogdHknN/6/+xGbYNuoZdAWrjNYOx4GUHXwhMHpYE9X50XNs efBUalY9C6UpjieLiW/iyU0+ZOa7frEwkhEHRQoqZPnxohItBuCuUl3Cf9xqXZc1tH6j ViYgiqtCznRIqNKoQmI3OPzR7uY1TrCSMlZKIIWZxminraoR6pwClgoniFFXQ/SJ5Qc/ /B1eQDEfAXWbRfYIKFlkrIp1RxGGNnbgj6FDIZOEFYuNBHJwXPxCsqniRwCoxa6VC15N /6McW0LcjyM9yYUrGT93V0x4fsZ+aiS5/h5Czbl8wK9gANb3/R27QgqycnvbzH1gFJKQ 4dBw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=njf5Dbi6; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id c4si7051870plr.430.2019.02.11.07.05.32; Mon, 11 Feb 2019 07:05:50 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=njf5Dbi6; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390545AbfBKPDE (ORCPT + 99 others); Mon, 11 Feb 2019 10:03:04 -0500 Received: from mail.kernel.org ([198.145.29.99]:51232 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390514AbfBKPC7 (ORCPT ); Mon, 11 Feb 2019 10:02:59 -0500 Received: from localhost (5356596B.cm-6-7b.dynamic.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 03E66222A6; Mon, 11 Feb 2019 15:02:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1549897377; bh=pliT+uMc+09A6/BqO+Sdv+Q+W9b/3BBY7rhuxyEHKts=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=njf5Dbi6V5FvnAU+wPBCUPHXyB3+S/vWt2A+f1qpb5Ewr8KnXQsBtf28h3HrHeIHX 48kjOliIPU5osZ084SZERCowNJp2yeWQRlHTI/r5zIb/GZxDFwHNRtNKKTRCSCh5AO 33hDezTO4xxywEasdqY1ryVY3Nayj8VRHeL63Wfs= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Lukas Wunner , Frank Pavlic , Martin Sperl , Florian Meier , Clive Messer , Matthias Reichl , Stefan Wahren , Florian Kauer , Vinod Koul Subject: [PATCH 4.14 181/205] dmaengine: bcm2835: Fix abort of transactions Date: Mon, 11 Feb 2019 15:19:39 +0100 Message-Id: <20190211141840.225619230@linuxfoundation.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190211141827.214852402@linuxfoundation.org> References: <20190211141827.214852402@linuxfoundation.org> User-Agent: quilt/0.65 X-stable: review X-Patchwork-Hint: ignore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 4.14-stable review patch. If anyone has any objections, please let me know. ------------------ From: Lukas Wunner commit 9e528c799d17a4ac37d788c81440b50377dd592d upstream. 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 Cc: Clive Messer Cc: Matthias Reichl Tested-by: Stefan Wahren Acked-by: Florian Kauer Signed-off-by: Vinod Koul Signed-off-by: Greg Kroah-Hartman --- drivers/dma/bcm2835-dma.c | 41 +++++++++-------------------------------- 1 file changed, 9 insertions(+), 32 deletions(-) --- a/drivers/dma/bcm2835-dma.c +++ b/drivers/dma/bcm2835-dma.c @@ -415,13 +415,11 @@ static void bcm2835_dma_fill_cb_chain_wi } } -static int bcm2835_dma_abort(void __iomem *chan_base) +static int bcm2835_dma_abort(struct bcm2835_chan *c) { - unsigned long cs; + void __iomem *chan_base = c->chan_base; long int timeout = 10000; - cs = readl(chan_base + BCM2835_DMA_CS); - /* * A zero control block address means the channel is idle. * (The ACTIVE flag in the CS register is not a reliable indicator.) @@ -433,25 +431,16 @@ static int bcm2835_dma_abort(void __iome 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 */ + /* Peripheral might be stuck and fail to signal AXI write responses */ 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); + dev_err(c->vc.chan.device->dev, + "failed to complete outstanding writes\n"); + writel(BCM2835_DMA_RESET, chan_base + BCM2835_DMA_CS); return 0; } @@ -804,7 +793,6 @@ static int bcm2835_dma_terminate_all(str 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); @@ -818,18 +806,7 @@ static int bcm2835_dma_terminate_all(str if (c->desc) { bcm2835_dma_desc_free(&c->desc->vd); c->desc = NULL; - bcm2835_dma_abort(c->chan_base); - - /* Wait for stopping */ - while (--timeout) { - if (!readl(c->chan_base + BCM2835_DMA_ADDR)) - break; - - cpu_relax(); - } - - if (!timeout) - dev_err(d->ddev.dev, "DMA transfer could not be terminated\n"); + bcm2835_dma_abort(c); } vchan_get_all_descriptors(&c->vc, &head);