From patchwork Mon Aug 29 12:05:33 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Horman X-Patchwork-Id: 9303741 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id DD9046077C for ; Mon, 29 Aug 2016 12:05:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CE38628741 for ; Mon, 29 Aug 2016 12:05:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C30352885D; Mon, 29 Aug 2016 12:05:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1715028741 for ; Mon, 29 Aug 2016 12:05:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757135AbcH2MFr (ORCPT ); Mon, 29 Aug 2016 08:05:47 -0400 Received: from kirsty.vergenet.net ([202.4.237.240]:47605 "EHLO kirsty.vergenet.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757053AbcH2MFq (ORCPT ); Mon, 29 Aug 2016 08:05:46 -0400 Received: from penelope.kanocho.kobe.vergenet.net (unknown [217.111.208.18]) by kirsty.vergenet.net (Postfix) with ESMTPSA id 7ED2425B7B3; Mon, 29 Aug 2016 22:05:37 +1000 (AEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=verge.net.au; s=mail; t=1472472337; bh=9cb4cNeQNHXaKLTNaC9hyb6vZ11GkTVRrGOr6IRf310=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=pVIJNoM7JOHPAAw6YMlmxC+k6JbCW4TamJZaqChr7RsUWswcWkWv0Xuu3N6kMqeYb TQybXrBuBhxZ4CflytydhyG2GCvOR1BB5Zrc0vA0cXvzCAP1d1Jzmdpw41kA42SHFy 8DMnegCHVh7UmcOQuCs9r/HgYyP8Uri7wBklIRC0= Received: by penelope.kanocho.kobe.vergenet.net (Postfix, from userid 7100) id 5749F602CB; Mon, 29 Aug 2016 14:05:33 +0200 (CEST) Date: Mon, 29 Aug 2016 14:05:33 +0200 From: Simon Horman To: Ulf Hansson Cc: Wolfram Sang , Magnus Damm , linux-mmc , Linux-Renesas Subject: Re: [PATCH v4 2/4] mmc: tmio: Add tuning support Message-ID: <20160829120533.GA8647@verge.net.au> References: <1469592803-13842-1-git-send-email-horms+renesas@verge.net.au> <1469592803-13842-3-git-send-email-horms+renesas@verge.net.au> <20160823135251.GA25665@verge.net.au> <20160825120424.GA3385@verge.net.au> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Organisation: Horms Solutions BV User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-mmc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-mmc@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Aug 26, 2016 at 10:01:35AM +0200, Ulf Hansson wrote: > On 25 August 2016 at 14:04, Simon Horman wrote: > > On Tue, Aug 23, 2016 at 05:02:56PM +0200, Ulf Hansson wrote: ... > >> >> I am wondering whether it would it be possible to keep a cache of the > >> >> currently used tuning values and then restore these values at > >> >> runtime_resume? > >> >> > >> >> In that way you wouldn't need to force new re-tuning cycle here. > >> > > >> > I will check with the hardware people to see if it is practical from > >> > that POV. > >> > >> Okay! > > > > The feedback I received is something like this: > > > > * The tap values calculated during retuning depend on the temperature of > > the SoC and card. > > * So after resume the values may be invalid. > > They values may also become invalid during long continues transfers, > which prevents the ->runtime_suspend() callback to be invoked - > because the temperature may increase. > > > * It may be possible to re-use the TAP values and re-tune if a transfer > > fails but the people I spoke with were unsure of the merit of that > > approach > > There's is a huge benefit! > > You will get a significant decreased latency at ->runtime_resume() as > you don't need to run a complete re-tuning cycle. > > I can't really give you fresh numbers as it's a long time I tried this > myself, although if you did some measurements on this it would be > nice! :-) > > > > > Personally my feeling is that we should initiate a retune on resume for > > now as that seems to be the safest option. > > I don't agree. :-) I think it's better to postpone re-tune until it's > actually needed. > > Re-tune happens in the request error handling path, but also if you > configure a re-tuning period. That ought to be sufficient, don't you > think? Hi Ulf, Is something like this close to what you have in mind? --- 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 diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c index 94e79449c533..fc43cf5ce958 100644 --- a/drivers/mmc/host/sh_mobile_sdhi.c +++ b/drivers/mmc/host/sh_mobile_sdhi.c @@ -357,11 +357,9 @@ static void sh_mobile_sdhi_prepare_tuning(struct tmio_mmc_host *host, #define SH_MOBILE_SDHI_MAX_TAP 3 -static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host, - bool *tap, int tap_size) +static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host) { struct sh_mobile_sdhi *priv = host_to_priv(host); - unsigned long tap_num; /* total number of taps */ unsigned long tap_cnt; /* counter of tuning success */ unsigned long tap_set; /* tap position */ unsigned long tap_start;/* start position of tuning success */ @@ -372,14 +370,6 @@ static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host, /* Clear SCC_RVSREQ */ sd_scc_write32(host, priv, SH_MOBILE_SDHI_SCC_RVSREQ, 0); - /* Select SCC */ - tap_num = (sd_scc_read32(host, priv, SH_MOBILE_SDHI_SCC_DTCNTL) >> - SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_SHIFT) & - SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK; - - if (tap_num * 2 != tap_size) - return -EINVAL; - /* * Find the longest consecutive run of successful probes. If that * is more than SH_MOBILE_SDHI_MAX_TAP probes long then use the @@ -389,8 +379,8 @@ static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host, ntap = 0; tap_start = 0; tap_end = 0; - for (i = 0; i < tap_num * 2; i++) { - if (tap[i] == 0) + for (i = 0; i < host->tap_num * 2; i++) { + if (test_bit(i, host->taps)) ntap++; else { if (ntap > tap_cnt) { @@ -409,7 +399,7 @@ static int sh_mobile_sdhi_select_tuning(struct tmio_mmc_host *host, } if (tap_cnt >= SH_MOBILE_SDHI_MAX_TAP) - tap_set = (tap_start + tap_end) / 2 % tap_num; + tap_set = (tap_start + tap_end) / 2 % host->tap_num; else return -EIO; diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h index c932fe876690..4186045cce0d 100644 --- a/drivers/mmc/host/tmio_mmc.h +++ b/drivers/mmc/host/tmio_mmc.h @@ -171,8 +171,12 @@ struct tmio_mmc_host { /* Mandatory callback for tuning to occur which is * optional for SDR50 and mandatory for SDR104 */ unsigned int (*init_tuning)(struct tmio_mmc_host *host); - int (*select_tuning)(struct tmio_mmc_host *host, bool *tap, - int tap_size); + int (*select_tuning)(struct tmio_mmc_host *host); + + /* Tuning values: 1 for success, 0 for failure */ + DECLARE_BITMAP(taps, BITS_PER_BYTE * sizeof(long)); + unsigned int tap_num; + bool use_saved_taps; }; struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev); diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c index ded8fa53e0a0..2b5c8b090ed3 100644 --- a/drivers/mmc/host/tmio_mmc_pio.c +++ b/drivers/mmc/host/tmio_mmc_pio.c @@ -773,43 +773,50 @@ static void tmio_mmc_hw_reset(struct mmc_host *mmc) static int tmio_mmc_execute_tuning(struct mmc_host *mmc, u32 opcode) { struct tmio_mmc_host *host = mmc_priv(mmc); - unsigned int num; - int i, ret = 0; - bool *tap; + int ret = 0; - if (!host->init_tuning || !host->select_tuning) - /* Tuning is not supported */ - goto out; + if (!host->tap_num) { + if (!host->init_tuning || !host->select_tuning) + /* Tuning is not supported */ + goto out; - num = host->init_tuning(host); - if (!num) - /* Tuning is not supported */ - goto out; + host->tap_num = host->init_tuning(host); + if (!host->tap_num) + /* Tuning is not supported */ + goto out; - tap = kmalloc(num * 2 * sizeof(*tap), GFP_KERNEL); - if (!tap) { - ret = -ENOMEM; - goto out; + if (host->tap_num * 2 >= sizeof(host->taps) * BITS_PER_BYTE) + dev_warn_once(&host->pdev->dev, + "Too many taps, skipping tuning. Please consider " + "updating size of taps field of tmio_mmc_host\n"); + + host->use_saved_taps = false; } - /* Issue CMD19 twice for each tap */ - for (i = 0; i < 2 * num; i++) { - if (host->prepare_tuning) - host->prepare_tuning(host, i % num); + if (!host->use_saved_taps) { + int i; + + bitmap_zero(host->taps, host->tap_num * 2); - ret = mmc_send_tuning(mmc, opcode, NULL); - if (ret && ret != -EILSEQ) - goto err_free; - tap[i] = (ret != 0); + /* Issue CMD19 twice for each tap */ + for (i = 0; i < 2 * host->tap_num; i++) { + if (host->prepare_tuning) + host->prepare_tuning(host, i % host->tap_num); - mdelay(1); + ret = mmc_send_tuning(mmc, opcode, NULL); + if (ret && ret != -EILSEQ) + goto out; + if (ret == 0) + set_bit(i, host->taps); + + mdelay(1); + } } - ret = host->select_tuning(host, tap, num * 2); + ret = host->select_tuning(host); -err_free: - kfree(tap); out: + host->use_saved_taps = false; if (ret < 0) { dev_warn(&host->pdev->dev, "Tuning procedure failed\n"); mmc_hw_reset(mmc); @@ -1271,6 +1278,7 @@ int tmio_mmc_host_runtime_suspend(struct device *dev) mmc_retune_timer_stop(host->mmc); mmc_retune_needed(host->mmc); + host->use_saved_taps = true; tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_ALL);