All of lore.kernel.org
 help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Jaehoon Chung <jh80.chung@samsung.com>,
	Seungwon Jeon <tgih.jun@samsung.com>,
	Ulf Hansson <ulf.hansson@linaro.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>,
	Sonny Rao <sonnyrao@chromium.org>,
	Andrew Bresticker <abrestic@chromium.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Addy Ke <addy.ke@rock-chips.com>,
	Alexandru Stan <amstan@chromium.org>,
	javier.martinez@collabora.co.uk, Chris Zhong <zyw@rock-chips.com>,
	Caesar Wang <wxt@rock-chips.com>,
	Doug Anderson <dianders@chromium.org>,
	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	[thread overview]
Message-ID: <1431964402-27457-1-git-send-email-dianders@chromium.org> (raw)

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 <dianders@chromium.org>
---
 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;
-- 
2.2.0.rc0.207.ga3a616c


             reply	other threads:[~2015-05-18 15:54 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-18 15:53 Doug Anderson [this message]
2015-05-26 18:02 ` [PATCH] mmc: dw_mmc: Wait for data transfer after response errors Alim Akhtar
2015-05-26 20:44   ` Doug Anderson
2015-05-27  1:53     ` Jaehoon Chung
2015-05-27 16:52       ` Doug Anderson
2016-03-17 12:12 Enric Balletbo Serra
2016-03-21 22:38 ` Doug Anderson
2016-03-24 11:26   ` Enric Balletbo Serra
2016-03-24 15:16     ` Doug Anderson
2016-03-24 15:30     ` Russell King - ARM Linux
2016-03-24 16:06       ` Doug Anderson
2016-03-24 16:22         ` Russell King - ARM Linux
2016-03-30 17:16           ` Enric Balletbo Serra
2016-03-30 17:26             ` Russell King - ARM Linux
     [not found]               ` <CAFqH_51sMLbURO4n7OTEuC8S-6w0Q4aRv47nEoCAmK8-MJ+Jbw@mail.gmail.com>
2016-03-30 20:39                 ` Russell King - ARM Linux
2016-03-31  1:56               ` Shawn Lin
2016-03-31  2:03             ` Jaehoon Chung
2016-03-31  6:39               ` Enric Balletbo Serra
2016-03-31 18:12           ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1431964402-27457-1-git-send-email-dianders@chromium.org \
    --to=dianders@chromium.org \
    --cc=abrestic@chromium.org \
    --cc=addy.ke@rock-chips.com \
    --cc=alim.akhtar@samsung.com \
    --cc=amstan@chromium.org \
    --cc=heiko@sntech.de \
    --cc=javier.martinez@collabora.co.uk \
    --cc=jh80.chung@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=sonnyrao@chromium.org \
    --cc=tgih.jun@samsung.com \
    --cc=ulf.hansson@linaro.org \
    --cc=wxt@rock-chips.com \
    --cc=zyw@rock-chips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.