From patchwork Mon May 18 15:53:22 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 6430271 Return-Path: X-Original-To: patchwork-linux-mmc@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 095779F3D1 for ; Mon, 18 May 2015 15:54:19 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 208B82041F for ; Mon, 18 May 2015 15:54:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1CB732042C for ; Mon, 18 May 2015 15:54:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754587AbbERPxt (ORCPT ); Mon, 18 May 2015 11:53:49 -0400 Received: from mail-ig0-f180.google.com ([209.85.213.180]:35992 "EHLO mail-ig0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754295AbbERPxl (ORCPT ); Mon, 18 May 2015 11:53:41 -0400 Received: by igbpi8 with SMTP id pi8so52239456igb.1 for ; Mon, 18 May 2015 08:53:40 -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; bh=9MZ2gm3BWtlYj9jkmpBeYvuv/Xs0fWEu9tRcPy/yYFU=; b=JzPOHf7yvLD6WaVGhOs6xG+uvB1G/ySXijGpOFCO0UsjcF5f+YXcxqDzj5XmZPu3RH 8uwnuG+R9uUwaezIwbxf4MvL6ju4n82p2DSw5nwGcJQ4jDJRAL6Pe80pjsGKvSZv9hYm qjoduHqBVACouC8kiKn79DANwP++azWA+zwlI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=9MZ2gm3BWtlYj9jkmpBeYvuv/Xs0fWEu9tRcPy/yYFU=; b=Yu48SNXkvZ9CL1wzoYAESqSjcWq5tJOTRFjbGRhqa4dMDJPCgLrsPXH9dBJ8A+PXhx iC70Kurz8UHS4Q0stQdtyNiIoNJdbWfbWB8d8zLWEJr9ActGZbAKYm23Jf2mfkbxwBQ6 XDdiMRD1ybtDR0r0Qzc5Ug/DbZ14+SGgIhF/NoMFXi0CyY7R24Q8HZZeYE3ECy3Q+6HE V11s0dKutWpo5SmKKIOVGurSBDypvKkKjb2ktOyJ4AaAoyN7dCQvve1x0a/nkZOD7ja1 y1jRewynOzTV2bQjg4J9BiVXZfJt5/qPQnKCKIWcwI2lscJNnGVdmIorX1/jMb8qrwP3 o9Mw== X-Gm-Message-State: ALoCoQm9evg8m1t6lQEw7RV7q/HIiRGNXY9rje4rpBtdkThv0ui9fPM5vNG2Fb/PnzF3ARd32VPJ X-Received: by 10.43.173.70 with SMTP id ob6mr34904857icc.45.1431964419929; Mon, 18 May 2015 08:53:39 -0700 (PDT) Received: from tictac.mtv.corp.google.com ([172.22.65.76]) by mx.google.com with ESMTPSA id h138sm7941645ioe.2.2015.05.18.08.53.38 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 18 May 2015 08:53:39 -0700 (PDT) From: Doug Anderson To: Jaehoon Chung , Seungwon Jeon , Ulf Hansson Cc: Alim Akhtar , Sonny Rao , Andrew Bresticker , Heiko Stuebner , Addy Ke , Alexandru Stan , javier.martinez@collabora.co.uk, Chris Zhong , Caesar Wang , Doug Anderson , linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] mmc: dw_mmc: Wait for data transfer after response errors Date: Mon, 18 May 2015 08:53:22 -0700 Message-Id: <1431964402-27457-1-git-send-email-dianders@chromium.org> X-Mailer: git-send-email 2.2.0.rc0.207.ga3a616c Sender: linux-mmc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=unavailable 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 According to the DesignWare state machine description, after we get a "response error" or "response CRC error" we move into data transfer mode. That means that we don't necessarily need to special case trying to deal with the failure right away. We can wait until we are notified that the data transfer is complete (with or without errors) and then we can deal with the failure. It may sound strange to defer dealing with a command that we know will fail anyway, but this appears to fix a bug. During tuning (CMD19) on a specific card on an rk3288-based system, we found that we could get a "response CRC error". Sending the stop command after the "response CRC error" would then throw the system into a confused state causing all future tuning phases to report failure. When in the confused state, the controller would show these (hex codes are interrupt status register): CMD ERR: 0x00000046 (cmd=19) CMD ERR: 0x0000004e (cmd=12) DATA ERR: 0x00000208 DATA ERR: 0x0000020c CMD ERR: 0x00000104 (cmd=19) CMD ERR: 0x00000104 (cmd=12) DATA ERR: 0x00000208 DATA ERR: 0x0000020c ... ... It is inherently difficult to deal with the complexity of trying to correctly send a stop command while a data transfer is taking place since you need to deal with different corner cases caused by the fact that the data transfer could complete (with errors or without errors) during various places in sending the stop command (dw_mci_stop_dma, send_stop_abort, etc) Instead of adding a bunch of extra complexity to deal with this, it seems much simpler to just use the more straightforward (and less error-prone) path of letting the data transfer finish. There shouldn't be any huge benefit to sending the stop command slightly earlier, anyway. Signed-off-by: Doug Anderson --- drivers/mmc/host/dw_mmc.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 5f5adaf..c081ce2 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -1574,6 +1574,26 @@ static void dw_mci_tasklet_func(unsigned long priv) } if (cmd->data && err) { + /* + * Controller will move into a data transfer + * state after a response error or response CRC + * error. Let's let that finish before trying + * to send a stop, so we'll go to + * STATE_SENDING_DATA. + * + * Although letting the data transfer take place + * will waste a bit of time (we already know + * the command was bad), it can't cause any + * errors since it's possible it would have + * taken place anyway if this tasklet got + * delayed. Allowing the transfer to take place + * avoids races and keeps things simple. + */ + if (err != -ETIMEDOUT) { + state = STATE_SENDING_DATA; + continue; + } + dw_mci_stop_dma(host); send_stop_abort(host, data); state = STATE_SENDING_STOP;