From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adrian Hunter Subject: Re: [PATCH 02/13] mmc: host: Add facility to support re-tuning Date: Thu, 15 Jan 2015 12:17:22 +0200 Message-ID: <54B793B2.8090100@intel.com> References: <1417801271-15575-1-git-send-email-adrian.hunter@intel.com> <1417801271-15575-3-git-send-email-adrian.hunter@intel.com> <54B51C55.9060500@intel.com> <54B52D53.8080106@intel.com> <54B53592.9070303@broadcom.com> <54B54187.7080008@broadcom.com> <54B63D94.2050602@intel.com> <54B66015.1010507@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: Received: from mga11.intel.com ([192.55.52.93]:40689 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751470AbbAOKTK (ORCPT ); Thu, 15 Jan 2015 05:19:10 -0500 In-Reply-To: Sender: linux-mmc-owner@vger.kernel.org List-Id: linux-mmc@vger.kernel.org To: Ulf Hansson Cc: Arend van Spriel , Chris Ball , linux-mmc , Aaron Lu , Philip Rakity , Girish K S , Al Cooper , arindam.nath@amd.com, zhangfei.gao@marvell.com On 14/01/15 14:59, Ulf Hansson wrote: > [...] > >>> >>> The value from the register is also just randomly selected, only >>> difference is that it's the HW that has randomly set it. >> >> Presumably the value is chosen based on the maximum rate of temperature >> change and the corresponding effect that has on the signal. >> >>> >>> Even if the above commit was merged, I don't think it was the correct >>> way of dealing with re-tuning. >>> >>> First of all, re-tuning this is a mmc protocol specific thing should >>> be managed from the mmc core, like the approach you have taken in your >>> $subject patchset. Second I question whether the timer is useful at >>> all. >> >> The SD Host Controller Specification does not document another way to do >> mode 1 re-tuning. The timer is it. Otherwise re-tuning is never done. >> >> In the patches I sent, the driver must call mmc_retune_needed() to set >> host->need_retune = 1 otherwise mmc_retune() does nothing. >> >> I would like to extend the model to include transparently re-tuning and >> re-trying when there is a CRC error, but that is a separate issue, not >> documented in the spec but recommended by others. >> > > That perfect and in line from what I heard as recommendations from > memory vendors as well. How would that work for SDIO? How do you know it is OK to retry SDIO operations? > > Now, can we stop arguing about the timer and try without it? > > If we do see a need for a more frequent re-tuning to happen, due to > that we get lots of CRC errors to recover from, then I think we should > look into using runtime PM instead of the timer. And that's because I > want to minimize the impact on performance. The minimum timer value is 1 second. The maximum is 1024 seconds. The ASUS T100TA had a timer value of 128 seconds. The timer is not a performance issue. There is a performance question with runtime PM because that happens far more frequently (typical auto-suspend delay is 50ms) and we re-tune after that. In fact I generalized that a bit in patch 13. [PATCH 13/13] mmc: sdhci: Change to new way of doing re-tuning Make use of mmc core support for re-tuning instead of doing it all in the sdhci driver. This patch also changes to flag the need for re-tuning always after runtime suspend when tuning has been used at initialization. Previously it was only done if the re-tuning timer was in use. One option to reduce the impact of the latency would be to increase the auto-suspend delay. I have cc'ed the author of "mmc: sdhci: add support for retuning mode 1"