From: Enric Balletbo Serra <eballetbo@gmail.com>
To: Jaehoon Chung <jh80.chung@samsung.com>
Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>,
Ulf Hansson <ulf.hansson@linaro.org>,
linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org,
Doug Anderson <dianders@chromium.org>,
Alim Akhtar <alim.akhtar@gmail.com>
Subject: Re: [RESEND PATCH v2 1/1] mmc: dw_mmc: Wait for data transfer after response errors.
Date: Tue, 21 Jun 2016 18:30:01 +0200 [thread overview]
Message-ID: <CAFqH_50ki3no8yfk29OBMpHSUQJyKKUz=2RLS5XTpRSyhKZ9eg@mail.gmail.com> (raw)
In-Reply-To: <5767C1EA.10101@samsung.com>
Hi Jaehoon,
2016-06-20 12:14 GMT+02:00 Jaehoon Chung <jh80.chung@samsung.com>:
> Hi Enric,
>
> On 04/26/2016 05:03 PM, Enric Balletbo i Serra wrote:
>> From: Doug Anderson <dianders@chromium.org>
>>
>> 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.
>
> I understood this patch what purpose has.
> Does it need to consider for other tuning cases?
>
Not sure, to be honest I only saw this during UHS tuning sequence on
this brand of cards I never reproduced the issue on other tuning
cases.
> Best Regards,
> Jaehoon Chung
>
>>
>> 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>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> Cc: Alim Akhtar <alim.akhtar@gmail.com>
>> ---
>> Changelog since v1:
>> - Fix the issue found by Alim with exynos letting the data transfer
>> take place only when MMC_SEND_TUNING_BLOCK is issued.
>>
>> drivers/mmc/host/dw_mmc.c | 27 +++++++++++++++++++++++++++
>> 1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 242f9a0..2ebeea8 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1761,6 +1761,33 @@ static void dw_mci_tasklet_func(unsigned long priv)
>> }
>>
>> if (cmd->data && err) {
>> + /*
>> + * During UHS tuning sequence, sending the stop
>> + * command after the response CRC error would
>> + * throw the system into a confused state
>> + * causing all future tuning phases to report
>> + * failure.
>> + *
>> + * In such case 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) &&
>> + (cmd->opcode == MMC_SEND_TUNING_BLOCK)) {
>> + state = STATE_SENDING_DATA;
>> + continue;
>> + }
>> +
>> dw_mci_stop_dma(host);
>> send_stop_abort(host, data);
>> state = STATE_SENDING_STOP;
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2016-06-21 16:30 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-26 8:03 [RESEND PATCH v2 0/1] mmc: dw_mmc: Fix UHS tuning on some brand of cards Enric Balletbo i Serra
2016-04-26 8:03 ` [RESEND PATCH v2 1/1] mmc: dw_mmc: Wait for data transfer after response errors Enric Balletbo i Serra
2016-06-20 10:14 ` Jaehoon Chung
2016-06-21 16:30 ` Enric Balletbo Serra [this message]
2016-06-22 1:26 ` Jaehoon Chung
2016-04-27 8:35 ` [RESEND PATCH v2 0/1] mmc: dw_mmc: Fix UHS tuning on some brand of cards Jaehoon Chung
2016-04-27 8:53 ` Enric Balletbo i Serra
2016-06-20 7:59 ` Enric Balletbo Serra
2016-06-20 8:02 ` Jaehoon Chung
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='CAFqH_50ki3no8yfk29OBMpHSUQJyKKUz=2RLS5XTpRSyhKZ9eg@mail.gmail.com' \
--to=eballetbo@gmail.com \
--cc=alim.akhtar@gmail.com \
--cc=dianders@chromium.org \
--cc=enric.balletbo@collabora.com \
--cc=jh80.chung@samsung.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=ulf.hansson@linaro.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).