Re: [PATCH] mmc: dw_mmc: Wait for data transfer after response errors

From: Doug Anderson <dianders@chromium.org>
To: Enric Balletbo Serra <eballetbo@gmail.com>,
	Jaehoon Chung <jh80.chung@samsung.com>
Cc: "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Alim Akhtar <alim.akhtar@gmail.com>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	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>,
	Chris Zhong <zyw@rock-chips.com>,
	Caesar Wang <wxt@rock-chips.com>,
	Javier Martinez Canillas <javier@osg.samsung.com>,
	Russell King <linux@arm.linux.org.uk>
Subject: Re: [PATCH] mmc: dw_mmc: Wait for data transfer after response errors
Date: Thu, 24 Mar 2016 08:16:49 -0700	[thread overview]
Message-ID: <CAD=FV=X2rO+zKoq3BOLcShwA555GMXdKhA92M_SijNDkfcjA1g@mail.gmail.com> (raw)
In-Reply-To: <CAFqH_51=a7qsLFv_v3uDTsQzz8YD=GiAo3SUcR6rW_MObm=M7Q@mail.gmail.com>

Hi,

On Thu, Mar 24, 2016 at 4:26 AM, Enric Balletbo Serra
<eballetbo@gmail.com> wrote:
>> Ah, that would make some sense why things work OK on Rockchip.  Adding
>> DW_MCI_QUIRK_BROKEN_DTO to peach probably doesn't make sense, then.
>> Hrm...
>>
>> Since my original debugging of the issue was over a year ago, I think
>> I've almost totally lost context of any debugging I did on the issue,
>> so I'm not sure I'm going to be too much help in giving any details
>> other than what I put in the original commit message.  From the
>> original message it appears that I thought we could solve this other
>> ways but just that my patch was easier than the alternative of
>> handling every error case.  Maybe we just need to go back to the
>> drawing board and handle the error directly?
>>
>
> I just saw that Russell introduced a patch [1] that will land on 4.6.
> I think that patch solves the same issue that we're trying to fix, but
> for sdhci controller.

Yes, the description sounds very similar for sure.

> The problem that we have on peach-pi, with our patch applied, is that
> when we get a response CRC error on a command and we move to start
> sending data, the transfer doesn't receives a timeout interrupt (I
> don't know why). As I told, on rockchip works due the DTO quirk.
> exynos is not using this quirk. Also, please correct me if I'm wrong,
> looks like the sdhci controller has a timer to signal the command
> timed out.

I haven't spent any amount of time looking at SDHCI driver.

If we have no other better solution, enabling the DTO timer for this
specific case on all dw_mmc might be a good idea?

> ooi, anyone knows what was the test case that caused the necessity of
> the DTO quirk?

If I remember correctly, the DTO quirk is necessary to get basic
tuning working on almost every UHS card out there on rk3288.  Take it
out and you'll see lots of problems.  If you have it in there and add
a printout when the DTO quirk fires, you'll see your printout a decent
amount during tuning.  Should be easy to test that...

Those same cards work fine on exynos devices without the DTO quirk.

-Doug