From patchwork Fri Dec 5 23:56:11 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandru M Stan X-Patchwork-Id: 5448461 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 57D9FBEEA8 for ; Sat, 6 Dec 2014 00:00:21 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5B4EE201C7 for ; Sat, 6 Dec 2014 00:00:20 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 42143200EC for ; Sat, 6 Dec 2014 00:00:19 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Xx2k7-0003da-1Z; Fri, 05 Dec 2014 23:56:59 +0000 Received: from mail-oi0-x241.google.com ([2607:f8b0:4003:c06::241]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Xx2k2-0003bw-1I for linux-arm-kernel@lists.infradead.org; Fri, 05 Dec 2014 23:56:55 +0000 Received: by mail-oi0-f65.google.com with SMTP id v63so211972oia.0 for ; Fri, 05 Dec 2014 15:56:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type; bh=P8IFvZjI9jmE+Zi3wm5nXHvp2FwaLg+7LdXzesyYxKI=; b=QUeKqLI129gdpMmVNMCxKP4PnI4z3sgJGCKBvIOyvwkijHeMvzxlIhIeDASoHPYK85 sayhiLjriZDj3trjRXQeHDdrRQqfILymDJrZ/8bX5k7qwGm/JCraMk3vARRjdvcnfJP3 MmqlfNwLDHDKmRjvBabYhu9gfQ+aBXxfJcyy8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=P8IFvZjI9jmE+Zi3wm5nXHvp2FwaLg+7LdXzesyYxKI=; b=UL3On9gLewx18MXonX5YpKlF6cZy6lnTfinOGUlSwNyWSBKr0MLxWcyBaKQ0xaYMmx dVNf+qutQYeyO+82Q5BB58zfJU8GIseS+zQ2jSz/7hZKD5Q3dkhrs15B2BzuR+/mbbAX yGI/DtQC9lZSP0JTAd3/xrB2OLBTYKw1yPUDpzjMt2AaIUy8zNd+uWSYrfkCMxUVk2oQ HV2uhgvS92FseINVPn5NoDcnu1/xdmvFqrRyMqnCz6yC21erSatx9XC2qTZe1J19UIkS HF7p1Q0j7ZhOvLpYNc9K2JbKg+BGQYYV8yySDxwPJMLwFFe1BD2CFZXVKE0d9YJENh0I qTtQ== X-Gm-Message-State: ALoCoQlZdZrQeRlAez7BX34hQ+lhsFtQ6Yw43xBYsvAMiySOz/4Ro5X+dDm6mGeoIVNMzpFcFqVj X-Received: by 10.60.99.99 with SMTP id ep3mr520881oeb.70.1417823791809; Fri, 05 Dec 2014 15:56:31 -0800 (PST) Received: from mail-ob0-f170.google.com (mail-ob0-f170.google.com. [209.85.214.170]) by mx.google.com with ESMTPSA id u10sm14603421oeo.14.2014.12.05.15.56.31 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 05 Dec 2014 15:56:31 -0800 (PST) Received: by mail-ob0-f170.google.com with SMTP id wp18so1355534obc.29 for ; Fri, 05 Dec 2014 15:56:31 -0800 (PST) X-Received: by 10.60.219.97 with SMTP id pn1mr469454oec.45.1417823791204; Fri, 05 Dec 2014 15:56:31 -0800 (PST) MIME-Version: 1.0 Received: by 10.202.50.70 with HTTP; Fri, 5 Dec 2014 15:56:11 -0800 (PST) In-Reply-To: References: <1415970338-2637-1-git-send-email-addy.ke@rock-chips.com> <1417576563-5033-1-git-send-email-addy.ke@rock-chips.com> From: Alexandru Stan Date: Fri, 5 Dec 2014 15:56:11 -0800 Message-ID: Subject: Re: [PATCH v3] mmc: dw_mmc: add quirk for broken data transfer over scheme To: Doug Anderson X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20141205_155654_183803_E33923DF X-CRM114-Status: GOOD ( 19.80 ) X-Spam-Score: -0.8 (/) Cc: Mark Rutland , Ulf Hansson , =?UTF-8?Q?Heiko_St=C3=BCbner?= , "linux-doc@vger.kernel.org" , Seungwon Jeon , Chris Ball , Jianqun Xu , chenfen , Chris , lintao , Jaehoon Chung , "open list:ARM/Rockchip SoC..." , han jiang , Dinh Nguyen , Tao Huang , "devicetree@vger.kernel.org" , Addy Ke , Pawel Moll , Ian Campbell , =?UTF-8?B?5aea5pm65oOF?= , zhangqing , Kever Yang , Eddie Cai , Rob Herring , Sonny Rao , "linux-arm-kernel@lists.infradead.org" , Lin Huang , Randy Dunlap , "linux-mmc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Kumar Gala , Olof Johansson , "zhenfu.fang" , zyf X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_LOW, 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 Seems like the BROKEN_DTO also affects EVENT_DATA_COMPLETE, sometimes we don't see that (but we do see EVENT_XFER_COMPLETE), leave the state machine and never come back to it to timeout. I have this happening on emmc specifically, haven't seen it on sdmmc yet. The fix is to also start the timer when we don't see a EVENT_DATA_COMPLETE. More details for how I debugged this can be found at: https://chromium-review.googlesource.com/#/c/233691/ In ~100 reboot tests I haven't seen any hangs, whereas before it was hanging about 20% of the time. Alexandru Stan (amstan) On Tue, Dec 2, 2014 at 9:08 PM, Doug Anderson wrote: > Addy, > > On Tue, Dec 2, 2014 at 7:16 PM, Addy Ke wrote: >> This patch add a new quirk to add a s/w timer to notify the driver >> to terminate current transfer and report a data timeout to the core, >> if DTO interrupt does NOT come within the given time. >> >> dw_mmc call mmc_request_done func to finish transfer depends on >> DTO interrupt. If DTO interrupt does not come in sending data state, >> the current transfer will be blocked. >> >> But this case really exists, when driver reads tuning data from >> card on RK3288-pink2 board. I measured waveforms by oscilloscope >> and found that card clock was always on and data lines were always >> holded high level in sending data state. >> >> We got the reply from synopsys: >> There are two counters but both use the same value of [31:8] bits. >> Data timeout counter doesn't wait for stop clock and you should get >> DRTO even when the clock is not stopped. >> Host Starvation timeout counter is triggered with stop clock condition. >> >> This means that host should get DRTO and DTO interrupt. >> >> But we really don't get any data-related interrupt in RK3X SoCs. >> And driver can't get data transfer state, it can do nothing but wait for. >> >> We don't know why we have this problem, but we need it to fix this problem now. >> And I will post a follow up change when we find the root cause. >> >> Signed-off-by: Addy Ke >> --- >> Changes in v2: >> - fix some typo. >> - remove extra timeout value (250ms). >> - remove dw_mci_dto_start_monitor func. >> - use "broken-dto" for new quirk and change Subject for it. >> >> Changes in v3: >> - Remove dts for broken-dto, just add this quirk in dw_mci_rockchip_init >> >> drivers/mmc/host/dw_mmc-rockchip.c | 3 +++ >> drivers/mmc/host/dw_mmc.c | 41 +++++++++++++++++++++++++++++++++++++- >> include/linux/mmc/dw_mmc.h | 5 +++++ >> 3 files changed, 48 insertions(+), 1 deletion(-) > > This seems reasonable to me. I do hope that you can get to a root > cause, but I don't think this is a terrible bit of code to carry. > Obviously it's up to the folks who maintain dw_mmc and the mmc > subsystem, but: > > Reviewed-by: Doug Anderson diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 50865e7..c678206 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -1685,8 +1685,17 @@ static void dw_mci_tasklet_func(unsigned long priv) case STATE_DATA_BUSY: if (!test_and_clear_bit(EVENT_DATA_COMPLETE, - &host->pending_events)) + &host->pending_events)) { + if (host->quirks & DW_MCI_QUIRK_BROKEN_DTO) { + drto_clks = mci_readl(host, TMOUT) >> 8; + drto_ms = DIV_ROUND_UP(drto_clks * 1000, + host->bus_hz); + + mod_timer(&host->dto_timer, jiffies + + msecs_to_jiffies(drto_ms)); + } break; + } host->data = NULL; set_bit(EVENT_DATA_COMPLETE, &host->completed_events);