From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Arend van Spriel" Subject: Re: [PATCH V2] sdhci: only reprogram retuning timer when flag is set Date: Wed, 13 Nov 2013 11:21:30 +0100 Message-ID: <528352AA.6000507@broadcom.com> References: <1383821960-2533-1-git-send-email-arend@broadcom.com> <1384163395-27038-1-git-send-email-arend@broadcom.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mms1.broadcom.com ([216.31.210.17]:2915 "EHLO mms1.broadcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753644Ab3KMKVq (ORCPT ); Wed, 13 Nov 2013 05:21:46 -0500 In-Reply-To: Sender: linux-mmc-owner@vger.kernel.org List-Id: linux-mmc@vger.kernel.org To: Dong Aisheng Cc: Chris Ball , "linux-mmc@vger.kernel.org" On 11/13/2013 06:02 AM, Dong Aisheng wrote: > Hi Arend, > > On Mon, Nov 11, 2013 at 5:49 PM, Arend van Spriel wrote: >> When the host->tuning_count is zero it means that the >> retuning is disabled. This is checked on the first >> run of sdhci_execute_tuning() by the if statement below: >> >> if (!(host->flags & SDHCI_NEEDS_RETUNING) && host->tuning_count && >> (host->tuning_mode == SDHCI_TUNING_MODE_1)) { >> >> So only when tuning_count is non-zero it will set the host >> flag SDHCI_USING_RETUNING_TIMER. The else statement is only >> for re-programming the timer, which means that flag must be >> set. Because that is not checked the else statement is executed >> in the first run when tuning_count is zero. >> >> This was seen on a host controller which indicated >> SDHCI_TUNING_MODE_1 (0) and tuning_count being zero. Suspect >> that (one of) these registers is not properly set. >> >> Signed-off-by: Arend van Spriel >> --- >> This patch applies to the mmc-next branch. >> >> V2: >> - add more explanation to the commit message >> - check host flag SDHCI_USING_RETUNING_TIMER >> --- >> drivers/mmc/host/sdhci.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> index bd8a098..5974599 100644 >> --- a/drivers/mmc/host/sdhci.c >> +++ b/drivers/mmc/host/sdhci.c >> @@ -2014,7 +2014,7 @@ out: >> host->tuning_count * HZ); >> /* Tuning mode 1 limits the maximum data length to 4MB */ >> mmc->max_blk_count = (4 * 1024 * 1024) / mmc->max_blk_size; >> - } else { >> + } else if (host->flags & SDHCI_USING_RETUNING_TIMER) { >> host->flags &= ~SDHCI_NEEDS_RETUNING; >> /* Reload the new initial value for timer */ >> if (host->tuning_mode == SDHCI_TUNING_MODE_1) > > I wonder if we could also remove this line? > It looks to me it's not neccesary to check the tuning_mode again since > we already check the flag > above and SDHCI_TUNING_MODE_1 seems like the prerequisite of > SDHCI_USING_RETUNING_TIMER. According the spec the other tuning modes also can use retuning timer. Currently, the mmc stack in upstream linux only supports tuning mode 1. When adding the other modes this if statement will probably go. Regards, Arend > Regards > Dong Aisheng > >> -- >> 1.7.10.4 >> >> >> -- >> 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 >