From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752114AbcFVBho (ORCPT ); Tue, 21 Jun 2016 21:37:44 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:38306 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751088AbcFVBhm (ORCPT ); Tue, 21 Jun 2016 21:37:42 -0400 X-AuditID: cbfee68e-f79266d000001428-30-5769e940d401 Message-id: <5769E93F.7020809@samsung.com> Date: Wed, 22 Jun 2016 10:26:23 +0900 From: Jaehoon Chung User-Agent: Mozilla/5.0 (X11; Linux i686; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-version: 1.0 To: Enric Balletbo Serra Cc: Enric Balletbo i Serra , Ulf Hansson , linux-mmc@vger.kernel.org, linux-kernel@vger.kernel.org, Doug Anderson , Alim Akhtar Subject: Re: [RESEND PATCH v2 1/1] mmc: dw_mmc: Wait for data transfer after response errors. References: <1461657838-25484-1-git-send-email-enric.balletbo@collabora.com> <1461657838-25484-2-git-send-email-enric.balletbo@collabora.com> <5767C1EA.10101@samsung.com> In-reply-to: Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprPIsWRmVeSWpSXmKPExsWyRsSkQNfhZWa4wat3AhZLb1VbnF12kM3i wu+VbBZrbh9itLi8aw6bxZH//YwWx9eGO7B7zG64yOKx4+4SRo+ds+6ye9y5tofN4/MmuQDW KC6blNSczLLUIn27BK6M70fusxVs1q34sXIvWwPja7kuRk4OCQETib1n3jBC2GISF+6tZ+ti 5OIQEljBKLFx/xX2LkYOsKJFE6Ug4rMYJVb/6WeHcB4wSixf2MoE0s0roCWxYOdLFhCbRUBV 4tPUe+wgNpuAjsT2b8fBakQFwiQerNvLClEvKPFj8j0WkAUiAnoSn+8JgcxkFnjFKNHQfoEV JC4sEC+xaqYjxK6TTBLtr56C9XIKBEvcPTubDaSGWUBdYsqUXJAws4C8xOY1b5lB6iUErrFL fL61hhXiHgGJb5MPsUA8Iyux6QAzxMOSEgdX3GCZwCg2C8lFsxCmzkIydQEj8ypG0dSC5ILi pPQiI73ixNzi0rx0veT83E2MwHg7/e9Z3w7GmwesDzEKcDAq8fBqrM0MF2JNLCuuzD3EaAp0 xERmKdHkfGBU55XEGxqbGVmYmpgaG5lbmimJ8yZI/QwWEkhPLEnNTk0tSC2KLyrNSS0+xMjE wSnVwDhxzYFbpsqm895XXJbj3e5fIPdOuqBxd0LChUrWBadERLTPWHjyTb6//+h98YBXZ5z7 Rda9+F/WZ2b93LheQOPT09bAa3d/NKx8d+LHLTMLKbO+myt9Dnd7CPyTt2b5e9stLy+syD55 D+OMxaIX1eeovrfa1JDn9+QTl01yZ2Pkfd9t5yJFviixFGckGmoxFxUnAgBIh5rvsgIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrDIsWRmVeSWpSXmKPExsVy+t9jQV2Hl5nhBkufclksvVVtcXbZQTaL C79XslmsuX2I0eLyrjlsFkf+9zNaHF8b7sDuMbvhIovHjrtLGD12zrrL7nHn2h42j8+b5AJY oxoYbTJSE1NSixRS85LzUzLz0m2VvIPjneNNzQwMdQ0tLcyVFPISc1NtlVx8AnTdMnOArlBS KEvMKQUKBSQWFyvp22GaEBripmsB0xih6xsSBNdjZIAGEtYwZnw/cp+tYLNuxY+Ve9kaGF/L dTFycEgImEgsmijVxcgJZIpJXLi3nq2LkYtDSGAWo8TqP/3sEM4DRonlC1uZQKp4BbQkFux8 yQJiswioSnyaeo8dxGYT0JHY/u04WI2oQJjEg3V7WSHqBSV+TL7HArJMREBP4vM9IZCZzAKv GCUa2i+wgsSFBeIlVs10hNh1kkmi/dVTsF5OgWCJu2dns4HUMAuoS0yZkgsSZhaQl9i85i3z BEagKxE2zEKomoWkagEj8ypGidSC5ILipPRco7zUcr3ixNzi0rx0veT83E2M4Jh+Jr2D8fAu 90OMAhyMSjy8O6ZmhguxJpYVV+YeYpTgYFYS4e18AhTiTUmsrEotyo8vKs1JLT7EaAoMgonM UqLJ+cB0k1cSb2hsYmZkaWRuaGFkbK4kzvv4/7owIYH0xJLU7NTUgtQimD4mDk6pBsZ+yazA rdo25c0nbW6/XXJ0jWbtt99c/FqumW4PBPnOWkmxdhxaqlsTLKXGHOVRLD/3a9Ia2f07T3vN /m8RIhxa9WCe13OlhH2i54TcMzc7H919+wFjW9zbW3dWzpR9wlqz51Pi5R0NM6tU9jB/yjhW /lak7pVsov4ZTcZiuQnekywqJi45X6XEUpyRaKjFXFScCACTbt4m/wIAAA== DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Enric, On 06/22/2016 01:30 AM, Enric Balletbo Serra wrote: > Hi Jaehoon, > > 2016-06-20 12:14 GMT+02:00 Jaehoon Chung : >> Hi Enric, >> >> On 04/26/2016 05:03 PM, Enric Balletbo i Serra wrote: >>> From: Doug Anderson >>> >>> 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 >>> Signed-off-by: Enric Balletbo i Serra >>> Cc: Alim Akhtar Applied this patch on my repository. Thanks! Best Regards, Jaehoon Chung >>> --- >>> 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 > -- > 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 > > >