From patchwork Thu Oct 12 20:11:18 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 10002763 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7DC6160216 for ; Thu, 12 Oct 2017 20:12:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 701D828E98 for ; Thu, 12 Oct 2017 20:12:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6510C28EA0; Thu, 12 Oct 2017 20:12:15 +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=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 83F7B28E98 for ; Thu, 12 Oct 2017 20:12:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:References: In-Reply-To:Message-Id:Date:Subject:To:From:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=UfmqMvaSNmuY9OLbgT5aLd2MX6kDiP7lBpPw7bUeELo=; b=gNLy/soQsq0yeiSLwl6VKJ5wrW mkWdd5hxEZ9kZXx3tTSk/xa1y9cqUChZ3yLcHEYDPmhR8Jg9+5ooa8vFOrtXo5TFW9I0ZoP9GYUVM YegaGaQlpIEuUtXs/Fx7HcDyD2S9xxmV1ZGO6C+c37HZfBZDR+xKmw177UBjpTR1FH9MEhP6EgxB7 nQ0Y5y2SHd8cOdY3+SqeSUkvqTln+ZRaEf7lc15LWuffRSLOuzdYf4exv/sR3dNcYQbIs2KIwAgQJ eyPJSC2ciCFxRCI+pQsA3yvYkpX/V8xQvl0ijmiES0qSpxapp7tDaJmZNxRTH1N6sD17bG4U7iF87 qpvC5KRw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1e2jq0-00031T-Oj; Thu, 12 Oct 2017 20:12:12 +0000 Received: from mail-pf0-x22f.google.com ([2607:f8b0:400e:c00::22f]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1e2jpu-0002og-BK for linux-rockchip@lists.infradead.org; Thu, 12 Oct 2017 20:12:10 +0000 Received: by mail-pf0-x22f.google.com with SMTP id x7so6463250pfa.1 for ; Thu, 12 Oct 2017 13:11:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=mOg+tjMIGdtyxfe97OfZairUNZ/ygZrHu1ZczlsOWE8=; b=XdD6Cjddt6/688hFbJDUQ/g/TsGRPnrFA15Q7MXivRHm/shMpefNchYKR4rMqO+MlT pzaZEDyaJ84Dc/9quLiFcKtcu3uVG+hTL9V1p+h6EF+KfkmK24L+Szs/JZEgUyDSOGF2 cpWGfuK3Eax93DkVxlYPB41UCpiXkcyy/z68M= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=mOg+tjMIGdtyxfe97OfZairUNZ/ygZrHu1ZczlsOWE8=; b=KG0Krrnd2A8O5y39SJIF7iiSt63HDhTS4LlwsqSM9mpcvcu4lRpYq6gdTLCvZYpf7p HFAC9Zvn11g+QRRk6nz79Sf8cNLqKxWLimgdUaViJ3QOrZ/BSTHWUAQcLRceNJzPG6yI bcWxDlanehCbv0xderwFQF29nfhYqQE6kJ3KK5QrVIGsI9nY5fvqv9pXpWeSZklJHueK HRExG67WQrVHGOhKw63hpTK/67VJqBO8Cc819abGScEoPagJfw8ny0ls4sS/igzP0zmS 9XRcqSlyvdNq1/DlkZmn3oWM1qYGgbzV8IkSO+6LMq9phKWhF2p9tuNb4JRnkq4t3clh Wg3A== X-Gm-Message-State: AMCzsaXQlSoyVlyHz7zHfzjajgtrSBMOS0Rvl0FTUTNgtoyCNva5R8ht rjAKUr5sKYKIjFdA6nZRZYQeMA== X-Google-Smtp-Source: AOwi7QCMxX+a2es/K2LnuKbEVptrPJrtexbMXp73UMIVBOjG5uCNGY4kLsEYkfAtwnQsy9xMeIzYnQ== X-Received: by 10.98.17.202 with SMTP id 71mr3155074pfr.142.1507839105621; Thu, 12 Oct 2017 13:11:45 -0700 (PDT) Received: from tictac.mtv.corp.google.com ([172.22.112.154]) by smtp.gmail.com with ESMTPSA id l3sm29749635pgn.36.2017.10.12.13.11.44 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 12 Oct 2017 13:11:44 -0700 (PDT) From: Douglas Anderson To: jh80.chung@samsung.com, ulf.hansson@linaro.org, shawn.lin@rock-chips.com Subject: [PATCH v2 5/5] mmc: dw_mmc: Cleanup the DTO timer like the CTO one Date: Thu, 12 Oct 2017 13:11:18 -0700 Message-Id: <20171012201118.23570-6-dianders@chromium.org> X-Mailer: git-send-email 2.15.0.rc0.271.g36b669edcc-goog In-Reply-To: <20171012201118.23570-1-dianders@chromium.org> References: <20171012201118.23570-1-dianders@chromium.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171012_131206_758700_FD028BE7 X-CRM114-Status: GOOD ( 16.39 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-samsung-soc@vger.kernel.org, kernel@esmil.dk, briannorris@chromium.org, xzy.xu@rock-chips.com, linux-mmc@vger.kernel.org, Douglas Anderson , linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org, amstan@chromium.org MIME-Version: 1.0 Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP The recent CTO timer introduced in commit 03de19212ea3 ("mmc: dw_mmc: introduce timer for broken command transfer over scheme") was causing observable problems due to race conditions. Previous patches have fixed those race conditions. It can be observed that these same race conditions ought to be theoretically possible with the DTO timer too though they are massively less likely to happen because the data timeout is always set to 0xffffff right now. That means even at a 200 MHz card clock we were arming the DTO timer for 94 ms: >>> (0xffffff * 1000. / 200000000) + 10 93.886075 We always also were setting the DTO timer _after_ starting the transfer, unlike how the old code was seting the CTO timer. In any case, even though the DTO timer is much less likely to have races, it still makes sense to add code to handle it _just in case_. Signed-off-by: Douglas Anderson Reviewed-by: Shawn Lin --- Changes in v2: - Cleanup the DTO timer new for v2 drivers/mmc/host/dw_mmc.c | 54 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 6bc87b1385a9..bc0808615431 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -1950,7 +1950,11 @@ static void dw_mci_set_drto(struct dw_mci *host) /* add a bit spare time */ drto_ms += 10; - mod_timer(&host->dto_timer, jiffies + msecs_to_jiffies(drto_ms)); + spin_lock_irqsave(&host->irq_lock, irqflags); + if (!test_bit(EVENT_DATA_COMPLETE, &host->pending_events)) + mod_timer(&host->dto_timer, + jiffies + msecs_to_jiffies(drto_ms)); + spin_unlock_irqrestore(&host->irq_lock, irqflags); } static bool dw_mci_clear_pending_cmd_complete(struct dw_mci *host) @@ -1971,6 +1975,18 @@ static bool dw_mci_clear_pending_cmd_complete(struct dw_mci *host) return true; } +static bool dw_mci_clear_pending_data_complete(struct dw_mci *host) +{ + if (!test_bit(EVENT_DATA_COMPLETE, &host->pending_events)) + return false; + + /* Extra paranoia just like dw_mci_clear_pending_cmd_complete() */ + WARN_ON(del_timer_sync(&host->dto_timer)); + clear_bit(EVENT_DATA_COMPLETE, &host->pending_events); + + return true; +} + static void dw_mci_tasklet_func(unsigned long priv) { struct dw_mci *host = (struct dw_mci *)priv; @@ -2112,8 +2128,7 @@ static void dw_mci_tasklet_func(unsigned long priv) /* fall through */ case STATE_DATA_BUSY: - if (!test_and_clear_bit(EVENT_DATA_COMPLETE, - &host->pending_events)) { + if (!dw_mci_clear_pending_data_complete(host)) { /* * If data error interrupt comes but data over * interrupt doesn't come within the given time. @@ -2683,6 +2698,8 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) } if (pending & SDMMC_INT_DATA_OVER) { + spin_lock_irqsave(&host->irq_lock, irqflags); + del_timer(&host->dto_timer); mci_writel(host, RINTSTS, SDMMC_INT_DATA_OVER); @@ -2695,6 +2712,8 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) } set_bit(EVENT_DATA_COMPLETE, &host->pending_events); tasklet_schedule(&host->tasklet); + + spin_unlock_irqrestore(&host->irq_lock, irqflags); } if (pending & SDMMC_INT_RXDR) { @@ -3044,7 +3063,31 @@ static void dw_mci_cto_timer(unsigned long arg) static void dw_mci_dto_timer(unsigned long arg) { struct dw_mci *host = (struct dw_mci *)arg; + unsigned long irqflags; + u32 pending; + + spin_lock_irqsave(&host->irq_lock, irqflags); + /* + * The DTO timer is much longer than the CTO timer, so it's even less + * likely that we'll these cases, but it pays to be paranoid. + */ + pending = mci_readl(host, MINTSTS); /* read-only mask reg */ + if (pending & SDMMC_INT_DATA_OVER) { + /* The interrupt should fire; no need to act but we can warn */ + dev_warn(host->dev, "Unexpected data interrupt latency\n"); + goto exit; + } + if (test_bit(EVENT_DATA_COMPLETE, &host->pending_events)) { + /* Presumably interrupt handler couldn't delete the timer */ + dev_warn(host->dev, "DTO timeout when already completed\n"); + goto exit; + } + + /* + * Continued paranoia to make sure we're in the state we expect. + * This paranoia isn't really justified but it seems good to be safe. + */ switch (host->state) { case STATE_SENDING_DATA: case STATE_DATA_BUSY: @@ -3059,8 +3102,13 @@ static void dw_mci_dto_timer(unsigned long arg) tasklet_schedule(&host->tasklet); break; default: + dev_warn(host->dev, "Unexpected data timeout, state %d\n", + host->state); break; } + +exit: + spin_unlock_irqrestore(&host->irq_lock, irqflags); } #ifdef CONFIG_OF