Received: by 2002:a25:23cc:0:0:0:0:0 with SMTP id j195csp689414ybj; Tue, 5 May 2020 06:16:43 -0700 (PDT) X-Google-Smtp-Source: APiQypLQ9GNRb22ZaBiNU8USdiMVURe2Hd5TZuII/ZEIEtlFF7D/3xB92jZHlMg9ME5M61n3m84b X-Received: by 2002:a05:6402:d0a:: with SMTP id eb10mr2528795edb.60.1588684603310; Tue, 05 May 2020 06:16:43 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1588684603; cv=none; d=google.com; s=arc-20160816; b=UeEU2MVyGJIecWT+TkgtGTBm+TsABuPpcAh0aIuly1skYiyiAjc4oEUttQxVXL+Q07 kJ90wpAWH6Dez+3+hGLiOTXlaVehK8Kp4kBzt2n9zGMt1aO7DFWTOCTszjjOp8lqAxP7 xFHjYtslYIbzFcsREHwNF7Y576lEUZX3+djd8lxggYqmxWCSq7ekZdhLrSraqprAbZHz U+uxf7CuG5N+qK8FdbzTpAzxuPFPsTj6gy86oW2CNNCql5dzffUkzEMmNvrW2suevRty teJ+86fy+meiyKj/Jg1dsG9+973HYMdMTa6p4Sw7vc/VQXaXheEcjReaHvOGz9Ftn4He 7wVQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:dkim-signature; bh=KsKPjiGD3v4nUxVeceFtJMoKp1s3SQbqenpM+1b5fpE=; b=TGZ+TJ3a3Ra5GMhxRYGpZAUQo0jbK5sY8868uwhoOr0qbYwjf84BMFp969kBOtkXFT tmRf01YnSE/gfthAjMI5/YyYz2/tk0a1EqQZZVOgU7/Qh8hTMX+uQuTIVENjqOeZFNey EOYJLs5I6Os6x0HbIjxVauBaGLiXBD4XoSHiPV1OweowMtkpl8dbxiPiwiVQJW7unKXQ RdYo2PKQHp+QxmqOGIzzNfRQbDrXgJ4lgK6IzNZ5ZefPVpSlw9vMWVh+0LQfrzR9ZqUR 6j7LgIbBQZUT48R8ZHU5UW+wb7GTypJd90RvLDgHa+RRPDOF7Qleuz+AhWwHPHVQlBwA KtSw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=Q6M4ixDU; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id x61si1190851ede.23.2020.05.05.06.16.18; Tue, 05 May 2020 06:16:43 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=Q6M4ixDU; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728980AbgEENOX (ORCPT + 99 others); Tue, 5 May 2020 09:14:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39122 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728497AbgEENOW (ORCPT ); Tue, 5 May 2020 09:14:22 -0400 Received: from mail-vk1-xa42.google.com (mail-vk1-xa42.google.com [IPv6:2607:f8b0:4864:20::a42]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0BDC0C061A0F for ; Tue, 5 May 2020 06:14:21 -0700 (PDT) Received: by mail-vk1-xa42.google.com with SMTP id i185so454057vki.12 for ; Tue, 05 May 2020 06:14:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=KsKPjiGD3v4nUxVeceFtJMoKp1s3SQbqenpM+1b5fpE=; b=Q6M4ixDU2PotV5YdJgoaS65scBDD6EAK+e/FJ2k1P714fM2+ma/nd5sp+bpfD27UVO 2kOF/9ZAJFgzAKFeADd0tl29NS0TvsAh3YeCxegL5YSiVjF7ksh76GSUbvaUDqTX8uXh o9S/F0lMuVNGeszZH3z6ZofBBG1wX7LqQR6Scd2p98dIUlxRKKpJx0Yf8mIwC28JTX4r 38nGUJEoUmUkwtPmtVrgDrX3SI3ZoEFcf5KoUF9GQUAlwZ/Oc3/7RgT/gHspux4FJccO 4s/pqW1rIJtiHAEZiptpqSbPzaE3wp0fhO8/2DpcnjTSSlZLAs1b3k3HVHHlhAMSXD2H bVnQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=KsKPjiGD3v4nUxVeceFtJMoKp1s3SQbqenpM+1b5fpE=; b=tvUGERYl5Y+CcPX5uqOu5idTc8uLBCD8IeyPmt1IdUJWJtLY4RF/yaSFnsgaf7wfpy +UUS85ggbqqzr3l3caHSxDK8FC9Yhkgn0pEWfqCRSKu50IMEj3ZFPqzdn+n9gknpU+Ff jdeKwxWnVyic+lqvP8uBzkBcbM1JFGle95DBXTrNdez1KnUNZX0oJKb34lCr13Idk7P9 R8T3BPiZpN9TC5s0k5o2nV9t4UNcAOQinLibpTAwESYFQFHn9oRwesMwMmW2ahXzyeZR MVeK5KjxI2Oi8FS0JzZjWq1DN84f/V9me1zClshq5b1Guh63i3PLZpsZT92B7+0MXfbA 7jHw== X-Gm-Message-State: AGi0PubrkAG27ibVnsRwlxhkgH4HnbedY15dVYzsjE6vX0t0+J8c/Srz qmdhLNjAfMHdNWaIBDtdBamCGDNSBCzG8clC/jbj2g== X-Received: by 2002:a1f:ff11:: with SMTP id p17mr2234355vki.25.1588684458767; Tue, 05 May 2020 06:14:18 -0700 (PDT) MIME-Version: 1.0 References: <1586835611-13857-1-git-send-email-yong.mao@mediatek.com> <1586835611-13857-2-git-send-email-yong.mao@mediatek.com> <1588066038.30914.28.camel@mhfsdcap03> <1588148417.10768.18.camel@mhfsdcap03> In-Reply-To: <1588148417.10768.18.camel@mhfsdcap03> From: Ulf Hansson Date: Tue, 5 May 2020 15:13:42 +0200 Message-ID: Subject: Re: [PATCH 1/3] mmc: core: need do mmc_power_cycle in mmc_sdio_resend_if_cond To: "yong.mao@mediatek.com" Cc: Matthias Kaehlcke , Chaotian Jing , Matthias Brugger , "linux-mmc@vger.kernel.org" , Linux ARM , "moderated list:ARM/Mediatek SoC support" , Linux Kernel Mailing List , srv_heupstream Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 29 Apr 2020 at 10:21, yong.mao@mediatek.com wrote: > > On Tue, 2020-04-28 at 14:13 +0200, Ulf Hansson wrote: > > On Tue, 28 Apr 2020 at 11:28, yong.mao@mediatek.com > > wrote: > > > > > > > > > On Fri, 2020-04-24 at 12:09 +0200, Ulf Hansson wrote: > > > > On Tue, 14 Apr 2020 at 05:40, Yong Mao wrote: > > > > > > > > > > From: yong mao > > > > > > > > > > When mmc_sdio_resned_if_cond is invoked, it indicates the SDIO > > > > > device is not in the right state. In this condition, the previous > > > > > implementation of mmc_sdio_resend_if_cond can't make sure SDIO > > > > > device be back to idle state. mmc_power_cycle can reset the SDIO > > > > > device by HW and also make sure SDIO device enter to idle state > > > > > correctly. > > > > > > > > > > Signed-off-by: Yong Mao > > > > > --- > > > > > drivers/mmc/core/sdio.c | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c > > > > > index ebb387a..ada0a80 100644 > > > > > --- a/drivers/mmc/core/sdio.c > > > > > +++ b/drivers/mmc/core/sdio.c > > > > > @@ -546,6 +546,7 @@ static int mmc_sdio_init_uhs_card(struct mmc_card *card) > > > > > static void mmc_sdio_resend_if_cond(struct mmc_host *host, > > > > > struct mmc_card *card) > > > > > { > > > > > + mmc_power_cycle(host, host->card->ocr); > > > > > > > > This looks wrong to me. mmc_sdio_resend_if_cond() is called from two places. > > > > > > > > 1. In the case when mmc_set_uhs_voltage() fails in > > > > mmc_sdio_init_card(), which means a call to mmc_power_cycle() has > > > > already been done. > > > > > > > Thanks for your comment. > > > Yes. It is right that mmc_power_cycle() has already been done when > > > mmc_sdio_resend_if_cond() is called. In normal re-initialization case, > > > this mmc_power_cycle() (currently in 1.8v voltage and 208Mhz clock) > > > can make SDIO device really back to idle state. Unfortunately, in some > > > special SDIO device, it will enter to unstable state. > > > > > > At this unstable state, device may keep data0 always low after receiving CMD11. > > > And then every other SDIO CMD can't be sent to device any more due to card > > > is busy(data0 is low). Therefore, previous implementation can't save the > > > device. At this time, mmc_power_cycle() may be the final solution to make > > > sure SDIO device can back to idle state correctly. > > > > Well, this still sounds a bit vague to me. I need to understand more > > exactly under what circumstances the problem occurs. > > > > What platform are you testing with and what SDIO card is being used? > The platform information is mt8173 + Marvell sdio device + kernel-3.18 I see, thanks for sharing this information. Forward/backporting against 3.18 is hard, perhaps impossible when it comes to this, sorry. A lot of SDIO core parts, especially related to re-initialization and power management have been changed since v3.18. "git log --oneline v3.18..v5.7-rc4 drivers/mmc/core/sdio*" will tell you. Would it be possible to move to a later kernel and test instead? I mean, the problem may already have been solved!? mt8173 should be rather well supported upstream, but perhaps lots are missing for the SDIO parts? > > > > > Is the problem happening during the system resume phase? > The problem happen when mmc_sdio_runtime_resume is invoked. > > > > Are the SDIO func driver using runtime PM and then is the host capable > > of MMC_CAP_POWER_OFF_CARD? > > > Yes. SDIO func driver uses runtime PM and MMC_CAP_POWER_OFF_CARD is > enabled. Alright, that explains the use case, thanks! > > > Is it easy to reproduce the problem for you? > > > There are only two units out of many produced units that can always > reproduce this issue. An idea to possibly help to narrow down the problem, could be to implement an "test SDIO func driver" and use that rather than the mwifiex driver (which I assume is the one you are using?). Then we could run various tests from it, like calling pm_runtime_get|put() for example. We already have a similar thing to replace the mmc/sd block device driver, so this could be useful for testing SDIO cards/funcs I think. > > > > > > > > 2. Wen sdio_read_cccr() fails and when we decide to retry the UHS-I > > > > voltage switch. Then perhaps it could make sense to run a power cycle. > > > > But if so, we better do it only for that path. > > > > > > > > I will continue to look a bit, as I think there are really more issues > > > > that we may want to look into while looking at this piece of code. > > > > However, allow me some more time before I can provide some more ideas > > > > of how to move forward. > > > In the actual project, we do encounter many relative issues about re-initialized card. > > > The following two categories are the most common issue we met before. > > > A. the SDIO card is initialized by UHS-I mode at the first time, but will be > > > re-initialized by High Speed mode at the second time. > > > ==> All this type of issues is relative with S18A in response of CMD5. > > > And most of the issues are related to the interval between powering off and > > > powering on card. This sounds a bit like the card gets re-initialized without it first being properly power cycled. Perhaps you call mmc_sw_reset() for a "test SDIO func driver", which re-initializes the card, but without doing a power cycle. Then that should give you the similar problem? > > > B. If there is something wrong in the flow of voltage switch(after CMD11), card will > > > always keep all data pins to low. And then it hangs up because data0 is always low. > > > Hope this information will be helpful for you. I keep repeating myself, but there seems to be a problem with the power cycling of the SDIO card. > > > > Thanks for sharing these details! I think we need to continue to debug > > this issue, to fully understand. > > > > In principle, it sounds to me that maybe mmc_power_cycle(), isn't > > really successfully power-cycling the SDIO card. Perhaps insert a few > > delays or so in that code to see how that would affect things? > > > > Anyway, how is the power to the SDIO card controlled in this case? Are > > you using a mmc-pwrseq? > > > vmmc is controlled through GPIO to supply 3.3v power. > And the vqmmc is supplied from PMIC which is always 1.8v. > (There is no 3.3v here. Perhaps this is one of the reasons to happen > this issues) If it's the Marvell 8787/8897/8997 SDIO module you are using, you most likely need a mmc-pwrseq to properly control the power to the SDIO module. Perhaps that is what is missing? See Documentation/devicetree/bindings/net/wireless/marvell-8xxx.txt and arch/arm/boot/dts/rk3288-veyron.dtsi for an example. > > > > > > > Anyway, we will wait for your advises. > > > > > > > > > sdio_reset(host); > > > > > mmc_go_idle(host); > > > > > mmc_send_if_cond(host, host->ocr_avail); > > > > > -- > > > > > 1.9.1 > > > > > > > > Kind regards > > > > Uffe > > > > I have a few patches in the pipe, which fixes some other problems in > > mmc_sdio_init_card(). Possibly those can be related, but I need a day > > or so to post them, let's see. > The codebase of this project is kernel-3.18. Maybe it is hard to apply > these new patches. Anyway, We will try it when we get the patches. > Thanks. As you are on a 3.18 kernel, the tests seem quite irrelevant, so I wouldn't bother with the backports. Kind regards Uffe