From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753176AbcGSJbk (ORCPT ); Tue, 19 Jul 2016 05:31:40 -0400 Received: from mail-wm0-f48.google.com ([74.125.82.48]:35871 "EHLO mail-wm0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752635AbcGSJbi (ORCPT ); Tue, 19 Jul 2016 05:31:38 -0400 MIME-Version: 1.0 In-Reply-To: <578CDBD8.2050806@linaro.org> References: <1463647662-27426-1-git-send-email-chaotian.jing@mediatek.com> <1463647662-27426-3-git-send-email-chaotian.jing@mediatek.com> <578CDBD8.2050806@linaro.org> From: Ulf Hansson Date: Tue, 19 Jul 2016 11:31:35 +0200 Message-ID: Subject: Re: [PATCH 2/3] mmc: mmc: do not use CMD13 to get status after speed mode switch To: Georgi Djakov Cc: Bjorn Andersson , Chaotian Jing , Matthias Brugger , Adrian Hunter , Wolfram Sang , Kuninori Morimoto , Masahiro Yamada , linux-mmc , lkml , "linux-arm-kernel@lists.infradead.org" , linux-mediatek@lists.infradead.org, srv_heupstream , Sascha Hauer , Srinivas Kandagatla Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 18 July 2016 at 15:38, Georgi Djakov wrote: > On 07/18/2016 03:50 PM, Ulf Hansson wrote: >> On 16 July 2016 at 00:10, Bjorn Andersson wrote: >>> On Thu, May 19, 2016 at 1:47 AM, Chaotian Jing >>> wrote: >>>> Per JEDEC spec, it is not recommended to use CMD13 to get card status >>>> after speed mode switch. below are two reason about this: >>>> 1. CMD13 cannot be guaranteed due to the asynchronous operation. >>>> Therefore it is not recommended to use CMD13 to check busy completion >>>> of the timing change indication. >>>> 2. After switch to HS200, CMD13 will get response of 0x800, and even the >>>> busy signal gets de-asserted, the response of CMD13 is aslo 0x800. >>>> >>>> Signed-off-by: Chaotian Jing >>>> --- >>>> drivers/mmc/core/mmc.c | 112 ++++++++++++++++++++----------------------------- >>>> 1 file changed, 45 insertions(+), 67 deletions(-) >>>> >>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >>> [..] >>>> @@ -1274,20 +1254,18 @@ static int mmc_select_hs200(struct mmc_card *card) >>>> err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, >>>> EXT_CSD_HS_TIMING, val, >>>> card->ext_csd.generic_cmd6_time, >>>> - true, send_status, true); >>>> + true, false, true); >>>> if (err) >>>> goto err; >>>> old_timing = host->ios.timing; >>>> mmc_set_timing(host, MMC_TIMING_MMC_HS200); >>>> - if (!send_status) { >>>> - err = mmc_switch_status(card); >>>> - /* >>>> - * mmc_select_timing() assumes timing has not changed if >>>> - * it is a switch error. >>>> - */ >>>> - if (err == -EBADMSG) >>>> - mmc_set_timing(host, old_timing); >>>> - } >>>> + err = mmc_switch_status(card); >>>> + /* >>>> + * mmc_select_timing() assumes timing has not changed if >>>> + * it is a switch error. >>>> + */ >>>> + if (err == -EBADMSG) >>>> + mmc_set_timing(host, old_timing); >>> >>> Sorry for not spotting this earlier, but with the move of the call of >>> mmc_switch_status() to after mmc_set_timing() we get following error >>> with sdhci-msm: >> >> Thanks for reporting! >> >>> >>> mmc0: mmc_select_hs200 failed, error -110 >>> mmc0: error -110 whilst initialising MMC card >>> mmc0: Reset 0x1 never completed. >>> sdhci: =========== REGISTER DUMP (mmc0)=========== >>> sdhci: Sys addr: 0x00000000 | Version: 0x00002e02 >>> sdhci: Blk size: 0x00004000 | Blk cnt: 0x00000000 >>> sdhci: Argument: 0x00000000 | Trn mode: 0x00000000 >>> sdhci: Present: 0x01f80000 | Host ctl: 0x00000000 >>> sdhci: Power: 0x00000000 | Blk gap: 0x00000000 >>> sdhci: Wake-up: 0x00000000 | Clock: 0x00000003 >>> sdhci: Timeout: 0x00000000 | Int stat: 0x00000000 >>> sdhci: Int enab: 0x00000000 | Sig enab: 0x00000000 >>> sdhci: AC12 err: 0x00000000 | Slot int: 0x00000000 >>> sdhci: Caps: 0x322dc8b2 | Caps_1: 0x00008007 >>> sdhci: Cmd: 0x00000000 | Max curr: 0x00000000 >>> sdhci: Host ctl2: 0x00000000 >>> sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0x0000000000000000 >>> sdhci: =========================================== >>> >>> But I if I understand the commit correctly this is the intention of >>> the patch (not the error, but the move). >> >> I tried dropping this change from my next branch, but there are some >> more changes on top that prevents a "clean" drop. It's certainly >> doable, but perhaps we can try to narrow down the problem to see if >> this could/should be fixed in the sdhci msm driver instead!? > > Yes, i am working on it. Seems that if we just ignore the -ETIMEDOUT > returned by mmc_switch_status() everything works as before. Currently > i am expecting more information about this controller specifics, in > order to understand if this could be fixed with a change in the driver > only. Thanks for looking into this! > >> >> I also noticed that below submitted change, which *isn't* applied for >> next, might be related. >> https://patchwork.kernel.org/patch/9197881/ >> > > It is not related to this issue, but it is good to have when this is > resolved, otherwise we will see the dumpregs output (pr_err now), > when controller is initialized without a card in the SD slot. Okay, thanks for clarifying. Although, I need an ack from Adrian before I apply it. > > Thanks, > Georgi Kind regards Uffe