From patchwork Tue Dec 16 14:11:04 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?David_Lanzend=C3=B6rfer?= X-Patchwork-Id: 5500881 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 6D9199F30B for ; Tue, 16 Dec 2014 14:14:07 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8C97B20A21 for ; Tue, 16 Dec 2014 14:14:06 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 907E020A22 for ; Tue, 16 Dec 2014 14:14:05 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Y0sqw-0001x7-24; Tue, 16 Dec 2014 14:11:54 +0000 Received: from [2001:470:26:484:6ef0:49ff:fee6:8dca] (helo=dizzy-6.o2s.ch) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Y0sqT-0001jo-8R for linux-arm-kernel@lists.infradead.org; Tue, 16 Dec 2014 14:11:29 +0000 Received: from dizzy-6.o2s.ch (localhost [IPv6:::1]) by dizzy-6.o2s.ch (Postfix) with ESMTP id E82A272108F; Tue, 16 Dec 2014 15:11:04 +0100 (CET) Subject: [PATCH 2/4] mmc: sunxi: Lock fix From: David =?utf-8?q?Lanzend=C3=B6rfer?= To: Ulf Hansson , Tomeu Vizoso , Arnd Bergmann , linux-mmc@vger.kernel.org, Chris Ball , linux-kernel@vger.kernel.org, Peter Griffin , Hans de Goede , Chen-Yu Tsai , David =?utf-8?q?Lanzend=C3=B6rfer?= , =?utf-8?b?5p2O5oOz?= , Maxime Ripard , linux-arm-kernel@lists.infradead.org Date: Tue, 16 Dec 2014 15:11:04 +0100 Message-ID: <20141216141104.3036.83687.stgit@dizzy-6.o2s.ch> In-Reply-To: <20141216140921.3036.14668.stgit@dizzy-6.o2s.ch> References: <20141216140921.3036.14668.stgit@dizzy-6.o2s.ch> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20141216_061125_503118_89064B25 X-CRM114-Status: GOOD ( 11.44 ) X-Spam-Score: 1.3 (+) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 1) Adding a comment in order to clarify the choice of the locks within sunxi_mmc_handle_manual_stop 2) As ?? has pointed out the wait_dma variable was not accessed within the spin lock block in sunxi_mmc_request and so (even if it should never happend) it would have theoretically been possible that some other function would access the variable at the same time as the function. This has been changed now and the function is using local variables outside the lock and copys the value over during the lock phase. Signed-off-by: David Lanzendörfer Reported-by: ?? --- drivers/mmc/host/sunxi-mmc.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index 1fe54a8..67e680c 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -572,6 +572,15 @@ static irqreturn_t sunxi_mmc_handle_manual_stop(int irq, void *dev_id) } dev_err(mmc_dev(host->mmc), "data error, sending stop command\n"); + + /* + * We will never have more than one outstanding request, + * and we do not complete the request until after + * we've cleared host->manual_stop_mrq so we do not need to + * spin lock this function. + * Additionally we have wait states within this function + * so having it in a lock is a very bad idea. + */ sunxi_mmc_send_manual_stop(host, mrq); spin_lock_irqsave(&host->lock, iflags); @@ -768,6 +777,7 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq) unsigned long iflags; u32 imask = SDXC_INTERRUPT_ERROR_BIT; u32 cmd_val = SDXC_START | (cmd->opcode & 0x3f); + bool wait_dma = host->wait_dma; int ret; /* Check for set_ios errors (should never happen) */ @@ -818,7 +828,7 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq) if (cmd->data->flags & MMC_DATA_WRITE) cmd_val |= SDXC_WRITE; else - host->wait_dma = true; + wait_dma = true; } else { imask |= SDXC_COMMAND_DONE; } @@ -852,6 +862,7 @@ static void sunxi_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq) } host->mrq = mrq; + host->wait_dma = wait_dma; mmc_writel(host, REG_IMASK, host->sdio_imask | imask); mmc_writel(host, REG_CARG, cmd->arg); mmc_writel(host, REG_CMDR, cmd_val);